fix(tracing): only set span.status=OK if UNSET (#1248)

In modernized OpenTelemetry-Python, if the SpanStatus
was not already set to OK, it can be changed and
the code for trace_call was accidentally unconditionally
setting the status to OK if there was no exception.
This change fixes that and adds tests to lock this behavior in.

Fixes #1246

Co-authored-by: Sri Harsha CH <[email protected]>

Author: odeke-em

google/cloud/spanner_v1/_opentelemetry_tracing.py

@@ -109,4 +109,10 @@ def trace_call(name, session, extra_attributes=None, observability_options=None)
             span.record_exception(error)
             raise
         else:
-            span.set_status(Status(StatusCode.OK))
+            if (not span._status) or span._status.status_code == StatusCode.UNSET:
+                # OpenTelemetry-Python only allows a status change
+                # if the current code is UNSET or ERROR. At the end
+                # of the generator's consumption, only set it to OK
+                # it wasn't previously set otherwise.
+                # https://github.com/googleapis/python-spanner/issues/1246
+                span.set_status(Status(StatusCode.OK))

tests/unit/test__opentelemetry_tracing.py

@@ -158,3 +158,40 @@ def test_trace_codeless_error(self):
             self.assertEqual(len(span_list), 1)
             span = span_list[0]
             self.assertEqual(span.status.status_code, StatusCode.ERROR)
+
+        def test_trace_call_terminal_span_status(self):
+            # Verify that we don't unconditionally set the terminal span status to
+            # SpanStatus.OK per https://github.com/googleapis/python-spanner/issues/1246
+            from opentelemetry.sdk.trace.export import SimpleSpanProcessor
+            from opentelemetry.sdk.trace.export.in_memory_span_exporter import (
+                InMemorySpanExporter,
+            )
+            from opentelemetry.trace.status import Status, StatusCode
+            from opentelemetry.sdk.trace import TracerProvider
+            from opentelemetry.sdk.trace.sampling import ALWAYS_ON
+
+            tracer_provider = TracerProvider(sampler=ALWAYS_ON)
+            trace_exporter = InMemorySpanExporter()
+            tracer_provider.add_span_processor(SimpleSpanProcessor(trace_exporter))
+            observability_options = dict(tracer_provider=tracer_provider)
+
+            session = _make_session()
+            with _opentelemetry_tracing.trace_call(
+                "VerifyTerminalSpanStatus",
+                session,
+                observability_options=observability_options,
+            ) as span:
+                span.set_status(Status(StatusCode.ERROR, "Our error exhibit"))
+
+            span_list = trace_exporter.get_finished_spans()
+            got_statuses = []
+
+            for span in span_list:
+                got_statuses.append(
+                    (span.name, span.status.status_code, span.status.description)
+                )
+
+            want_statuses = [
+                ("VerifyTerminalSpanStatus", StatusCode.ERROR, "Our error exhibit"),
+            ]
+            assert got_statuses == want_statuses