From 6abe73f86508eddfc0d20da9429ff028679cf999 Mon Sep 17 00:00:00 2001 From: Subham Sinha Date: Mon, 24 Nov 2025 09:24:32 +0530 Subject: [PATCH 1/2] feat(spanner): make built-in metrics enabled by default --- google/cloud/spanner_v1/client.py | 5 ++--- tests/unit/test_client.py | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/google/cloud/spanner_v1/client.py b/google/cloud/spanner_v1/client.py index eb5b0a6ca6..4d562d354b 100644 --- a/google/cloud/spanner_v1/client.py +++ b/google/cloud/spanner_v1/client.py @@ -52,7 +52,6 @@ from google.cloud.spanner_v1._helpers import _metadata_with_prefix from google.cloud.spanner_v1.instance import Instance from google.cloud.spanner_v1.metrics.constants import ( - ENABLE_SPANNER_METRICS_ENV_VAR, METRIC_EXPORT_INTERVAL_MS, ) from google.cloud.spanner_v1.metrics.spanner_metrics_tracer_factory import ( @@ -75,7 +74,7 @@ _CLIENT_INFO = client_info.ClientInfo(client_library_version=__version__) EMULATOR_ENV_VAR = "SPANNER_EMULATOR_HOST" -ENABLE_BUILTIN_METRICS_ENV_VAR = "SPANNER_ENABLE_BUILTIN_METRICS" +SPANNER_DISABLE_BUILTIN_METRICS_ENV_VAR = "SPANNER_DISABLE_BUILTIN_METRICS" _EMULATOR_HOST_HTTP_SCHEME = ( "%s contains a http scheme. When used with a scheme it may cause gRPC's " "DNS resolver to endlessly attempt to resolve. %s is intended to be used " @@ -102,7 +101,7 @@ def _get_spanner_optimizer_statistics_package(): def _get_spanner_enable_builtin_metrics(): - return os.getenv(ENABLE_SPANNER_METRICS_ENV_VAR) == "true" + return os.getenv(SPANNER_DISABLE_BUILTIN_METRICS_ENV_VAR) != "true" class Client(ClientWithProject): diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index f0d246673a..94481836ce 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -22,6 +22,7 @@ from tests._builders import build_scoped_credentials +@mock.patch.dict(os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "true"}) class TestClient(unittest.TestCase): PROJECT = "PROJECT" PATH = "projects/%s" % (PROJECT,) @@ -161,8 +162,7 @@ def test_constructor_custom_client_info(self): creds = build_scoped_credentials() self._constructor_test_helper(expected_scopes, creds, client_info=client_info) - # Disable metrics to avoid google.auth.default calls from Metric Exporter - @mock.patch.dict(os.environ, {"SPANNER_ENABLE_BUILTIN_METRICS": ""}) + # Metrics are disabled by default for tests in this class def test_constructor_implicit_credentials(self): from google.cloud.spanner_v1 import client as MUT @@ -255,8 +255,8 @@ 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") + @mock.patch.dict(os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "false"}) def test_constructor_w_metrics_initialization_error( self, mock_spanner_metrics_factory ): From 63ca819537d332686e5634dc1134767c1e3fca5a Mon Sep 17 00:00:00 2001 From: Subham Sinha Date: Tue, 2 Dec 2025 14:52:58 +0530 Subject: [PATCH 2/2] feat(metrics): add system test for built-in metrics --- google/cloud/spanner_v1/metrics/constants.py | 1 - tests/system/test_metrics.py | 92 ++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/system/test_metrics.py diff --git a/google/cloud/spanner_v1/metrics/constants.py b/google/cloud/spanner_v1/metrics/constants.py index a47aecc9ed..a5f709881b 100644 --- a/google/cloud/spanner_v1/metrics/constants.py +++ b/google/cloud/spanner_v1/metrics/constants.py @@ -20,7 +20,6 @@ GOOGLE_CLOUD_REGION_KEY = "cloud.region" GOOGLE_CLOUD_REGION_GLOBAL = "global" SPANNER_METHOD_PREFIX = "/google.spanner.v1." -ENABLE_SPANNER_METRICS_ENV_VAR = "SPANNER_ENABLE_BUILTIN_METRICS" # Monitored resource labels MONITORED_RES_LABEL_KEY_PROJECT = "project_id" diff --git a/tests/system/test_metrics.py b/tests/system/test_metrics.py new file mode 100644 index 0000000000..acc8d45cee --- /dev/null +++ b/tests/system/test_metrics.py @@ -0,0 +1,92 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import mock +import pytest + +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import InMemoryMetricReader + +from google.cloud.spanner_v1 import Client + +# System tests are skipped if the environment variables are not set. +PROJECT = os.environ.get("GOOGLE_CLOUD_PROJECT") +INSTANCE_ID = os.environ.get("SPANNER_TEST_INSTANCE") +DATABASE_ID = "test_metrics_db_system" + + +pytestmark = pytest.mark.skipif( + not all([PROJECT, INSTANCE_ID]), reason="System test environment variables not set." +) + + +@pytest.fixture(scope="module") +def metrics_database(): + """Create a database for the test.""" + client = Client(project=PROJECT) + instance = client.instance(INSTANCE_ID) + database = instance.database(DATABASE_ID) + if database.exists(): # Clean up from previous failed run + database.drop() + op = database.create() + op.result(timeout=300) # Wait for creation to complete + yield database + if database.exists(): + database.drop() + + +def test_builtin_metrics_with_default_otel(metrics_database): + """ + Verifies that built-in metrics are collected by default when a + transaction is executed. + """ + reader = InMemoryMetricReader() + meter_provider = MeterProvider(metric_readers=[reader]) + + # Patch the client's metric setup to use our in-memory reader. + with mock.patch( + "google.cloud.spanner_v1.client.MeterProvider", + return_value=meter_provider, + ): + with mock.patch.dict(os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "false"}): + with metrics_database.snapshot() as snapshot: + list(snapshot.execute_sql("SELECT 1")) + + metric_data = reader.get_metrics_data() + + assert len(metric_data.resource_metrics) >= 1 + assert len(metric_data.resource_metrics[0].scope_metrics) >= 1 + + collected_metrics = { + metric.name + for metric in metric_data.resource_metrics[0].scope_metrics[0].metrics + } + expected_metrics = { + "spanner/operation_latencies", + "spanner/attempt_latencies", + "spanner/operation_count", + "spanner/attempt_count", + "spanner/gfe_latencies", + } + assert expected_metrics.issubset(collected_metrics) + + for metric in metric_data.resource_metrics[0].scope_metrics[0].metrics: + if metric.name == "spanner/operation_count": + point = next(iter(metric.data.data_points)) + assert point.value == 1 + assert point.attributes["method"] == "ExecuteSql" + return + + pytest.fail("Metric 'spanner/operation_count' not found.")