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

Commit 7266686

Browse files
sinhasubhamsinhasubham
authored
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
1 parent a09961b commit 7266686

4 files changed

Lines changed: 108 additions & 21 deletions

File tree

‎google/cloud/spanner_v1/client.py‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"""
2626
import grpc
2727
import os
28+
import logging
2829
import warnings
2930

3031
from google.api_core.gapic_v1 import client_info
@@ -97,6 +98,9 @@ def _get_spanner_optimizer_statistics_package():
9798
return os.getenv(OPTIMIZER_STATISITCS_PACKAGE_ENV_VAR, "")
9899

99100

101+
log = logging.getLogger(__name__)
102+
103+
100104
def _get_spanner_enable_builtin_metrics():
101105
return os.getenv(ENABLE_SPANNER_METRICS_ENV_VAR) == "true"
102106

@@ -240,19 +244,24 @@ def __init__(
240244
and HAS_GOOGLE_CLOUD_MONITORING_INSTALLED
241245
):
242246
meter_provider = metrics.NoOpMeterProvider()
243-
if not _get_spanner_emulator_host():
244-
meter_provider = MeterProvider(
245-
metric_readers=[
246-
PeriodicExportingMetricReader(
247-
CloudMonitoringMetricsExporter(
248-
project_id=project, credentials=credentials
247+
try:
248+
if not _get_spanner_emulator_host():
249+
meter_provider = MeterProvider(
250+
metric_readers=[
251+
PeriodicExportingMetricReader(
252+
CloudMonitoringMetricsExporter(
253+
project_id=project, credentials=credentials
254+
),
255+
export_interval_millis=METRIC_EXPORT_INTERVAL_MS,
249256
),
250-
export_interval_millis=METRIC_EXPORT_INTERVAL_MS,
251-
)
252-
]
257+
]
258+
)
259+
metrics.set_meter_provider(meter_provider)
260+
SpannerMetricsTracerFactory()
261+
except Exception as e:
262+
log.warning(
263+
"Failed to initialize Spanner built-in metrics. Error: %s", e
253264
)
254-
metrics.set_meter_provider(meter_provider)
255-
SpannerMetricsTracerFactory()
256265
else:
257266
SpannerMetricsTracerFactory(enabled=False)
258267

‎google/cloud/spanner_v1/metrics/spanner_metrics_tracer_factory.py‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
from .metrics_tracer_factory import MetricsTracerFactory
1919
import os
20+
import logging
2021
from .constants import (
2122
SPANNER_SERVICE_NAME,
2223
GOOGLE_CLOUD_REGION_KEY,
@@ -33,9 +34,6 @@
3334

3435
import mmh3
3536

36-
# Override Resource detector logging to not warn when GCP resources are not detected
37-
import logging
38-
3937
logging.getLogger("opentelemetry.resourcedetector.gcp_resource_detector").setLevel(
4038
logging.ERROR
4139
)
@@ -48,6 +46,8 @@
4846
from google.cloud.spanner_v1 import __version__
4947
from uuid import uuid4
5048

49+
log = logging.getLogger(__name__)
50+
5151

5252
class SpannerMetricsTracerFactory(MetricsTracerFactory):
5353
"""A factory for creating SpannerMetricsTracer instances."""
@@ -158,15 +158,23 @@ def _generate_client_hash(client_uid: str) -> str:
158158
def _get_location() -> str:
159159
"""Get the location of the resource.
160160
161+
In case of any error during detection, this method will log a warning
162+
and default to the "global" location.
163+
161164
Returns:
162165
str: The location of the resource. If OpenTelemetry is not installed, returns a global region.
163166
"""
164167
if not HAS_OPENTELEMETRY_INSTALLED:
165168
return GOOGLE_CLOUD_REGION_GLOBAL
166-
detector = gcp_resource_detector.GoogleCloudResourceDetector()
167-
resources = detector.detect()
168-
169-
if GOOGLE_CLOUD_REGION_KEY not in resources.attributes:
170-
return GOOGLE_CLOUD_REGION_GLOBAL
171-
else:
172-
return resources[GOOGLE_CLOUD_REGION_KEY]
169+
try:
170+
detector = gcp_resource_detector.GoogleCloudResourceDetector()
171+
resources = detector.detect()
172+
173+
if GOOGLE_CLOUD_REGION_KEY in resources.attributes:
174+
return resources.attributes[GOOGLE_CLOUD_REGION_KEY]
175+
except Exception as e:
176+
log.warning(
177+
"Failed to detect GCP resource location for Spanner metrics, defaulting to 'global'. Error: %s",
178+
e,
179+
)
180+
return GOOGLE_CLOUD_REGION_GLOBAL

‎tests/unit/test_client.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,29 @@ def test_constructor_w_directed_read_options(self):
255255
expected_scopes, creds, directed_read_options=self.DIRECTED_READ_OPTIONS
256256
)
257257

258+
@mock.patch.dict(os.environ, {"SPANNER_ENABLE_BUILTIN_METRICS": "true"})
259+
@mock.patch("google.cloud.spanner_v1.client.SpannerMetricsTracerFactory")
260+
def test_constructor_w_metrics_initialization_error(
261+
self, mock_spanner_metrics_factory
262+
):
263+
"""
264+
Test that Client constructor handles exceptions during metrics
265+
initialization and logs a warning.
266+
"""
267+
from google.cloud.spanner_v1.client import Client
268+
269+
mock_spanner_metrics_factory.side_effect = Exception("Metrics init failed")
270+
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()
280+
258281
def test_constructor_route_to_leader_disbled(self):
259282
from google.cloud.spanner_v1 import client as MUT
260283

‎tests/unit/test_spanner_metrics_tracer_factory.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,17 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515

16+
import pytest
17+
import unittest
18+
from unittest import mock
19+
20+
from google.cloud.spanner_v1.metrics.constants import GOOGLE_CLOUD_REGION_KEY
1621
from google.cloud.spanner_v1.metrics.spanner_metrics_tracer_factory import (
1722
SpannerMetricsTracerFactory,
1823
)
24+
from opentelemetry.sdk.resources import Resource
25+
26+
pytest.importorskip("opentelemetry")
1927

2028

2129
class TestSpannerMetricsTracerFactory:
@@ -48,3 +56,42 @@ def test_get_location(self):
4856
location = SpannerMetricsTracerFactory._get_location()
4957
assert isinstance(location, str)
5058
assert location # Simply asserting for non empty as this can change depending on the instance this test runs in.
59+
60+
61+
class TestSpannerMetricsTracerFactoryGetLocation(unittest.TestCase):
62+
@mock.patch(
63+
"opentelemetry.resourcedetector.gcp_resource_detector.GoogleCloudResourceDetector.detect"
64+
)
65+
def test_get_location_with_region(self, mock_detect):
66+
"""Test that _get_location returns the region when detected."""
67+
mock_resource = Resource.create({GOOGLE_CLOUD_REGION_KEY: "us-central1"})
68+
mock_detect.return_value = mock_resource
69+
70+
location = SpannerMetricsTracerFactory._get_location()
71+
assert location == "us-central1"
72+
73+
@mock.patch(
74+
"opentelemetry.resourcedetector.gcp_resource_detector.GoogleCloudResourceDetector.detect"
75+
)
76+
def test_get_location_without_region(self, mock_detect):
77+
"""Test that _get_location returns 'global' when no region is detected."""
78+
mock_resource = Resource.create({}) # No region attribute
79+
mock_detect.return_value = mock_resource
80+
81+
location = SpannerMetricsTracerFactory._get_location()
82+
assert location == "global"
83+
84+
@mock.patch(
85+
"opentelemetry.resourcedetector.gcp_resource_detector.GoogleCloudResourceDetector.detect"
86+
)
87+
def test_get_location_with_exception(self, mock_detect):
88+
"""Test that _get_location returns 'global' and logs a warning on exception."""
89+
mock_detect.side_effect = Exception("detector failed")
90+
91+
with self.assertLogs(
92+
"google.cloud.spanner_v1.metrics.spanner_metrics_tracer_factory",
93+
level="WARNING",
94+
) as log:
95+
location = SpannerMetricsTracerFactory._get_location()
96+
assert location == "global"
97+
self.assertIn("Failed to detect GCP resource location", log.output[0])

0 commit comments

Comments
 (0)