OT Exporter retry when there are network issues (#16087)

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

@@ -2,18 +2,21 @@
 
 ## 1.0.0b2 (Unreleased)
 
+- Fix to only retry upon request error
+  ([#16087](https://github.com/Azure/azure-sdk-for-python/pull/16087))
+
   **Breaking Changes**
   - Rename Azure Trace exporter class, only allow connection string configuration
     ([#15349](https://github.com/Azure/azure-sdk-for-python/pull/15349))
 
-- OpenTelemetry Exporter use Resources API to retrieve cloud role props
-  ([#15816](https://github.com/Azure/azure-sdk-for-python/pull/15816))
+  - OpenTelemetry Exporter use Resources API to retrieve cloud role props
+    ([#15816](https://github.com/Azure/azure-sdk-for-python/pull/15816))
 
-- Change span to envelope conversion to adhere to common schema and other languages
-  ([#15344](https://github.com/Azure/azure-sdk-for-python/pull/15344))
+  - Change span to envelope conversion to adhere to common schema and other languages
+    ([#15344](https://github.com/Azure/azure-sdk-for-python/pull/15344))
 
-- This library is renamed to `azure-opentelemetry-exporter-azuremonitor`.
-  ([#15344](https://github.com/Azure/azure-sdk-for-python/pull/15344))
+  - This library is renamed to `azure-opentelemetry-exporter-azuremonitor`.
+    ([#16030](https://github.com/Azure/azure-sdk-for-python/pull/16030))
 
 ## 1.0.0b1 (2020-11-13)
 

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

@@ -8,7 +8,7 @@
 
 from opentelemetry.sdk.trace.export import SpanExportResult
 
-from azure.core.exceptions import HttpResponseError
+from azure.core.exceptions import HttpResponseError, ServiceRequestError
 from azure.core.pipeline.policies import ContentDecodePolicy, HttpLoggingPolicy, RequestIdPolicy
 from azure.opentelemetry.exporter.azuremonitor._generated import AzureMonitorClient
 from azure.opentelemetry.exporter.azuremonitor._generated._configuration import AzureMonitorClientConfiguration
@@ -132,12 +132,18 @@ def _transmit(self, envelopes: typing.List[TelemetryItem]) -> ExportResult:
                 if is_retryable_code(response_error.status_code):
                     return ExportResult.FAILED_RETRYABLE
                 return ExportResult.FAILED_NOT_RETRYABLE
-            except Exception as ex:
+            except ServiceRequestError as request_error:
+                # Errors when we're fairly sure that the server did not receive the
+                # request, so it should be safe to retry.
                 logger.warning(
-                    "Retrying due to transient client side error %s.", ex
+                    "Retrying due to server request error: %s.", request_error
                 )
-                # client side error (retryable)
                 return ExportResult.FAILED_RETRYABLE
+            except Exception as ex:
+                logger.error(
+                    "Envelopes could not be exported and are not retryable: %s.", ex
+                )
+                return ExportResult.FAILED_NOT_RETRYABLE
             return ExportResult.FAILED_NOT_RETRYABLE
         # No spans to export
         return ExportResult.SUCCESS
@@ -148,12 +154,14 @@ def is_retryable_code(response_code: int) -> bool:
     Determine if response is retryable
     """
     return bool(response_code in (
-        206,  # Retriable
+        206,  # Partial success
         408,  # Timeout
         429,  # Throttle, too Many Requests
         439,  # Quota, too Many Requests over extended time
         500,  # Internal Server Error
+        502,  # BadGateway
         503,  # Service Unavailable
+        504,  # Gateway timeout
     ))
 
 

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

@@ -12,6 +12,7 @@
 from opentelemetry.sdk.metrics.export import MetricsExportResult
 from opentelemetry.sdk.trace.export import SpanExportResult
 
+from azure.core.exceptions import HttpResponseError, ServiceRequestError
 from azure.opentelemetry.exporter.azuremonitor.export._base import (
     BaseExporter,
     ExportResult,
@@ -116,10 +117,29 @@ def test_transmit_request_timeout(self):
             result = self._base._transmit(self._envelopes_to_export)
         self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
 
+    def test_transmit_http_error_retryable(self):
+        with mock.patch("azure.opentelemetry.exporter.azuremonitor.export._base.is_retryable_code") as m:
+            m.return_value = True
+            with mock.patch("requests.Session.request", throw(HttpResponseError)):
+                result = self._base._transmit(self._envelopes_to_export)
+            self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
+
+    def test_transmit_http_error_retryable(self):
+        with mock.patch("azure.opentelemetry.exporter.azuremonitor.export._base.is_retryable_code") as m:
+            m.return_value = False
+            with mock.patch("requests.Session.request", throw(HttpResponseError)):
+                result = self._base._transmit(self._envelopes_to_export)
+            self.assertEqual(result, ExportResult.FAILED_NOT_RETRYABLE)
+
+    def test_transmit_request_error(self):
+        with mock.patch("requests.Session.request", throw(ServiceRequestError, message="error")):
+            result = self._base._transmit(self._envelopes_to_export)
+        self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
+
     def test_transmit_request_exception(self):
         with mock.patch("requests.Session.request", throw(Exception)):
             result = self._base._transmit(self._envelopes_to_export)
-        self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
+        self.assertEqual(result, ExportResult.FAILED_NOT_RETRYABLE)
 
     def test_transmission_200(self):
         with mock.patch("requests.Session.request") as post:
@@ -236,12 +256,24 @@ def test_transmission_500(self):
             result = self._base._transmit(self._envelopes_to_export)
         self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
 
+    def test_transmission_502(self):
+        with mock.patch("requests.Session.request") as post:
+            post.return_value = MockResponse(503, "{}")
+            result = self._base._transmit(self._envelopes_to_export)
+        self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
+
     def test_transmission_503(self):
         with mock.patch("requests.Session.request") as post:
             post.return_value = MockResponse(503, "{}")
             result = self._base._transmit(self._envelopes_to_export)
         self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
 
+    def test_transmission_504(self):
+        with mock.patch("requests.Session.request") as post:
+            post.return_value = MockResponse(504, "{}")
+            result = self._base._transmit(self._envelopes_to_export)
+        self.assertEqual(result, ExportResult.FAILED_RETRYABLE)
+
     def test_transmission_empty(self):
         status = self._base._transmit([])
         self.assertEqual(status, ExportResult.SUCCESS)