Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

Commit 36e9f70

Browse files
sinhasubhamsinhasubham
committed
fix(metrics): prevent thread leak by ensuring singleton initialization
1 parent 2c5eb96 commit 36e9f70

File tree

3 files changed

+116
-33
lines changed

3 files changed

+116
-33
lines changed

‎google/cloud/spanner_v1/client.py‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ def _get_spanner_optimizer_statistics_package():
9999

100100
log = logging.getLogger(__name__)
101101

102+
_metrics_monitor_initialized = False
103+
102104

103105
def _get_spanner_enable_builtin_metrics_env():
104106
return os.getenv(SPANNER_DISABLE_BUILTIN_METRICS_ENV_VAR) != "true"
@@ -252,30 +254,33 @@ def __init__(
252254
):
253255
warnings.warn(_EMULATOR_HOST_HTTP_SCHEME)
254256
# Check flag to enable Spanner builtin metrics
257+
global _metrics_monitor_initialized
255258
if (
256259
_get_spanner_enable_builtin_metrics_env()
257260
and not disable_builtin_metrics
258261
and HAS_GOOGLE_CLOUD_MONITORING_INSTALLED
259262
):
260-
meter_provider = metrics.NoOpMeterProvider()
261-
try:
262-
if not _get_spanner_emulator_host():
263-
meter_provider = MeterProvider(
264-
metric_readers=[
265-
PeriodicExportingMetricReader(
266-
CloudMonitoringMetricsExporter(
267-
project_id=project, credentials=credentials
263+
if not _metrics_monitor_initialized:
264+
meter_provider = metrics.NoOpMeterProvider()
265+
try:
266+
if not _get_spanner_emulator_host():
267+
meter_provider = MeterProvider(
268+
metric_readers=[
269+
PeriodicExportingMetricReader(
270+
CloudMonitoringMetricsExporter(
271+
project_id=project, credentials=credentials
272+
),
273+
export_interval_millis=METRIC_EXPORT_INTERVAL_MS,
268274
),
269-
export_interval_millis=METRIC_EXPORT_INTERVAL_MS,
270-
),
271-
]
275+
]
276+
)
277+
metrics.set_meter_provider(meter_provider)
278+
SpannerMetricsTracerFactory()
279+
_metrics_monitor_initialized = True
280+
except Exception as e:
281+
log.warning(
282+
"Failed to initialize Spanner built-in metrics. Error: %s", e
272283
)
273-
metrics.set_meter_provider(meter_provider)
274-
SpannerMetricsTracerFactory()
275-
except Exception as e:
276-
log.warning(
277-
"Failed to initialize Spanner built-in metrics. Error: %s", e
278-
)
279284
else:
280285
SpannerMetricsTracerFactory(enabled=False)
281286

‎tests/unit/test_client.py‎

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,28 +255,44 @@ def test_constructor_w_directed_read_options(self):
255255
expected_scopes, creds, directed_read_options=self.DIRECTED_READ_OPTIONS
256256
)
257257

258+
@mock.patch("google.cloud.spanner_v1.client.metrics")
259+
@mock.patch("google.cloud.spanner_v1.client.CloudMonitoringMetricsExporter")
260+
@mock.patch("google.cloud.spanner_v1.client.PeriodicExportingMetricReader")
261+
@mock.patch("google.cloud.spanner_v1.client.MeterProvider")
258262
@mock.patch("google.cloud.spanner_v1.client.SpannerMetricsTracerFactory")
259263
@mock.patch.dict(os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "false"})
260264
def test_constructor_w_metrics_initialization_error(
261-
self, mock_spanner_metrics_factory
265+
self,
266+
mock_spanner_metrics_factory,
267+
mock_meter_provider,
268+
mock_periodic_reader,
269+
mock_exporter,
270+
mock_metrics,
262271
):
263272
"""
264273
Test that Client constructor handles exceptions during metrics
265274
initialization and logs a warning.
266275
"""
267276
from google.cloud.spanner_v1.client import Client
277+
from google.cloud.spanner_v1 import client as MUT
268278

279+
MUT._metrics_monitor_initialized = False
269280
mock_spanner_metrics_factory.side_effect = Exception("Metrics init failed")
270281
creds = build_scoped_credentials()
271-
272-
with self.assertLogs("google.cloud.spanner_v1.client", level="WARNING") as log:
273-
client = Client(project=self.PROJECT, credentials=creds)
274-
self.assertIsNotNone(client)
275-
self.assertIn(
276-
"Failed to initialize Spanner built-in metrics. Error: Metrics init failed",
277-
log.output[0],
278-
)
279-
mock_spanner_metrics_factory.assert_called_once()
282+
try:
283+
with self.assertLogs(
284+
"google.cloud.spanner_v1.client", level="WARNING"
285+
) as log:
286+
client = Client(project=self.PROJECT, credentials=creds)
287+
self.assertIsNotNone(client)
288+
self.assertIn(
289+
"Failed to initialize Spanner built-in metrics. Error: Metrics init failed",
290+
log.output[0],
291+
)
292+
mock_spanner_metrics_factory.assert_called_once()
293+
mock_metrics.set_meter_provider.assert_called_once()
294+
finally:
295+
MUT._metrics_monitor_initialized = False
280296

