From 73e7a9989db2f717230377c52e0fab98165b622b Mon Sep 17 00:00:00 2001 From: Subham Sinha Date: Fri, 17 Oct 2025 13:20:10 +0530 Subject: [PATCH 1/2] fix(spanner): resolve TypeError in metrics resource detection --- .../cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py b/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py index fd00c4de9c..af52d852c6 100644 --- a/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py +++ b/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py @@ -169,4 +169,4 @@ def _get_location() -> str: if GOOGLE_CLOUD_REGION_KEY not in resources.attributes: return GOOGLE_CLOUD_REGION_GLOBAL else: - return resources[GOOGLE_CLOUD_REGION_KEY] + return resources.attributes[GOOGLE_CLOUD_REGION_KEY] From ebb4883e72e0eb1021a0a8b195ee43a3fb9b9d59 Mon Sep 17 00:00:00 2001 From: Subham Sinha Date: Fri, 17 Oct 2025 17:33:47 +0530 Subject: [PATCH 2/2] fix(spanner): add exception handling for metrics initialization --- google/cloud/spanner_v1/client.py | 31 +++++++----- .../metrics/spanner_metrics_tracer_factory.py | 28 +++++++---- tests/unit/test_client.py | 23 +++++++++ .../test_spanner_metrics_tracer_factory.py | 47 +++++++++++++++++++ 4 files changed, 108 insertions(+), 21 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index e0e8c44058..6ebabbb34e 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -25,6 +25,7 @@ """ import grpc import os +import logging import warnings from google.api_core.gapic_v1 import client_info @@ -97,6 +98,9 @@ def _get_spanner_optimizer_statistics_package(): return os.getenv(OPTIMIZER_STATISITCS_PACKAGE_ENV_VAR, "") +log = logging.getLogger(__name__) + + def _get_spanner_enable_builtin_metrics(): return os.getenv(ENABLE_SPANNER_METRICS_ENV_VAR) == "true" @@ -240,19 +244,24 @@ def __init__( and HAS_GOOGLE_CLOUD_MONITORING_INSTALLED ): meter_provider = metrics.NoOpMeterProvider() - if not _get_spanner_emulator_host(): - meter_provider = MeterProvider( - metric_readers=[ - PeriodicExportingMetricReader( - CloudMonitoringMetricsExporter( - project_id=project, credentials=credentials + 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, ), - export_interval_millis=METRIC_EXPORT_INTERVAL_MS, - ) - ] + ] + ) + metrics.set_meter_provider(meter_provider) + SpannerMetricsTracerFactory() + except Exception as e: + log.warning( + "Failed to initialize Spanner built-in metrics. Error: %s", e ) - metrics.set_meter_provider(meter_provider) - SpannerMetricsTracerFactory() else: SpannerMetricsTracerFactory(enabled=False) diff --git a/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py b/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py index af52d852c6..881a5bfca9 100644 --- a/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py +++ b/google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py @@ -17,6 +17,7 @@ from .metrics_tracer_factory import MetricsTracerFactory import os +import logging from .constants import ( SPANNER_SERVICE_NAME, GOOGLE_CLOUD_REGION_KEY, @@ -33,9 +34,6 @@ import mmh3 - # Override Resource detector logging to not warn when GCP resources are not detected - import logging - logging.getLogger("opentelemetry.resourcedetector.gcp_resource_detector").setLevel( logging.ERROR ) @@ -48,6 +46,8 @@ from google.cloud.spanner_v1 import __version__ from uuid import uuid4 +log = logging.getLogger(__name__) + class SpannerMetricsTracerFactory(MetricsTracerFactory): """A factory for creating SpannerMetricsTracer instances.""" @@ -158,15 +158,23 @@ def _generate_client_hash(client_uid: str) -> str: def _get_location() -> str: """Get the location of the resource. + In case of any error during detection, this method will log a warning + and default to the "global" location. + Returns: str: The location of the resource. If OpenTelemetry is not installed, returns a global region. """ if not HAS_OPENTELEMETRY_INSTALLED: return GOOGLE_CLOUD_REGION_GLOBAL - detector = gcp_resource_detector.GoogleCloudResourceDetector() - resources = detector.detect() - - if GOOGLE_CLOUD_REGION_KEY not in resources.attributes: - return GOOGLE_CLOUD_REGION_GLOBAL - else: - return resources.attributes[GOOGLE_CLOUD_REGION_KEY] + try: + detector = gcp_resource_detector.GoogleCloudResourceDetector() + resources = detector.detect() + + if GOOGLE_CLOUD_REGION_KEY in resources.attributes: + return resources.attributes[GOOGLE_CLOUD_REGION_KEY] + except Exception as e: + log.warning( + "Failed to detect GCP resource location for Spanner metrics, defaulting to 'global'. Error: %s", + e, + ) + return GOOGLE_CLOUD_REGION_GLOBAL diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 212dc9ee4f..f0d246673a 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -255,6 +255,29 @@ def test_constructor_w_directed_read_options(self): expected_scopes, creds, directed_read_options=self.DIRECTED_READ_OPTIONS ) + @mock.patch.dict(os.environ, {"SPANNER_ENABLE_BUILTIN_METRICS": "true"}) + @mock.patch("google.cloud.spanner_v1.client.SpannerMetricsTracerFactory") + def test_constructor_w_metrics_initialization_error( + self, mock_spanner_metrics_factory + ): + """ + Test that Client constructor handles exceptions during metrics + initialization and logs a warning. + """ + from google.cloud.spanner_v1.client import Client + + mock_spanner_metrics_factory.side_effect = Exception("Metrics init failed") + creds = build_scoped_credentials() + + with self.assertLogs("google.cloud.spanner_v1.client", level="WARNING") as log: + client = Client(project=self.PROJECT, credentials=creds) + self.assertIsNotNone(client) + self.assertIn( + "Failed to initialize Spanner built-in metrics. Error: Metrics init failed", + log.output[0], + ) + mock_spanner_metrics_factory.assert_called_once() + def test_constructor_route_to_leader_disbled(self): from google.cloud.spanner_v1 import client as MUT diff --git a/tests/unit/test_spanner_metrics_tracer_factory.py b/tests/unit/test_spanner_metrics_tracer_factory.py index 8ee4d53d3d..48fe1b4837 100644 --- a/tests/unit/test_spanner_metrics_tracer_factory.py +++ b/tests/unit/test_spanner_metrics_tracer_factory.py @@ -13,9 +13,17 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest +import unittest +from unittest import mock + +from google.cloud.spanner_v1.metrics.constants import GOOGLE_CLOUD_REGION_KEY from google.cloud.spanner_v1.metrics.spanner_metrics_tracer_factory import ( SpannerMetricsTracerFactory, ) +from opentelemetry.sdk.resources import Resource + +pytest.importorskip("opentelemetry") class TestSpannerMetricsTracerFactory: @@ -48,3 +56,42 @@ def test_get_location(self): location = SpannerMetricsTracerFactory._get_location() assert isinstance(location, str) assert location # Simply asserting for non empty as this can change depending on the instance this test runs in. + + +class TestSpannerMetricsTracerFactoryGetLocation(unittest.TestCase): + @mock.patch( + "opentelemetry.resourcedetector.gcp_resource_detector.GoogleCloudResourceDetector.detect" + ) + def test_get_location_with_region(self, mock_detect): + """Test that _get_location returns the region when detected.""" + mock_resource = Resource.create({GOOGLE_CLOUD_REGION_KEY: "us-central1"}) + mock_detect.return_value = mock_resource + + location = SpannerMetricsTracerFactory._get_location() + assert location == "us-central1" + + @mock.patch( + "opentelemetry.resourcedetector.gcp_resource_detector.GoogleCloudResourceDetector.detect" + ) + def test_get_location_without_region(self, mock_detect): + """Test that _get_location returns 'global' when no region is detected.""" + mock_resource = Resource.create({}) # No region attribute + mock_detect.return_value = mock_resource + + location = SpannerMetricsTracerFactory._get_location() + assert location == "global" + + @mock.patch( + "opentelemetry.resourcedetector.gcp_resource_detector.GoogleCloudResourceDetector.detect" + ) + def test_get_location_with_exception(self, mock_detect): + """Test that _get_location returns 'global' and logs a warning on exception.""" + mock_detect.side_effect = Exception("detector failed") + + with self.assertLogs( + "google.cloud.spanner_v1.metrics.spanner_metrics_tracer_factory", + level="WARNING", + ) as log: + location = SpannerMetricsTracerFactory._get_location() + assert location == "global" + self.assertIn("Failed to detect GCP resource location", log.output[0])