From 196f1239088e07b937911d9362d6fd6b7a145899 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sat, 4 Jun 2022 01:33:12 +0530 Subject: [PATCH 1/7] Allow `set_status` to accept the StatusCode and optional description --- .../src/opentelemetry/trace/span.py | 14 ++++++-- .../src/opentelemetry/sdk/trace/__init__.py | 29 +++++++++++----- opentelemetry-sdk/tests/trace/test_trace.py | 34 ++++++++++++++++++- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index cb9992557cc..805b2b06b18 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -5,7 +5,7 @@ import typing from collections import OrderedDict -from opentelemetry.trace.status import Status +from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util import types # The key MUST begin with a lowercase letter or a digit, @@ -137,7 +137,11 @@ def is_recording(self) -> bool: """ @abc.abstractmethod - def set_status(self, status: Status) -> None: + def set_status( + self, + status: typing.Union[Status, StatusCode], + description: typing.Optional[str] = None, + ) -> None: """Sets the Status of the Span. If used, this will override the default Span status. """ @@ -524,7 +528,11 @@ def add_event( def update_name(self, name: str) -> None: pass - def set_status(self, status: Status) -> None: + def set_status( + self, + status: typing.Union[Status, StatusCode], + description: typing.Optional[str] = None, + ) -> None: pass def record_exception( diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index f5279673781..3ed6d722ac8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -889,16 +889,29 @@ def is_recording(self) -> bool: return self._end_time is None @_check_span_ended - def set_status(self, status: trace_api.Status) -> None: + def set_status( + self, + status: typing.Union[trace_api.Status, trace_api.StatusCode], + description: typing.Optional[str] = None, + ) -> None: # Ignore future calls if status is already set to OK # Ignore calls to set to StatusCode.UNSET - if ( - self._status - and self._status.status_code is StatusCode.OK - or status.status_code is StatusCode.UNSET - ): - return - self._status = status + if isinstance(status, trace_api.Status): + if ( + self._status + and self._status.status_code is StatusCode.OK + or status.status_code is StatusCode.UNSET + ): + return + self._status = status + elif isinstance(status, trace_api.StatusCode): + if ( + self._status + and self._status.status_code is StatusCode.OK + or status is StatusCode.UNSET + ): + return + self._status = trace_api.Status(status, description) def __exit__( self, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index ad165789734..403b69cbf25 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -45,7 +45,7 @@ get_span_with_dropped_attributes_events_links, new_tracer, ) -from opentelemetry.trace import StatusCode +from opentelemetry.trace import Status, StatusCode from opentelemetry.util._time import _time_ns @@ -903,6 +903,38 @@ def test_span_override_start_and_end_time(self): span.end(end_time) self.assertEqual(end_time, span.end_time) + def test_span_set_status(self): + + span1 = self.tracer.start_span("span1") + span1.set_status(Status(status_code=StatusCode.ERROR)) + self.assertEqual(span1.status.status_code, StatusCode.ERROR) + self.assertEqual(span1.status.description, None) + + span2 = self.tracer.start_span("span2") + span2.set_status( + Status(status_code=StatusCode.ERROR, description="desc") + ) + self.assertEqual(span2.status.status_code, StatusCode.ERROR) + self.assertEqual(span2.status.description, "desc") + + span3 = self.tracer.start_span("span3") + span3.set_status(StatusCode.ERROR) + self.assertEqual(span3.status.status_code, StatusCode.ERROR) + self.assertEqual(span3.status.description, None) + + span4 = self.tracer.start_span("span4") + span4.set_status(StatusCode.ERROR, "span4 desc") + self.assertEqual(span4.status.status_code, StatusCode.ERROR) + self.assertEqual(span4.status.description, "span4 desc") + + span5 = self.tracer.start_span("span5") + span5.set_status( + Status(status_code=StatusCode.ERROR, description="desc"), + description="ignored", + ) + self.assertEqual(span5.status.status_code, StatusCode.ERROR) + self.assertEqual(span5.status.description, "desc") + def test_ended_span(self): """Events, attributes are not allowed after span is ended""" From 1dfb0f012eb758a2786c834d1d1cae8dc650d0ff Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sat, 4 Jun 2022 01:37:58 +0530 Subject: [PATCH 2/7] Update the refs --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 3ed6d722ac8..1e2b7179f52 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -891,12 +891,12 @@ def is_recording(self) -> bool: @_check_span_ended def set_status( self, - status: typing.Union[trace_api.Status, trace_api.StatusCode], + status: typing.Union[Status, StatusCode], description: typing.Optional[str] = None, ) -> None: # Ignore future calls if status is already set to OK # Ignore calls to set to StatusCode.UNSET - if isinstance(status, trace_api.Status): + if isinstance(status, Status): if ( self._status and self._status.status_code is StatusCode.OK @@ -904,14 +904,14 @@ def set_status( ): return self._status = status - elif isinstance(status, trace_api.StatusCode): + elif isinstance(status, StatusCode): if ( self._status and self._status.status_code is StatusCode.OK or status is StatusCode.UNSET ): return - self._status = trace_api.Status(status, description) + self._status = Status(status, description) def __exit__( self, From 7caa735467e9ea7323804570b9610fee3f92d41a Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sat, 4 Jun 2022 01:42:17 +0530 Subject: [PATCH 3/7] Add CHANGELOG entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5807688a184..82f0ab8e1bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2727](https://github.com/open-telemetry/opentelemetry-python/pull/2727)) - fix: update entry point object references for metrics ([#2731](https://github.com/open-telemetry/opentelemetry-python/pull/2731)) +- Allow set_status to accept the StatusCode and optional description + ([#2735](https://github.com/open-telemetry/opentelemetry-python/pull/2735)) ## [1.12.0rc1-0.31b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc1-0.31b0) - 2022-05-17 From 286a59d7bc6e20d4781b6d5196baf15273512d1f Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 8 Jun 2022 00:50:26 +0530 Subject: [PATCH 4/7] Add warning message when both status and desc passed --- .../src/opentelemetry/sdk/trace/__init__.py | 4 ++++ opentelemetry-sdk/tests/trace/test_trace.py | 9 +++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 1e2b7179f52..e8a9fe2e623 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -903,6 +903,10 @@ def set_status( or status.status_code is StatusCode.UNSET ): return + if description is not None: + logger.warning( + f"Description {description} ignored. Use either `Status` or `(StatusCode, Description)`" + ) self._status = status elif isinstance(status, StatusCode): if ( diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 403b69cbf25..b89734db2f2 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -928,10 +928,11 @@ def test_span_set_status(self): self.assertEqual(span4.status.description, "span4 desc") span5 = self.tracer.start_span("span5") - span5.set_status( - Status(status_code=StatusCode.ERROR, description="desc"), - description="ignored", - ) + with self.assertLogs(level=WARNING): + span5.set_status( + Status(status_code=StatusCode.ERROR, description="desc"), + description="ignored", + ) self.assertEqual(span5.status.status_code, StatusCode.ERROR) self.assertEqual(span5.status.description, "desc") From 215db89ec399dd69e3a42d2e51e16dc900fc29f4 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 8 Jun 2022 01:09:22 +0530 Subject: [PATCH 5/7] Fix lint --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index e8a9fe2e623..7dc65600f4c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -905,7 +905,8 @@ def set_status( return if description is not None: logger.warning( - f"Description {description} ignored. Use either `Status` or `(StatusCode, Description)`" + "Description %s ignored. Use either `Status` or `(StatusCode, Description)`", + description, ) self._status = status elif isinstance(status, StatusCode): From 46e99db3eddd926c277156f47287068211308fcc Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sat, 11 Jun 2022 23:57:03 +0530 Subject: [PATCH 6/7] fix lint --- opentelemetry-api/tests/trace/test_globals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/tests/trace/test_globals.py b/opentelemetry-api/tests/trace/test_globals.py index 0ead559f862..5cb05828bb7 100644 --- a/opentelemetry-api/tests/trace/test_globals.py +++ b/opentelemetry-api/tests/trace/test_globals.py @@ -12,7 +12,7 @@ class TestSpan(trace.NonRecordingSpan): recorded_exception = None recorded_status = Status(status_code=StatusCode.UNSET) - def set_status(self, status): + def set_status(self, status, description): self.recorded_status = status def end(self, end_time=None): From d03c06374bc132c9b756c6256eb1d25c4ad3a06c Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 12 Jun 2022 00:01:32 +0530 Subject: [PATCH 7/7] Make arg optional --- opentelemetry-api/tests/trace/test_globals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/tests/trace/test_globals.py b/opentelemetry-api/tests/trace/test_globals.py index 5cb05828bb7..a448437e984 100644 --- a/opentelemetry-api/tests/trace/test_globals.py +++ b/opentelemetry-api/tests/trace/test_globals.py @@ -12,7 +12,7 @@ class TestSpan(trace.NonRecordingSpan): recorded_exception = None recorded_status = Status(status_code=StatusCode.UNSET) - def set_status(self, status, description): + def set_status(self, status, description=None): self.recorded_status = status def end(self, end_time=None):