Skip to content

Commit 151cc92

Browse files
Address review: bind retry_on_413 env var to instance variable, remove redundant deadline check
- Cache _OTEL_PYTHON_EXPERIMENTAL_OTLP_RETRY_ON_413 as self._retry_entity_too_large in constructor instead of reading environ on every retry iteration - Remove redundant deadline/shutdown guard inside bisectable block; the retry-exit check (backoff > remaining time) already covers this case - Update deadline-expired tests to match new control flow
1 parent 03f61fc commit 151cc92

4 files changed

Lines changed: 20 additions & 42 deletions

File tree

exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/_log_exporter/__init__.py

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,12 @@ def __init__(
141141
{"Content-Encoding": self._compression.value}
142142
)
143143
self._shutdown = False
144+
self._retry_entity_too_large = (
145+
environ.get(_OTEL_PYTHON_EXPERIMENTAL_OTLP_RETRY_ON_413, "")
146+
.strip()
147+
.lower()
148+
== "true"
149+
)
144150

145151
def _export(
146152
self, serialized_data: bytes, timeout_sec: Optional[float] = None
@@ -217,12 +223,7 @@ def _export_batch(
217223
_is_payload_too_large(resp)
218224
and len(batch) > 1
219225
and remaining_bisects > 0
220-
and environ.get(
221-
_OTEL_PYTHON_EXPERIMENTAL_OTLP_RETRY_ON_413, ""
222-
)
223-
.strip()
224-
.lower()
225-
== "true"
226+
and self._retry_entity_too_large
226227
)
227228

228229
if not retryable and not bisectable:
@@ -245,15 +246,6 @@ def _export_batch(
245246
return LogRecordExportResult.FAILURE
246247

247248
if bisectable:
248-
if time() >= deadline_sec or self._shutdown:
249-
_logger.error(
250-
"Payload too large but %s, dropping %d log records",
251-
"shutdown in progress"
252-
if self._shutdown
253-
else "deadline expired",
254-
len(batch),
255-
)
256-
return LogRecordExportResult.FAILURE
257249
mid = len(batch) // 2
258250
_logger.warning(
259251
"Payload too large (%d log records), splitting into two batches",

exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ def __init__(
136136
{"Content-Encoding": self._compression.value}
137137
)
138138
self._shutdown = False
139+
self._retry_entity_too_large = (
140+
environ.get(_OTEL_PYTHON_EXPERIMENTAL_OTLP_RETRY_ON_413, "")
141+
.strip()
142+
.lower()
143+
== "true"
144+
)
139145

140146
def _export(
141147
self, serialized_data: bytes, timeout_sec: Optional[float] = None
@@ -210,12 +216,7 @@ def _export_batch(
210216
_is_payload_too_large(resp)
211217
and len(spans) > 1
212218
and remaining_bisects > 0
213-
and environ.get(
214-
_OTEL_PYTHON_EXPERIMENTAL_OTLP_RETRY_ON_413, ""
215-
)
216-
.strip()
217-
.lower()
218-
== "true"
219+
and self._retry_entity_too_large
219220
)
220221

221222
if not retryable and not bisectable:
@@ -238,15 +239,6 @@ def _export_batch(
238239
return SpanExportResult.FAILURE
239240

240241
if bisectable:
241-
if time() >= deadline_sec or self._shutdown:
242-
_logger.error(
243-
"Payload too large but %s, dropping %d spans",
244-
"shutdown in progress"
245-
if self._shutdown
246-
else "deadline expired",
247-
len(spans),
248-
)
249-
return SpanExportResult.FAILURE
250242
mid = len(spans) // 2
251243
_logger.warning(
252244
"Payload too large (%d spans), splitting into two batches",

exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -702,8 +702,8 @@ def test_413_partial_failure(self, mock_post):
702702
def test_413_deadline_expired_returns_failure(self, mock_post, mock_time):
703703
"""When a 413 is received but the deadline has expired, return FAILURE without splitting."""
704704
# time() calls: export() deadline_sec setup, _export timeout calc,
705-
# retry-exit check (deadline_sec - time()), bisect deadline check
706-
mock_time.side_effect = [100.0, 100.0, 100.0, 110.1]
705+
# retry-exit check (deadline_sec - time())
706+
mock_time.side_effect = [100.0, 100.0, 110.1]
707707
exporter = OTLPLogExporter(timeout=10)
708708

709709
resp_413 = Response()
@@ -720,10 +720,7 @@ def test_413_deadline_expired_returns_failure(self, mock_post, mock_time):
720720
self.assertEqual(result, LogRecordExportResult.FAILURE)
721721
self.assertEqual(mock_post.call_count, 1)
722722
self.assertTrue(
723-
any(
724-
"deadline expired" in record.message
725-
for record in warning.records
726-
)
723+
any("timeout" in record.message for record in warning.records)
727724
)
728725

729726
@patch.dict(

exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,8 @@ def test_413_partial_failure(self, mock_post):
561561
def test_413_deadline_expired_returns_failure(self, mock_post, mock_time):
562562
"""When a 413 is received but the deadline has expired, return FAILURE without splitting."""
563563
# time() calls: export() deadline_sec setup, _export timeout calc,
564-
# retry-exit check (deadline_sec - time()), bisect deadline check
565-
mock_time.side_effect = [100.0, 100.0, 100.0, 110.1]
564+
# retry-exit check (deadline_sec - time())
565+
mock_time.side_effect = [100.0, 100.0, 110.1]
566566
exporter = OTLPSpanExporter(timeout=10)
567567

568568
resp_413 = Response()
@@ -598,10 +598,7 @@ def test_413_deadline_expired_returns_failure(self, mock_post, mock_time):
598598
self.assertEqual(result, SpanExportResult.FAILURE)
599599
self.assertEqual(mock_post.call_count, 1)
600600
self.assertTrue(
601-
any(
602-
"deadline expired" in record.message
603-
for record in warning.records
604-
)
601+
any("timeout" in record.message for record in warning.records)
605602
)
606603

607604
@patch.dict(

0 commit comments

Comments
 (0)