281297
@mock.patch("google.cloud.spanner_v1.client.SpannerMetricsTracerFactory")
282298
@mock.patch.dict(os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "true"})
@@ -293,6 +309,61 @@ def test_constructor_w_disable_builtin_metrics_using_env(
293309
self.assertIsNotNone(client)
294310
mock_spanner_metrics_factory.assert_called_once_with(enabled=False)
295311

312+
self.assertIsNotNone(client)
313+
mock_spanner_metrics_factory.assert_called_once_with(enabled=False)
314+
315+
@mock.patch("google.cloud.spanner_v1.client.metrics")
316+
@mock.patch("google.cloud.spanner_v1.client.CloudMonitoringMetricsExporter")
317+
@mock.patch("google.cloud.spanner_v1.client.PeriodicExportingMetricReader")
318+
@mock.patch("google.cloud.spanner_v1.client.MeterProvider")
319+
@mock.patch("google.cloud.spanner_v1.client.SpannerMetricsTracerFactory")
320+
@mock.patch.dict(os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "false"})
321+
def test_constructor_metrics_singleton_behavior(
322+
self,
323+
mock_spanner_metrics_factory,
324+
mock_meter_provider,
325+
mock_periodic_reader,
326+
mock_exporter,
327+
mock_metrics,
328+
):
329+
"""
330+
Test that metrics are only initialized once.
331+
"""
332+
from google.cloud.spanner_v1 import client as MUT
333+
334+
# Reset global state for this test
335+
MUT._metrics_monitor_initialized = False
336+
try:
337+
creds = build_scoped_credentials()
338+
339+
# First client initialization
340+
client1 = MUT.Client(project=self.PROJECT, credentials=creds)
341+
self.assertIsNotNone(client1)
342+
mock_metrics.set_meter_provider.assert_called_once()
343+
mock_spanner_metrics_factory.assert_called_once()
344+
345+
# Verify MeterProvider chain was created
346+
mock_meter_provider.assert_called_once()
347+
mock_periodic_reader.assert_called_once()
348+
mock_exporter.assert_called_once()
349+
350+
self.assertTrue(MUT._metrics_monitor_initialized)
351+
352+
# Reset mocks to verify they are NOT called again
353+
mock_metrics.set_meter_provider.reset_mock()
354+
mock_spanner_metrics_factory.reset_mock()
355+
mock_meter_provider.reset_mock()
356+
357+
# Second client initialization
358+
client2 = MUT.Client(project=self.PROJECT, credentials=creds)
359+
self.assertIsNotNone(client2)
360+
mock_metrics.set_meter_provider.assert_not_called()
361+
mock_spanner_metrics_factory.assert_not_called()
362+
mock_meter_provider.assert_not_called()
363+
self.assertTrue(MUT._metrics_monitor_initialized)
364+
finally:
365+
MUT._metrics_monitor_initialized = False
366+
296367
@mock.patch("google.cloud.spanner_v1.client.SpannerMetricsTracerFactory")
297368
def test_constructor_w_disable_builtin_metrics_using_option(
298369
self, mock_spanner_metrics_factory

‎tests/unit/test_metrics.py‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,24 @@ def patched_client(monkeypatch):
6060
if SpannerMetricsTracerFactory._metrics_tracer_factory is not None:
6161
SpannerMetricsTracerFactory._metrics_tracer_factory = None
6262

63-
client = Client(
64-
project="test",
65-
credentials=TestCredentials(),
66-
# client_options={"api_endpoint": "none"}
67-
)
68-
yield client
63+
# Reset the global flag to ensure metrics initialization runs
64+
from google.cloud.spanner_v1 import client as client_module
65+
66+
client_module._metrics_monitor_initialized = False
67+
68+
with patch("google.cloud.spanner_v1.client.CloudMonitoringMetricsExporter"):
69+
client = Client(
70+
project="test",
71+
credentials=TestCredentials(),
72+
# client_options={"api_endpoint": "none"}
73+
)
74+
yield client
6975

7076
# Resetting
7177
metrics.set_meter_provider(metrics.NoOpMeterProvider())
7278
SpannerMetricsTracerFactory._metrics_tracer_factory = None
7379
SpannerMetricsTracerFactory.current_metrics_tracer = None
80+
client_module._metrics_monitor_initialized = False
7481

7582

7683
def test_metrics_emission_with_failure_attempt(patched_client):

0 commit comments

Comments
 (0)