Commit c41fbb3
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 <57220027+harshachinta@users.noreply.github.com>1 parent da80152 commit c41fbb3
2 files changed
Lines changed: 44 additions & 1 deletion
File tree
- packages/google-cloud-spanner
- google/cloud/spanner_v1
- tests/unit
Lines changed: 7 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -109,4 +109,10 @@ def trace_call(name, session, extra_attributes=None, observability_options=None) | |||
| 109 | 109 | span.record_exception(error) | |
| 110 | 110 | raise | |
| 111 | 111 | else: | |
| 112 | - span.set_status(Status(StatusCode.OK)) | ||
| 112 | + if (not span._status) or span._status.status_code == StatusCode.UNSET: | ||
| 113 | + # OpenTelemetry-Python only allows a status change | ||
| 114 | + # if the current code is UNSET or ERROR. At the end | ||
| 115 | + # of the generator's consumption, only set it to OK | ||
| 116 | + # it wasn't previously set otherwise. | ||
| 117 | + # https://github.com/googleapis/python-spanner/issues/1246 | ||
| 118 | + span.set_status(Status(StatusCode.OK)) | ||
Lines changed: 37 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -158,3 +158,40 @@ def test_trace_codeless_error(self): | |||
| 158 | 158 | self.assertEqual(len(span_list), 1) | |
| 159 | 159 | span = span_list[0] | |
| 160 | 160 | self.assertEqual(span.status.status_code, StatusCode.ERROR) | |
| 161 | + | ||
| 162 | + def test_trace_call_terminal_span_status(self): | ||
| 163 | + # Verify that we don't unconditionally set the terminal span status to | ||
| 164 | + # SpanStatus.OK per https://github.com/googleapis/python-spanner/issues/1246 | ||
| 165 | + from opentelemetry.sdk.trace.export import SimpleSpanProcessor | ||
| 166 | + from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( | ||
| 167 | + InMemorySpanExporter, | ||
| 168 | + ) | ||
| 169 | + from opentelemetry.trace.status import Status, StatusCode | ||
| 170 | + from opentelemetry.sdk.trace import TracerProvider | ||
| 171 | + from opentelemetry.sdk.trace.sampling import ALWAYS_ON | ||
| 172 | + | ||
| 173 | + tracer_provider = TracerProvider(sampler=ALWAYS_ON) | ||
| 174 | + trace_exporter = InMemorySpanExporter() | ||
| 175 | + tracer_provider.add_span_processor(SimpleSpanProcessor(trace_exporter)) | ||
| 176 | + observability_options = dict(tracer_provider=tracer_provider) | ||
| 177 | + | ||
| 178 | + session = _make_session() | ||
| 179 | + with _opentelemetry_tracing.trace_call( | ||
| 180 | + "VerifyTerminalSpanStatus", | ||
| 181 | + session, | ||
| 182 | + observability_options=observability_options, | ||
| 183 | + ) as span: | ||
| 184 | + span.set_status(Status(StatusCode.ERROR, "Our error exhibit")) | ||
| 185 | + | ||
| 186 | + span_list = trace_exporter.get_finished_spans() | ||
| 187 | + got_statuses = [] | ||
| 188 | + | ||
| 189 | + for span in span_list: | ||
| 190 | + got_statuses.append( | ||
| 191 | + (span.name, span.status.status_code, span.status.description) | ||
| 192 | + ) | ||
| 193 | + | ||
| 194 | + want_statuses = [ | ||
| 195 | + ("VerifyTerminalSpanStatus", StatusCode.ERROR, "Our error exhibit"), | ||
| 196 | + ] | ||
| 197 | + assert got_statuses == want_statuses | ||
0 commit comments