fix: remove redundant MetricsCapture from trace_call#16495
Open
waiho-gumloop wants to merge 1 commit intogoogleapis:mainfrom
Open
fix: remove redundant MetricsCapture from trace_call#16495waiho-gumloop wants to merge 1 commit intogoogleapis:mainfrom
waiho-gumloop wants to merge 1 commit intogoogleapis:mainfrom
Conversation
The bare MetricsCapture() inside trace_call creates a MetricsTracer without project_id or instance_id. Since every caller already provides its own MetricsCapture(resource_info) with correct labels, the one inside trace_call is redundant and records operation metrics with incomplete resource labels on every operation. Fixes googleapis#16173
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
trace_call()wraps every Spanner operation with a bareMetricsCapture()that creates aMetricsTracerwithoutproject_idorinstance_id. Since every caller oftrace_callalready provides its ownMetricsCapture(resource_info)with correct labels, the one insidetrace_callis redundant and harmful.The redundant tracer records operation metrics (via
record_operation_completion()) 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 by thePeriodicExportingMetricReader. Cloud Monitoring rejects them with:Root cause
When Python evaluates
with trace_call(...) as span, MetricsCapture(resource_info):, the execution order is:trace_call.__enter__()creates internal bareMetricsCapture()-> tracer_A (noproject_id/instance_id)MetricsCapture(resource_info).__enter__()-> tracer_B (has correct labels, overwrites tracer_A in context var)MetricsInterceptorMetricsCapture.__exit__()-> records correct metrics on tracer_B, resets context to tracer_Atrace_call.__exit__()-> records metrics on tracer_A with incomplete labelsHistory
MetricsCapture()everywhere.MetricsCapture(self._resource_info)but did not remove the bare one insidetrace_call.Fix
Remove the bare
MetricsCapture()fromtrace_call. All ~27 call sites already provide their ownMetricsCapturewith correct resource labels.Testing
All existing unit tests pass. The change only removes the redundant context manager; span/trace behavior is unchanged.
Fixes #16173