Span.set_status priority (open-telemetry#1902)

Author: lzchen

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
 

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__(

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")
         )