fix(metrics): prevent thread leak by ensuring singleton initialization #1492

sinhasubham · 2026-02-04T10:34:24Z

Summary:
This PR fixes a critical memory and thread leak in the google-cloud-spanner client when built-in metrics are enabled (default behavior).
Previously, the Client constructor unconditionally initialized a new OpenTelemetry MeterProvider and PeriodicExportingMetricReader on every instantiation. Each reader spawned a new background thread for metric exporting that was never cleaned up or reused. In environments where Client objects are frequently created (e.g., Cloud Functions, web servers, or data pipelines), this caused a linear accumulation of threads, leading to RuntimeError: can't start new thread and OOM crashes.

Fix Implementation:
Refactored Metrics Initialization (Thread Safety & Memory Leak Fix):
Implemented a Singleton pattern for the OpenTelemetry MeterProvider using threading.Lock to prevent infinite background thread creation (memory leak).
Moved metrics initialization logic to a cleaner helper function _initialize_metrics in client.py.
Replaced global mutable state in SpannerMetricsTracerFactory with contextvars.ContextVar to ensure thread-safe, isolated metric tracing across concurrent requests.
Updated MetricsInterceptor and MetricsCapture to correctly use the thread-local tracer.
Fixed Batch.commit Idempotency (AlreadyExists Regression):
Modified Batch.commit to initialize nth_request and the attempt counter outside the retry loop.
This ensures that retries (e.g., on ABORTED) reuse the same Request ID, allowing Cloud Spanner to correctly deduplicate requests and preventing spurious AlreadyExists (409) errors.
Verification:
Added tests/unit/test_metrics_concurrency.py to verify tracer isolation and thread safety.
Cleaned up tests/unit/test_metrics.py and consolidated mocks in conftest.py.

gemini-code-assist · 2026-02-04T10:34:44Z

Summary of Changes

Hello @sinhasubham, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical issue in the google-cloud-spanner client where enabling built-in metrics caused a linear accumulation of background threads and memory, leading to RuntimeError and out-of-memory crashes in environments with frequent Client instantiations. The fix ensures that the OpenTelemetry metrics components are initialized using a singleton pattern, guaranteeing they are set up only once per process. This change significantly improves resource management and stability for applications utilizing the Spanner client with metrics enabled.

Highlights

Thread and Memory Leak Fix: Implemented a singleton pattern for OpenTelemetry metrics initialization to prevent the creation of multiple PeriodicExportingMetricReader instances, which previously led to an accumulation of background threads and potential memory leaks.
Singleton Implementation: Introduced a global boolean flag _metrics_monitor_initialized in client.py to ensure that the metrics subsystem is initialized only once per process, regardless of how many Client objects are instantiated.
Enhanced Test Coverage: Added a new unit test (test_constructor_metrics_singleton_behavior) to explicitly verify that the metrics components are initialized only once. Existing tests were also updated to properly manage the global state of the metrics initialization flag.
Robust Error Handling: Ensured that exceptions during metrics initialization are caught and logged as warnings, preventing crashes while still informing developers of potential issues.

Changelog

google/cloud/spanner_v1/client.py
- Added a global boolean variable _metrics_monitor_initialized to track the initialization status of the metrics monitor.
- Wrapped the OpenTelemetry MeterProvider and PeriodicExportingMetricReader initialization logic within a conditional block, executing only if _metrics_monitor_initialized is False.
- Set _metrics_monitor_initialized to True after successful metrics setup to prevent re-initialization.
- Declared _metrics_monitor_initialized as global within the Client.__init__ method to allow modification.
tests/unit/test_client.py
- Introduced a new test case test_constructor_metrics_singleton_behavior to verify that metrics initialization occurs exactly once across multiple Client instantiations.
- Modified test_constructor_w_metrics_initialization_error to include additional mocks for metrics-related components and to reset the global _metrics_monitor_initialized flag for proper test isolation.
- Ensured that mock_metrics.set_meter_provider is asserted as called once in the error handling test.
tests/unit/test_metrics.py
- Updated the patched_client fixture to explicitly reset the client_module._metrics_monitor_initialized flag to False before and after tests. This ensures a clean state for metrics initialization in each test run.
- Added a patch for CloudMonitoringMetricsExporter within the patched_client fixture to prevent actual external calls during tests.

