From f312b3d8b59f428954169e90088c9d23d57262c0 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 9 Jun 2021 15:26:08 -0700 Subject: [PATCH 1/2] status --- CHANGELOG.md | 3 ++ .../src/opentelemetry/sdk/trace/__init__.py | 5 +++ opentelemetry-sdk/tests/trace/test_trace.py | 38 ++++++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de7868b5009..14597278954 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Updated `opentelemetry-opencensus-exporter` to use `service_name` of spans instead of resource ([#1897](https://github.com/open-telemetry/opentelemetry-python/pull/1897)) +- Ignore calls to `Span.set_status` with `StatusCode.UNSET` and also if previous status already + had `StatusCode.OK`. + ([#1897](https://github.com/open-telemetry/opentelemetry-python/pull/1897)) ## [1.3.0-0.22b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.3.0-0.22b0) - 2021-06-01 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 66b4b383905..651624d8296 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -806,6 +806,11 @@ def is_recording(self) -> bool: @_check_span_ended def set_status(self, status: trace_api.Status) -> 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 def __exit__( diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 0781042a337..dc5bbb3d69b 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -913,7 +913,6 @@ def error_status_test(context): with self.assertRaises(AssertionError): with context as root: raise AssertionError("unknown") - self.assertIs(root.status.status_code, StatusCode.ERROR) self.assertEqual( root.status.description, "AssertionError: unknown" @@ -928,18 +927,53 @@ def error_status_test(context): .start_as_current_span("root") ) - def test_last_status_wins(self): + def test_status_cannot_override_ok(self): def error_status_test(context): with self.assertRaises(AssertionError): with context as root: root.set_status(trace_api.status.Status(StatusCode.OK)) raise AssertionError("unknown") + self.assertIs(root.status.status_code, StatusCode.OK) + self.assertIsNone(root.status.description) + + error_status_test( + trace.TracerProvider().get_tracer(__name__).start_span("root") + ) + error_status_test( + trace.TracerProvider() + .get_tracer(__name__) + .start_as_current_span("root") + ) + def test_status_cannot_set_unset(self): + def unset_status_test(context): + with self.assertRaises(AssertionError): + with context as root: + raise AssertionError("unknown") + root.set_status(trace_api.status.Status(StatusCode.UNSET)) self.assertIs(root.status.status_code, StatusCode.ERROR) self.assertEqual( root.status.description, "AssertionError: unknown" ) + unset_status_test( + trace.TracerProvider().get_tracer(__name__).start_span("root") + ) + unset_status_test( + trace.TracerProvider() + .get_tracer(__name__) + .start_as_current_span("root") + ) + + def test_last_status_wins(self): + def error_status_test(context): + with self.assertRaises(AssertionError): + with context as root: + raise AssertionError("unknown") + root.set_status(trace_api.status.Status(StatusCode.OK)) + self.assertIs(root.status.status_code, StatusCode.OK) + self.assertIsNone(root.status.description) + error_status_test( trace.TracerProvider().get_tracer(__name__).start_span("root") ) From 5b5a79047a064756fc285279bf2236d7b84db173 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 9 Jun 2021 15:31:14 -0700 Subject: [PATCH 2/2] lint --- CHANGELOG.md | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14597278954..6bb68e05313 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1897](https://github.com/open-telemetry/opentelemetry-python/pull/1897)) - Ignore calls to `Span.set_status` with `StatusCode.UNSET` and also if previous status already had `StatusCode.OK`. - ([#1897](https://github.com/open-telemetry/opentelemetry-python/pull/1897)) + ([#1902](https://github.com/open-telemetry/opentelemetry-python/pull/1902)) ## [1.3.0-0.22b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.3.0-0.22b0) - 2021-06-01 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 651624d8296..bbb56e4b6e7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -808,8 +808,11 @@ def is_recording(self) -> bool: def set_status(self, status: trace_api.Status) -> 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: + if ( + self._status + and self._status.status_code is StatusCode.OK + or status.status_code is StatusCode.UNSET + ): return self._status = status