diff --git a/.changelog/5473.fixed b/.changelog/5473.fixed new file mode 100644 index 0000000000..82e490fce6 --- /dev/null +++ b/.changelog/5473.fixed @@ -0,0 +1 @@ +`opentelemetry-exporter-otlp-proto-http`: retry exports on HTTP 429 (Too Many Requests) responses diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py index 46db16dd86..63d22e8f38 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_common/__init__.py @@ -26,6 +26,12 @@ class RequestPayloadTooLargeError(Exception): def _is_retryable(resp: requests.Response) -> bool: if resp.status_code == 408: return True + # 429 (Too Many Requests) is listed as a retryable status code by the OTLP + # specification, so a throttled export should be retried with backoff rather + # than dropped. See + # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md#failures-1 + if resp.status_code == 429: + return True if resp.status_code >= 500 and resp.status_code <= 599: return True return False diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py index 9ead610069..811f7eab0a 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py @@ -1427,6 +1427,24 @@ def test_preferred_aggregation_override(self): exporter._preferred_aggregation[Histogram], histogram_aggregation ) + @patch.object(Session, "post") + def test_retryable_status_code_429(self, mock_post): + # HTTP 429 (Too Many Requests) is retryable per the OTLP spec, so the + # exporter should back off and retry instead of dropping the batch. + exporter = OTLPMetricExporter(timeout=1.5) + + resp = Response() + resp.status_code = 429 + resp.reason = "Too Many Requests" + mock_post.return_value = resp + with self.assertLogs(level=WARNING): + self.assertEqual( + exporter.export(self.metrics["sum_int"]), + MetricExportResult.FAILURE, + ) + # More than one call proves the 429 response was retried. + self.assertGreater(mock_post.call_count, 1) + @patch.dict( "os.environ", {OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED: "true"} ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_common.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_common.py new file mode 100644 index 0000000000..aa6b5d1760 --- /dev/null +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_common.py @@ -0,0 +1,28 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + +import unittest + +import requests + +from opentelemetry.exporter.otlp.proto.http._common import _is_retryable + + +class TestIsRetryable(unittest.TestCase): + @staticmethod + def _response(status_code: int) -> requests.Response: + resp = requests.Response() + resp.status_code = status_code + return resp + + def test_retryable_status_codes(self): + # 408 (Request Timeout), 429 (Too Many Requests) and any 5xx are + # retryable per the OTLP specification. + for status_code in (408, 429, 500, 502, 503, 504, 599): + with self.subTest(status_code=status_code): + self.assertTrue(_is_retryable(self._response(status_code))) + + def test_non_retryable_status_codes(self): + for status_code in (200, 400, 401, 403, 404, 409): + with self.subTest(status_code=status_code): + self.assertFalse(_is_retryable(self._response(status_code))) diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py index 3663b0eb9b..81d1807b30 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py @@ -457,6 +457,24 @@ def test_2xx_status_code(self, mock_otlp_metric_exporter): LogRecordExportResult.SUCCESS, ) + @patch.object(Session, "post") + def test_retryable_status_code_429(self, mock_post): + # HTTP 429 (Too Many Requests) is retryable per the OTLP spec, so the + # exporter should back off and retry instead of dropping the batch. + exporter = OTLPLogExporter(timeout=1.5) + + resp = Response() + resp.status_code = 429 + resp.reason = "Too Many Requests" + mock_post.return_value = resp + with self.assertLogs(level=WARNING): + self.assertEqual( + exporter.export(self._get_sdk_log_data()), + LogRecordExportResult.FAILURE, + ) + # More than one call proves the 429 response was retried. + self.assertGreater(mock_post.call_count, 1) + @patch.dict( "os.environ", {OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED: " true "} ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py index fa8b5fc1f4..e07339e9f3 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py @@ -313,6 +313,24 @@ def test_exporter_metrics_disabled_by_default(self, _mock_export): self.assertIsNone(self.metric_reader.get_metrics_data()) + @patch.object(Session, "post") + def test_retryable_status_code_429(self, mock_post): + # HTTP 429 (Too Many Requests) is retryable per the OTLP spec, so the + # exporter should back off and retry instead of dropping the batch. + exporter = OTLPSpanExporter(timeout=1.5) + + resp = Response() + resp.status_code = 429 + resp.reason = "Too Many Requests" + mock_post.return_value = resp + with self.assertLogs(level=WARNING): + self.assertEqual( + exporter.export([BASIC_SPAN]), + SpanExportResult.FAILURE, + ) + # More than one call proves the 429 response was retried. + self.assertGreater(mock_post.call_count, 1) + @patch.dict( "os.environ", {OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED: " true "} )