Activity

The author sinhasubham initiated this pull request to address a critical thread and memory leak in the Spanner client's metrics subsystem.
The core changes involve modifying the client.py file to implement a singleton pattern for metrics initialization.
New unit tests were added in test_client.py to validate the correct singleton behavior of the metrics setup.
Existing test fixtures in test_metrics.py were updated to ensure proper isolation and state management for tests involving metrics initialization.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses a critical thread and memory leak by ensuring the metrics subsystem is initialized only once. The approach uses a global flag to track initialization, which is a good start. However, the current implementation is not thread-safe and could still lead to multiple initializations under concurrent Client instantiations. I've provided a suggestion to add a threading.Lock to make the initialization truly a singleton. Additionally, I found a minor code duplication in one of the tests.

google/cloud/spanner_v1/client.py

tests/unit/test_client.py

olavloite · 2026-02-09T08:04:37Z

google/cloud/spanner_v1/client.py

+            if not _metrics_monitor_initialized:
+                with _metrics_monitor_lock:
+                    if not _metrics_monitor_initialized:
+                        meter_provider = metrics.NoOpMeterProvider()
+                        try:
+                            if not _get_spanner_emulator_host():
+                                meter_provider = MeterProvider(
+                                    metric_readers=[
+                                        PeriodicExportingMetricReader(
+                                            CloudMonitoringMetricsExporter(
+                                                project_id=project,
+                                                credentials=credentials,
+                                            ),
+                                            export_interval_millis=METRIC_EXPORT_INTERVAL_MS,
+                                        ),
+                                    ]
+                                )
+                            metrics.set_meter_provider(meter_provider)
+                            SpannerMetricsTracerFactory()
+                            _metrics_monitor_initialized = True
+                        except Exception as e:
+                            log.warning(
+                                "Failed to initialize Spanner built-in metrics. Error: %s",
+                                e,
+                            )


nit: could this potentially be moved to a separate function to keep the init function a bit shorter/cleaner?

olavloite · 2026-02-09T08:05:24Z

tests/unit/test_metrics.py

+        client = Client(
+            project="test",
+            credentials=TestCredentials(),
+            # client_options={"api_endpoint": "none"}


nit: remove

olavloite · 2026-02-10T12:26:26Z

/gemini review

gemini-code-assist

Code Review

This pull request provides a crucial fix for a thread and memory leak issue caused by repeated initialization of OpenTelemetry providers. The implementation correctly uses a singleton pattern with double-checked locking for the metrics provider initialization, ensuring it only runs once. The switch from a global mutable tracer to a thread-safe contextvars.ContextVar is well-executed and effectively isolates metric tracing across concurrent operations, as demonstrated by the new concurrency tests. The accompanying fix for batch commit idempotency is also a valuable improvement. The test suite has been significantly enhanced with new concurrency tests and refactoring of existing ones, providing strong confidence in the correctness of these changes. I have one suggestion for improving the API clarity in the SpannerMetricsTracerFactory. Overall, this is an excellent and well-tested contribution that addresses a critical issue.

google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py

olavloite

Looks overall good to me, with a few small nits/questions.

olavloite · 2026-02-10T14:26:40Z

google/cloud/spanner_v1/metrics/metrics_capture.py

 from .spanner_metrics_tracer_factory import SpannerMetricsTracerFactory


+from contextvars import Token


nit: group this with the other import

google/cloud/spanner_v1/metrics/metrics_capture.py

olavloite · 2026-02-10T14:42:53Z

google/cloud/spanner_v1/metrics/metrics_interceptor.py

Would it make sense to check whether the resources have already been set for this tracer, and if so, skip the extraction and setting them again? (Is there any possibility that they would ever change during the lifetime of a tracer, and if that even is possible, is it something that we would want?)

makes sense. Added a check to skip extraction if tracer.client_attributes are already populated.

olavloite · 2026-02-10T14:46:36Z

google/cloud/spanner_v1/batch.py

-                    getattr(database, "_next_nth_request", 0),
-                    1,
+                    nth_request,
+                    attempt.increment(),


I think that the existing code was correct. There are two different things that can be retried in Spanner:

Aborted transactions: When a read/write transaction is aborted, then the entire transaction is retried. This should not cause attempt to be increased, even in this case, where the entire transaction is just a single Commit call.

Unavailable: A single RPC can fail due to network errors, server temporarily being down etc. This is normally retried by Gax. In this case, only a single RPC (so not the entire transaction) is retried. It is only in these cases that attempt should be increased.

Thanks for the clarification. I've moved attempt back inside wrapped_method so it resets on Aborted retries, but kept nth_request outside to ensure the logical request ID remains stable.

Hmm.... I think that there is still a misunderstanding here. The original code was (AFAICT) correct, meaning that there should be no changes here. Could you otherwise explain why this is being changed here / what issue is being fixed?

Put another way: In my opinion, the logical request ID should not remain the same during these retries. These requests should be considered new requests, as the reason that it is being retried, is that the transaction was aborted. Transactions that are being retried due to the original attempt being aborted by Spanner should be considered new requests.

in that case, previous logic makes sense. I was under impression request id should be preserved in abort cases. Reverted to old logic.

olavloite · 2026-02-10T14:50:03Z

google/cloud/spanner_v1/client.py

        ):
            warnings.warn(_EMULATOR_HOST_HTTP_SCHEME)
        # Check flag to enable Spanner builtin metrics
