Address review: bound bisect depth, respect shutdown, simplify control flow

- Add _MAX_BISECTS=5 to cap recursive splitting depth
- Combine 413 guard with len>1 and remaining_bisects>0 check so
  single-item 413 falls through to the existing non-retryable path
- Check self._shutdown alongside deadline in the 413 handler
- Add tests for max bisect depth exhaustion and shutdown during 413

Author: Krishnachaitanyakc

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

@@ -70,6 +70,7 @@
 DEFAULT_LOGS_EXPORT_PATH = "v1/logs"
 DEFAULT_TIMEOUT = 10  # in seconds
 _MAX_RETRYS = 6
+_MAX_BISECTS = 5
 
 
 class OTLPLogExporter(LogRecordExporter):
@@ -185,10 +186,13 @@ def export(
             return LogRecordExportResult.FAILURE
 
         deadline_sec = time() + self._timeout
-        return self._export_batch(batch, deadline_sec)
+        return self._export_batch(batch, deadline_sec, _MAX_BISECTS)
 
     def _export_batch(
-        self, batch: Sequence[ReadableLogRecord], deadline_sec: float
+        self,
+        batch: Sequence[ReadableLogRecord],
+        deadline_sec: float,
+        remaining_bisects: int,
     ) -> LogRecordExportResult:
         serialized_data = encode_logs(batch).SerializeToString()
 
@@ -208,17 +212,17 @@ def _export_batch(
                 retryable = _is_retryable(resp)
                 status_code = resp.status_code
 
-                if _is_payload_too_large(resp):
-                    # 413 handling always returns here; will not fall through
-                    # to the 'if not retryable' check below.
-                    if len(batch) <= 1:
+                if (
+                    _is_payload_too_large(resp)
+                    and len(batch) > 1
+                    and remaining_bisects > 0
+                ):
+                    if time() >= deadline_sec or self._shutdown:
                         _logger.error(
-                            "Single log record exceeds backend payload size limit, dropping log record"
-                        )
-                        return LogRecordExportResult.FAILURE
-                    if time() >= deadline_sec:
-                        _logger.error(
-                            "Payload too large but deadline expired, dropping %d log records",
+                            "Payload too large but %s, dropping %d log records",
+                            "shutdown in progress"
+                            if self._shutdown
+                            else "deadline expired",
                             len(batch),
                         )
                         return LogRecordExportResult.FAILURE
@@ -227,13 +231,18 @@ def _export_batch(
                         "Payload too large (%d log records), splitting into two batches",
                         len(batch),
                     )
-                    first = self._export_batch(list(batch[:mid]), deadline_sec)
+                    first = self._export_batch(
+                        list(batch[:mid]),
+                        deadline_sec,
+                        remaining_bisects - 1,
+                    )
                     if first != LogRecordExportResult.SUCCESS:
                         return LogRecordExportResult.FAILURE
-                    second = self._export_batch(
-                        list(batch[mid:]), deadline_sec
+                    return self._export_batch(
+                        list(batch[mid:]),
+                        deadline_sec,
+                        remaining_bisects - 1,
                     )
-                    return second
 
             if not retryable:
                 _logger.error(

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

@@ -66,6 +66,7 @@
 DEFAULT_TRACES_EXPORT_PATH = "v1/traces"
 DEFAULT_TIMEOUT = 10  # in seconds
 _MAX_RETRYS = 6
+_MAX_BISECTS = 5
 
 
 class OTLPSpanExporter(SpanExporter):
@@ -178,10 +179,13 @@ def export(self, spans: Sequence[ReadableSpan]) -> SpanExportResult:
             return SpanExportResult.FAILURE
 
         deadline_sec = time() + self._timeout
-        return self._export_batch(spans, deadline_sec)
+        return self._export_batch(spans, deadline_sec, _MAX_BISECTS)
 
     def _export_batch(
-        self, spans: Sequence[ReadableSpan], deadline_sec: float
+        self,
+        spans: Sequence[ReadableSpan],
+        deadline_sec: float,
+        remaining_bisects: int,
     ) -> SpanExportResult:
         serialized_data = encode_spans(spans).SerializePartialToString()
 
@@ -201,17 +205,17 @@ def _export_batch(
                 retryable = _is_retryable(resp)
                 status_code = resp.status_code
 
-                if _is_payload_too_large(resp):
-                    # 413 handling always returns here; will not fall through
-                    # to the 'if not retryable' check below.
-                    if len(spans) <= 1:
+                if (
+                    _is_payload_too_large(resp)
+                    and len(spans) > 1
+                    and remaining_bisects > 0
+                ):
+                    if time() >= deadline_sec or self._shutdown:
                         _logger.error(
-                            "Single span exceeds backend payload size limit, dropping span"
-                        )
-                        return SpanExportResult.FAILURE
-                    if time() >= deadline_sec:
-                        _logger.error(
-                            "Payload too large but deadline expired, dropping %d spans",
+                            "Payload too large but %s, dropping %d spans",
+                            "shutdown in progress"
+                            if self._shutdown
+                            else "deadline expired",
                             len(spans),
                         )
                         return SpanExportResult.FAILURE
@@ -220,13 +224,18 @@ def _export_batch(
                         "Payload too large (%d spans), splitting into two batches",
                         len(spans),
                     )
-                    first = self._export_batch(list(spans[:mid]), deadline_sec)
+                    first = self._export_batch(
+                        list(spans[:mid]),
+                        deadline_sec,
+                        remaining_bisects - 1,
+                    )
                     if first != SpanExportResult.SUCCESS:
                         return SpanExportResult.FAILURE
-                    second = self._export_batch(
-                        list(spans[mid:]), deadline_sec
+                    return self._export_batch(
+                        list(spans[mid:]),
+                        deadline_sec,
+                        remaining_bisects - 1,
                     )
-                    return second
 
             if not retryable:
                 _logger.error(

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

@@ -594,7 +594,7 @@ def test_413_splits_batch_and_succeeds(self, mock_post):
 
     @patch.object(Session, "post")
     def test_413_single_log_returns_failure(self, mock_post):
-        """When a single log record is too large, the exporter should return FAILURE."""
+        """When a single log record is too large, the exporter should return FAILURE via the non-retryable path."""
         exporter = OTLPLogExporter(timeout=10)
 
         resp_413 = Response()
@@ -612,8 +612,7 @@ def test_413_single_log_returns_failure(self, mock_post):
         self.assertEqual(mock_post.call_count, 1)
         self.assertTrue(
             any(
-                "Single log record exceeds backend payload size limit"
-                in record.message
+                "Failed to export logs batch code: 413" in record.message
                 for record in warning.records
             )
         )
@@ -695,3 +694,45 @@ def test_413_deadline_expired_returns_failure(self, mock_post, mock_time):
                 for record in warning.records
             )
         )
+
+    @patch.object(Session, "post")
+    def test_413_max_bisects_exceeded_returns_failure(self, mock_post):
+        """When max bisect depth is exhausted, the exporter should stop splitting and return FAILURE."""
+        exporter = OTLPLogExporter(timeout=10)
+
+        resp_413 = Response()
+        resp_413.status_code = 413
+        resp_413.reason = "Request Entity Too Large"
+
+        # Always return 413 — should stop after _MAX_BISECTS depth
+        mock_post.return_value = resp_413
+
+        log_data = self._get_sdk_log_data()
+
+        with self.assertLogs(level=WARNING):
+            result = exporter.export(log_data)
+
+        self.assertEqual(result, LogRecordExportResult.FAILURE)
+        # Should not recurse indefinitely — bounded by _MAX_BISECTS
+
+    @patch.object(Session, "post")
+    def test_413_shutdown_returns_failure(self, mock_post):
+        """When a 413 is received but shutdown is in progress, return FAILURE without splitting."""
+        exporter = OTLPLogExporter(timeout=10)
+
+        resp_413 = Response()
+        resp_413.status_code = 413
+        resp_413.reason = "Request Entity Too Large"
+
+        mock_post.return_value = resp_413
+
+        log_data = self._get_sdk_log_data()
+
+        # Set shutdown before export
+        exporter._shutdown = True
+
+        with self.assertLogs(level=WARNING):
+            result = exporter.export(log_data)
+
+        # export() itself returns FAILURE early on shutdown
+        self.assertEqual(result, LogRecordExportResult.FAILURE)

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

@@ -430,7 +430,7 @@ def test_413_splits_batch_and_succeeds(self, mock_post):
 
     @patch.object(Session, "post")
     def test_413_single_span_returns_failure(self, mock_post):
-        """When a single span is too large, the exporter should return FAILURE."""
+        """When a single span is too large, the exporter should return FAILURE via the non-retryable path."""
         exporter = OTLPSpanExporter(timeout=10)
 
         resp_413 = Response()
@@ -446,8 +446,7 @@ def test_413_single_span_returns_failure(self, mock_post):
         self.assertEqual(mock_post.call_count, 1)
         self.assertTrue(
             any(
-                "Single span exceeds backend payload size limit"
-                in record.message
+                "Failed to export span batch code: 413" in record.message
                 for record in warning.records
             )
         )
@@ -582,3 +581,80 @@ def test_413_deadline_expired_returns_failure(self, mock_post, mock_time):
                 for record in warning.records
             )
         )
+
+    @patch.object(Session, "post")
+    def test_413_max_bisects_exceeded_returns_failure(self, mock_post):
+        """When max bisect depth is exhausted, the exporter should stop splitting and return FAILURE."""
+        exporter = OTLPSpanExporter(timeout=10)
+
+        resp_413 = Response()
+        resp_413.status_code = 413
+        resp_413.reason = "Request Entity Too Large"
+
+        # Always return 413 — should stop after _MAX_BISECTS depth
+        mock_post.return_value = resp_413
+
+        # Create a batch large enough to bisect _MAX_BISECTS times (2^5 = 32 items)
+        spans = []
+        for idx in range(32):
+            spans.append(
+                _Span(
+                    f"span{idx}",
+                    context=Mock(
+                        **{
+                            "trace_state": {},
+                            "span_id": 10217189687419569865 + idx,
+                            "trace_id": 67545097771067222548457157018666467027,
+                        }
+                    ),
+                )
+            )
+
+        with self.assertLogs(level=WARNING):
+            result = exporter.export(spans)
+
+        self.assertEqual(result, SpanExportResult.FAILURE)
+        # Should not recurse indefinitely — bounded by _MAX_BISECTS
+        # At depth 5, batch is 1 item → falls through to non-retryable path
+        self.assertLessEqual(mock_post.call_count, 63)
+
+    @patch.object(Session, "post")
+    def test_413_shutdown_returns_failure(self, mock_post):
+        """When a 413 is received but shutdown is in progress, return FAILURE without splitting."""
+        exporter = OTLPSpanExporter(timeout=10)
+
+        resp_413 = Response()
+        resp_413.status_code = 413
+        resp_413.reason = "Request Entity Too Large"
+
+        mock_post.return_value = resp_413
+
+        span1 = _Span(
+            "span1",
+            context=Mock(
+                **{
+                    "trace_state": {},
+                    "span_id": 10217189687419569865,
+                    "trace_id": 67545097771067222548457157018666467027,
+                }
+            ),
+        )
+        span2 = _Span(
+            "span2",
+            context=Mock(
+                **{
+                    "trace_state": {},
+                    "span_id": 10217189687419569866,
+                    "trace_id": 67545097771067222548457157018666467027,
+                }
+            ),
+        )
+
+        # Set shutdown before export
+        exporter._shutdown = True
+
+        with self.assertLogs(level=WARNING):
+            result = exporter.export([span1, span2])
+
+        # export() itself returns FAILURE early on shutdown
+        self.assertEqual(result, SpanExportResult.FAILURE)