From 11733979fc1ae25a1b2605b2e4320684ee87fcde Mon Sep 17 00:00:00 2001 From: alrex Date: Thu, 14 May 2020 22:19:57 -0700 Subject: [PATCH] api: ensure status is always string (#640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensuring that exporters can always expect status descriptions to be a string. This was implemented to be defensive against instrumentations such as PyMongo, which currently set status as a dict. Co-authored-by: Mauricio Vásquez Co-authored-by: Yusuke Tsutsumi --- .../otcollector/trace_exporter/__init__.py | 1 + .../tests/test_otcollector_trace_exporter.py | 9 +++++ .../src/opentelemetry/trace/status.py | 9 ++++- opentelemetry-api/tests/trace/test_status.py | 35 +++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 opentelemetry-api/tests/trace/test_status.py diff --git a/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py b/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py index fb6237e86d8..914d97ec5a9 100644 --- a/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py +++ b/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py @@ -98,6 +98,7 @@ def translate_to_collector(spans: Sequence[Span]): code=span.status.canonical_code.value, message=span.status.description, ) + collector_span = trace_pb2.Span( name=trace_pb2.TruncatableString(value=span.name), kind=utils.get_collector_span_kind(span.kind), diff --git a/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py b/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py index 4a0a556137d..97d276af403 100644 --- a/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py +++ b/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py @@ -154,6 +154,11 @@ def test_translate_to_collector(self): ) otel_spans[0].end(end_time=end_times[0]) otel_spans[1].start(start_time=start_times[1]) + otel_spans[1].set_status( + trace_api.Status( + trace_api.status.StatusCanonicalCode.INTERNAL, {"test", "val"}, + ) + ) otel_spans[1].end(end_time=end_times[1]) otel_spans[2].start(start_time=start_times[2]) otel_spans[2].end(end_time=end_times[2]) @@ -263,6 +268,10 @@ def test_translate_to_collector(self): output_spans[0].links.link[0].type, trace_pb2.Span.Link.Type.TYPE_UNSPECIFIED, ) + self.assertEqual( + output_spans[1].status.code, + trace_api.status.StatusCanonicalCode.INTERNAL.value, + ) self.assertEqual( output_spans[2].links.link[0].type, trace_pb2.Span.Link.Type.PARENT_LINKED_SPAN, diff --git a/opentelemetry-api/src/opentelemetry/trace/status.py b/opentelemetry-api/src/opentelemetry/trace/status.py index 4ae2ad96d08..4191369dfe9 100644 --- a/opentelemetry-api/src/opentelemetry/trace/status.py +++ b/opentelemetry-api/src/opentelemetry/trace/status.py @@ -13,8 +13,11 @@ # limitations under the License. import enum +import logging import typing +logger = logging.getLogger(__name__) + class StatusCanonicalCode(enum.Enum): """Represents the canonical set of status codes of a finished Span.""" @@ -167,7 +170,11 @@ def __init__( description: typing.Optional[str] = None, ): self._canonical_code = canonical_code - self._description = description + self._description = None + if description is not None and not isinstance(description, str): + logger.warning("Invalid status description type, expected str") + else: + self._description = description @property def canonical_code(self) -> StatusCanonicalCode: diff --git a/opentelemetry-api/tests/trace/test_status.py b/opentelemetry-api/tests/trace/test_status.py new file mode 100644 index 00000000000..6c086f3ae04 --- /dev/null +++ b/opentelemetry-api/tests/trace/test_status.py @@ -0,0 +1,35 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest +from logging import WARNING + +from opentelemetry.trace.status import Status, StatusCanonicalCode + + +class TestStatus(unittest.TestCase): + def test_constructor(self): + status = Status() + self.assertIs(status.canonical_code, StatusCanonicalCode.OK) + self.assertIsNone(status.description) + + status = Status(StatusCanonicalCode.UNAVAILABLE, "unavailable") + self.assertIs(status.canonical_code, StatusCanonicalCode.UNAVAILABLE) + self.assertEqual(status.description, "unavailable") + + def test_invalid_description(self): + with self.assertLogs(level=WARNING): + status = Status(description={"test": "val"}) # type: ignore + self.assertIs(status.canonical_code, StatusCanonicalCode.OK) + self.assertEqual(status.description, None)