+        global _metrics_monitor_initialized


Is this still needed here?

olavloite · 2026-02-10T14:50:14Z

tests/unit/conftest.py

@@ -0,0 +1,13 @@
+import pytest


nit: add copyright header

olavloite · 2026-02-10T14:51:16Z

tests/unit/test_metrics_concurrency.py

@@ -0,0 +1,80 @@
+import threading


nit: add copyright header

google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py

sinhasubham requested review from a team as code owners February 4, 2026 10:34

blunderbuss-gcf bot assigned bhatt4982 Feb 4, 2026

product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/python-spanner API. labels Feb 4, 2026

gemini-code-assist bot reviewed Feb 4, 2026

View reviewed changes

google/cloud/spanner_v1/client.py Show resolved Hide resolved

google/cloud/spanner_v1/client.py Outdated Show resolved Hide resolved

tests/unit/test_client.py Outdated Show resolved Hide resolved

sinhasubham force-pushed the fix/oom-client-metrics branch 2 times, most recently from 1ef3da1 to 67c682e Compare February 5, 2026 09:22

olavloite approved these changes Feb 9, 2026

View reviewed changes

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Feb 10, 2026

sinhasubham force-pushed the fix/oom-client-metrics branch from 519755b to 1341f21 Compare February 10, 2026 07:08

gemini-code-assist bot reviewed Feb 10, 2026

View reviewed changes

google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py Outdated Show resolved Hide resolved

olavloite self-requested a review February 10, 2026 14:26

olavloite reviewed Feb 10, 2026

View reviewed changes

sinhasubham force-pushed the fix/oom-client-metrics branch 7 times, most recently from cdf3d0e to 6c01381 Compare February 11, 2026 10:46

sinhasubham added 2 commits February 11, 2026 16:20

fix(metrics): prevent thread leak by ensuring singleton initialization

f00f594

fix: Metrics thread-safety refactor and Batch.commit idempotency fix

390ef1d

sinhasubham force-pushed the fix/oom-client-metrics branch from 6c01381 to 390ef1d Compare February 11, 2026 10:51

olavloite approved these changes Feb 11, 2026

View reviewed changes

sinhasubham merged commit e792136 into main Feb 11, 2026
23 of 24 checks passed

sinhasubham deleted the fix/oom-client-metrics branch February 11, 2026 16:57

		from .spanner_metrics_tracer_factory import SpannerMetricsTracerFactory


		from contextvars import Token

fix(metrics): prevent thread leak by ensuring singleton initialization #1492

fix(metrics): prevent thread leak by ensuring singleton initialization #1492

Uh oh!

Conversation

sinhasubham commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Feb 4, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olavloite commented Feb 10, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

olavloite left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sinhasubham commented Feb 4, 2026 •

edited

Loading