From bc87c88c6061bc231ad5dc4cba3a37ac21dae8c2 Mon Sep 17 00:00:00 2001 From: waiho-gumloop Date: Tue, 24 Mar 2026 19:09:43 -0700 Subject: [PATCH] fix: remove redundant MetricsCapture from trace_call trace_call() wraps every Spanner operation with a bare MetricsCapture() that creates a MetricsTracer without project_id or instance_id. Since every caller of trace_call already provides its own MetricsCapture with resource_info, this inner one is redundant. The redundant tracer records operation metrics with incomplete resource labels on every operation. Because OpenTelemetry uses cumulative aggregation, these orphan data points persist for the process lifetime and get re-exported every 60 seconds. Cloud Monitoring rejects them with INVALID_ARGUMENT (missing instance_id), producing repeated error logs. Removing the bare MetricsCapture from trace_call eliminates the orphan metric data points entirely. Callers continue to provide their own MetricsCapture(resource_info) with correct labels. Fixes: https://github.com/googleapis/python-spanner/issues/1319 --- .../spanner_v1/_opentelemetry_tracing.py | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/google/cloud/spanner_v1/_opentelemetry_tracing.py b/google/cloud/spanner_v1/_opentelemetry_tracing.py index 3513d524c6..1f9189a32d 100644 --- a/google/cloud/spanner_v1/_opentelemetry_tracing.py +++ b/google/cloud/spanner_v1/_opentelemetry_tracing.py @@ -30,7 +30,6 @@ _metadata_with_span_context, ) from google.cloud.spanner_v1.gapic_version import __version__ as gapic_version -from google.cloud.spanner_v1.metrics.metrics_capture import MetricsCapture from google.cloud.spanner_v1.services.spanner.client import SpannerClient TRACER_NAME = "cloud.google.com/python/spanner" @@ -129,29 +128,28 @@ def trace_call( with tracer.start_as_current_span( name, kind=trace.SpanKind.CLIENT, attributes=attributes ) as span: - with MetricsCapture(): - try: - if enable_end_to_end_tracing: - _metadata_with_span_context(metadata) - yield span - except Exception as error: - span.set_status(Status(StatusCode.ERROR, str(error))) - # OpenTelemetry-Python imposes invoking span.record_exception on __exit__ - # on any exception. We should file a bug later on with them to only - # invoke .record_exception if not already invoked, hence we should not - # invoke .record_exception on our own else we shall have 2 exceptions. - raise - else: - # All spans still have set_status available even if for example - # NonRecordingSpan doesn't have "_status". - absent_span_status = getattr(span, "_status", None) is None - if absent_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)) + try: + if enable_end_to_end_tracing: + _metadata_with_span_context(metadata) + yield span + except Exception as error: + span.set_status(Status(StatusCode.ERROR, str(error))) + # OpenTelemetry-Python imposes invoking span.record_exception on __exit__ + # on any exception. We should file a bug later on with them to only + # invoke .record_exception if not already invoked, hence we should not + # invoke .record_exception on our own else we shall have 2 exceptions. + raise + else: + # All spans still have set_status available even if for example + # NonRecordingSpan doesn't have "_status". + absent_span_status = getattr(span, "_status", None) is None + if absent_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)) def get_current_span():