diff --git a/CHANGELOG.md b/CHANGELOG.md index de7868b5009..6bb68e05313 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`. + ([#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 66b4b383905..bbb56e4b6e7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -806,6 +806,14 @@ 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") )