Skip to content

Commit e5f122a

Browse files
[monitor opentelemetry exporter] Handle ServiceResponseError as retryable to prevent telemetry loss (#47870)
* Handle ServiceResponseError as retryable to prevent telemetry loss ServiceResponseError (e.g. read timeout) was not caught by the exporter's _transmit method. Since ServiceResponseError and ServiceRequestError are sibling classes (both extend AzureError directly), a read timeout fell through to the generic 'except Exception' handler, which marked the batch as FAILED_NOT_RETRYABLE — permanently dropping the telemetry instead of persisting it to local storage for retry. This change adds ServiceResponseError to the import and catch chain, treating it as FAILED_RETRYABLE so that envelopes are written to disk and retried on the next successful export cycle, consistent with the existing ServiceRequestError handling and the azure-core docstring which states these errors 'can be retried for idempotent or safe operations'. * Cleanup verbose comment * Retrigger CI/CD pipeline * Add CHANGELOG * Add ServiceResponseTimeoutError to the classification categories * Add tests and expand network exceptions category * Fix format * Retrigger CI/CD pipeline --------- Co-authored-by: Radhika Gupta <guptaradhika@microsoft.com>
1 parent 2cad71a commit e5f122a

6 files changed

Lines changed: 73 additions & 5 deletions

File tree

sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
## 1.0.0b56 (Unreleased)
44

55
### Features Added
6+
- Retry payloads for which request is sent successfully but no response is received, per [SPEC.](https://github.com/aep-health-and-standards/Telemetry-Collection-Spec/pull/1018)
7+
([#47870](https://github.com/Azure/azure-sdk-for-python/pull/47870))
68

79
### Breaking Changes
810

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from urllib.parse import urlparse
1313
import psutil
1414

15-
from azure.core.exceptions import HttpResponseError, ServiceRequestError
15+
from azure.core.exceptions import HttpResponseError, ServiceRequestError, ServiceResponseError
1616
from azure.core.pipeline.policies import (
1717
ContentDecodePolicy,
1818
HttpLoggingPolicy,
@@ -598,6 +598,21 @@ def _transmit(self, envelopes: List[TelemetryItem], _skip_rate_limit: bool = Fal
598598
exc_type = request_error.__class__.__name__ # type: ignore
599599
_update_requests_map(_REQ_EXCEPTION_NAME[1], value=exc_type)
600600
result = ExportResult.FAILED_RETRYABLE
601+
except ServiceResponseError as response_error:
602+
# The request was sent but the client failed to receive a response
603+
# (e.g. read timeout).
604+
logger.warning("Retrying due to server response error: %s.", response_error.message)
605+
606+
# Track retry items in customer sdkstats for client-side exceptions
607+
if self._should_collect_customer_sdkstats():
608+
track_retry_items(envelopes, response_error)
609+
610+
if self._should_collect_stats():
611+
exc_type = response_error.exc_type
612+
if exc_type is None or exc_type is type(None): # pylint: disable=unidiomatic-typecheck
613+
exc_type = response_error.__class__.__name__ # type: ignore
614+
_update_requests_map(_REQ_EXCEPTION_NAME[1], value=exc_type)
615+
result = ExportResult.FAILED_RETRYABLE
601616
except Exception as ex:
602617
logger.exception( # pylint: disable=do-not-log-exceptions-if-not-debug, do-not-use-logging-exception
603618
"Envelopes could not be exported and are not retryable: %s.", ex

sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/statsbeat/customer/_utils.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55

66
# mypy: disable-error-code="import-untyped"
77
from requests import ReadTimeout, Timeout # pylint: disable=networking-import-outside-azure-core-transport
8-
from azure.core.exceptions import ServiceRequestTimeoutError
8+
from azure.core.exceptions import (
9+
ServiceRequestTimeoutError,
10+
ServiceResponseTimeoutError,
11+
ServiceResponseError,
12+
ServiceRequestError,
13+
)
914
from azure.monitor.opentelemetry.exporter._constants import (
1015
_REQUEST,
1116
RetryCode,
@@ -95,11 +100,14 @@ def _determine_client_retry_code(
95100
"""
96101
timeout_exception_types = (
97102
ServiceRequestTimeoutError,
103+
ServiceResponseTimeoutError,
98104
ReadTimeout,
99105
TimeoutError,
100106
Timeout,
101107
)
102108
network_exception_types = (
109+
ServiceRequestError,
110+
ServiceResponseError,
103111
ConnectionError,
104112
OSError,
105113
)

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/customer_sdk_stats/test_utlities.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@
77
from datetime import datetime
88

99
from requests.exceptions import ConnectionError, ReadTimeout, Timeout
10-
from azure.core.exceptions import ServiceRequestTimeoutError, HttpResponseError
10+
from azure.core.exceptions import (
11+
ServiceRequestTimeoutError,
12+
HttpResponseError,
13+
ServiceResponseTimeoutError,
14+
ServiceResponseError,
15+
ServiceRequestError,
16+
)
1117

1218
from azure.monitor.opentelemetry.exporter._generated.exporter.models import (
1319
TelemetryItem,
@@ -310,6 +316,7 @@ def test_determine_client_retry_code_timeout_errors(self):
310316
ReadTimeout("Read timeout"),
311317
TimeoutError("Generic timeout"),
312318
Timeout("Requests timeout"),
319+
ServiceResponseTimeoutError("Service response timeout"),
313320
]
314321

315322
for error in timeout_errors:
@@ -343,6 +350,8 @@ def test_determine_client_retry_code_network_errors(self):
343350
network_errors = [
344351
ConnectionError("Connection failed"),
345352
OSError("OS error occurred"),
353+
ServiceResponseError("Service response error"),
354+
ServiceRequestError("Service request error"),
346355
]
347356

348357
for error in network_errors:

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_base_customer_sdkstats.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from unittest import mock
77
from datetime import datetime
88

9-
from azure.core.exceptions import HttpResponseError, ServiceRequestError
9+
from azure.core.exceptions import HttpResponseError, ServiceRequestError, ServiceResponseError
1010
from azure.monitor.opentelemetry.exporter.export._base import (
1111
BaseExporter,
1212
ExportResult,
@@ -215,6 +215,16 @@ def test_transmit_service_request_error_customer_sdkstats_track_retry_items(self
215215
track_retry_mock.assert_called_once()
216216
self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
217217

218+
@mock.patch("azure.monitor.opentelemetry.exporter.export._base.track_retry_items")
219+
def test_transmit_service_response_error_customer_sdkstats_track_retry_items(self, track_retry_mock):
220+
"""Test that _track_retry_items is called on ServiceResponseError (e.g. read timeout)"""
221+
exporter = self._create_exporter_with_customer_sdkstats_enabled()
222+
with mock.patch.object(AzureMonitorClient, "track", side_effect=ServiceResponseError("Read timed out")):
223+
result = exporter._transmit(self._envelopes_to_export)
224+
225+
track_retry_mock.assert_called_once()
226+
self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
227+
218228
@mock.patch("azure.monitor.opentelemetry.exporter.export._base.track_dropped_items")
219229
def test_transmit_general_exception_customer_sdkstats_track_dropped_items(self, track_dropped_mock):
220230
"""Test that _track_dropped_items is called on general exceptions"""

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_base_exporter.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from unittest import mock
88
from datetime import datetime, timedelta, timezone
99

10-
from azure.core.exceptions import HttpResponseError, ServiceRequestError
10+
from azure.core.exceptions import HttpResponseError, ServiceRequestError, ServiceResponseError
1111
from azure.core.pipeline.transport import HttpResponse
1212
from azure.monitor.opentelemetry.exporter.export._base import (
1313
_get_auth_policy,
@@ -861,6 +861,11 @@ def test_transmit_request_error(self):
861861
result = self._base._transmit(self._envelopes_to_export)
862862
self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
863863

864+
def test_transmit_response_error(self):
865+
with mock.patch.object(AzureMonitorClient, "track", throw(ServiceResponseError, message="Read timed out")):
866+
result = self._base._transmit(self._envelopes_to_export)
867+
self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
868+
864869
def test_transmission_200(self):
865870
with mock.patch.object(AzureMonitorClient, "track") as post:
866871
post.return_value = TrackResponse(
@@ -1180,6 +1185,25 @@ def test_transmit_request_error_statsbeat(self, stats_mock):
11801185
self.assertEqual(_REQUESTS_MAP["count"], 1)
11811186
self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
11821187

1188+
@mock.patch.dict(
1189+
os.environ,
1190+
{
1191+
"APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL": "false",
1192+
"APPLICATIONINSIGHTS_SDKSTATS_DISABLED": "false",
1193+
},
1194+
)
1195+
@mock.patch("azure.monitor.opentelemetry.exporter.statsbeat._statsbeat.collect_statsbeat_metrics")
1196+
def test_transmit_response_error_statsbeat(self, stats_mock):
1197+
exporter = BaseExporter(disable_offline_storage=True)
1198+
with mock.patch.object(AzureMonitorClient, "track", throw(ServiceResponseError, message="Read timed out")):
1199+
result = exporter._transmit(self._envelopes_to_export)
1200+
stats_mock.assert_called_once()
1201+
self.assertEqual(len(_REQUESTS_MAP), 3)
1202+
self.assertEqual(_REQUESTS_MAP[_REQ_EXCEPTION_NAME[1]]["ServiceResponseError"], 1)
1203+
self.assertIsNotNone(_REQUESTS_MAP[_REQ_DURATION_NAME[1]])
1204+
self.assertEqual(_REQUESTS_MAP["count"], 1)
1205+
self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
1206+
11831207
def test_transmit_request_exception(self):
11841208
with mock.patch.object(AzureMonitorClient, "track", throw(Exception)):
11851209
result = self._base._transmit(self._envelopes_to_export)

0 commit comments

Comments
 (0)