From fe26adaa07f88324c933a75f1a4dd199c9c8b2d7 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 7 Jul 2022 16:16:57 +0200 Subject: [PATCH] Fix warning message for OTLP gRPC exporter mixin (#2781) * Fix warning message for OTLP gRPC exporter mixin Fixes #2780 * Refactor export parameter type * Add changelog entry * Use fixed warning messages for traces and metrics * Use subclass-specific error messages * Fix test cases * Fix lint --- CHANGELOG.md | 6 +- .../otlp/proto/grpc/_log_exporter/__init__.py | 4 ++ .../exporter/otlp/proto/grpc/exporter.py | 39 +++++++++-- .../proto/grpc/metric_exporter/__init__.py | 4 ++ .../proto/grpc/trace_exporter/__init__.py | 4 ++ .../tests/logs/test_otlp_logs_exporter.py | 4 ++ .../metrics/test_otlp_metrics_exporter.py | 4 ++ .../tests/test_otlp_exporter_mixin.py | 68 ++++++++++++++++++- .../tests/test_otlp_trace_exporter.py | 4 ++ 9 files changed, 126 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d874d0b4f..5026184349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,15 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.12.0rc2-0.32b0...HEAD) - +- Fix OTLP gRPC exporter warning message + ([#2781](https://github.com/open-telemetry/opentelemetry-python/pull/2781)) - Fix tracing decorator with late configuration ([#2754](https://github.com/open-telemetry/opentelemetry-python/pull/2754)) - ## [1.12.0rc2-0.32b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc2-0.32b0) - 2022-07-04 - - - Fix instrument name and unit regexes ([#2796](https://github.com/open-telemetry/opentelemetry-python/pull/2796)) - Add optional sessions parameter to all Exporters leveraging requests.Session diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_log_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_log_exporter/__init__.py index 51433d5740..489cf35c37 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_log_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_log_exporter/__init__.py @@ -154,3 +154,7 @@ def export(self, batch: Sequence[LogData]) -> LogExportResult: def shutdown(self) -> None: pass + + @property + def _exporting(self) -> str: + return "logs" diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py index b965061c5c..4405bcad68 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py @@ -14,7 +14,7 @@ """OTLP Exporter""" -import logging +from logging import getLogger from abc import ABC, abstractmethod from collections.abc import Sequence from os import environ @@ -23,6 +23,7 @@ from typing import Sequence as TypingSequence from typing import TypeVar from urllib.parse import urlparse +from opentelemetry.sdk.trace import ReadableSpan from backoff import expo from google.rpc.error_details_pb2 import RetryInfo @@ -52,8 +53,9 @@ ) from opentelemetry.sdk.resources import Resource as SDKResource from opentelemetry.util.re import parse_headers +from opentelemetry.sdk.metrics.export import MetricsData -logger = logging.getLogger(__name__) +logger = getLogger(__name__) SDKDataT = TypeVar("SDKDataT") ResourceDataT = TypeVar("ResourceDataT") TypingResourceT = TypeVar("TypingResourceT") @@ -277,8 +279,19 @@ def _translate_attributes(self, attributes) -> TypingSequence[KeyValue]: logger.exception(error) return output - def _export(self, data: TypingSequence[SDKDataT]) -> ExportResultT: - + def _export( + self, data: Union[TypingSequence[ReadableSpan], MetricsData] + ) -> ExportResultT: + + # FIXME remove this check if the export type for traces + # gets updated to a class that represents the proto + # TracesData and use the code below instead. + # logger.warning( + # "Transient error %s encountered while exporting %s, retrying in %ss.", + # error.code(), + # data.__class__.__name__, + # delay, + # ) max_value = 64 # expo returns a generator that yields delay values which grow # exponentially. Once delay is greater than max_value, the yielded @@ -321,15 +334,20 @@ def _export(self, data: TypingSequence[SDKDataT]) -> ExportResultT: ) logger.warning( - "Transient error %s encountered while exporting span batch, retrying in %ss.", + ( + "Transient error %s encountered while exporting " + "%s, retrying in %ss." + ), error.code(), + self._exporting, delay, ) sleep(delay) continue else: logger.error( - "Failed to export span batch, error code: %s", + "Failed to export %s, error code: %s", + self._exporting, error.code(), ) @@ -342,3 +360,12 @@ def _export(self, data: TypingSequence[SDKDataT]) -> ExportResultT: def shutdown(self) -> None: pass + + @property + @abstractmethod + def _exporting(self) -> str: + """ + Returns a string that describes the overall exporter, to be used in + warning messages. + """ + pass diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py index c5f4acad06..fb316ab2e8 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py @@ -206,3 +206,7 @@ def export( def shutdown(self, timeout_millis: float = 30_000, **kwargs) -> None: pass + + @property + def _exporting(self) -> str: + return "metrics" diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py index 084a5d93b1..5626012536 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py @@ -289,3 +289,7 @@ def _translate_data( def export(self, spans: Sequence[ReadableSpan]) -> SpanExportResult: return self._export(spans) + + @property + def _exporting(self): + return "traces" diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py index 4ee8f6a0b3..a9c63eaa0a 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py @@ -161,6 +161,10 @@ def setUp(self): def tearDown(self): self.server.stop(None) + def test_exporting(self): + # pylint: disable=protected-access + self.assertEqual(self.exporter._exporting, "logs") + @patch( "opentelemetry.exporter.otlp.proto.grpc.exporter.ssl_channel_credentials" ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py index 8936272bef..c25ab06263 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py @@ -298,6 +298,10 @@ def setUp(self): def tearDown(self): self.server.stop(None) + def test_exporting(self): + # pylint: disable=protected-access + self.assertEqual(self.exporter._exporting, "metrics") + @patch( "opentelemetry.exporter.otlp.proto.grpc.exporter.ssl_channel_credentials" ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_exporter_mixin.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_exporter_mixin.py index a7627b237c..3f44ef228e 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_exporter_mixin.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_exporter_mixin.py @@ -12,13 +12,21 @@ # See the License for the specific language governing permissions and # limitations under the License. +from logging import WARNING +from types import MethodType +from typing import Sequence from unittest import TestCase -from unittest.mock import patch +from unittest.mock import Mock, patch from grpc import Compression from opentelemetry.exporter.otlp.proto.grpc.exporter import ( + ExportServiceRequestT, InvalidCompressionValueException, + OTLPExporterMixin, + RpcError, + SDKDataT, + StatusCode, environ_to_compression, ) @@ -47,3 +55,61 @@ def test_environ_to_compression(self): ) with self.assertRaises(InvalidCompressionValueException): environ_to_compression("test_invalid") + + @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.expo") + def test_export_warning(self, mock_expo): + + mock_expo.configure_mock(**{"return_value": [0]}) + + rpc_error = RpcError() + + def code(self): + return None + + rpc_error.code = MethodType(code, rpc_error) + + class OTLPMockExporter(OTLPExporterMixin): + + _result = Mock() + _stub = Mock( + **{"return_value": Mock(**{"Export.side_effect": rpc_error})} + ) + + def _translate_data( + self, data: Sequence[SDKDataT] + ) -> ExportServiceRequestT: + pass + + @property + def _exporting(self) -> str: + return "mock" + + otlp_mock_exporter = OTLPMockExporter() + + with self.assertLogs(level=WARNING) as warning: + # pylint: disable=protected-access + otlp_mock_exporter._export(Mock()) + self.assertEqual( + warning.records[0].message, + "Failed to export mock, error code: None", + ) + + def code(self): # pylint: disable=function-redefined + return StatusCode.CANCELLED + + def trailing_metadata(self): + return {} + + rpc_error.code = MethodType(code, rpc_error) + rpc_error.trailing_metadata = MethodType(trailing_metadata, rpc_error) + + with self.assertLogs(level=WARNING) as warning: + # pylint: disable=protected-access + otlp_mock_exporter._export([]) + self.assertEqual( + warning.records[0].message, + ( + "Transient error StatusCode.CANCELLED encountered " + "while exporting mock, retrying in 0s." + ), + ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py index 3d83672901..a5cb4e699a 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py @@ -217,6 +217,10 @@ def setUp(self): def tearDown(self): self.server.stop(None) + def test_exporting(self): + # pylint: disable=protected-access + self.assertEqual(self.exporter._exporting, "traces") + @patch.dict( "os.environ", {