fix(spanner): resolve TypeError in metrics resource detection (#1446)

* fix(spanner): resolve TypeError in metrics resource detection

* fix(spanner): add exception handling for metrics initialization

Author: sinhasubham

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)
 

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[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

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
 

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])