Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions google/cloud/spanner_v1/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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 "
Expand All @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are making Metrics default enabled with this changes, please make sure that metrics are not enabled when emulator is enabled and when customers use NoCredentials

Check https://github.com/googleapis/java-spanner/blob/main/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java#L2088

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes these cases are getting handled.
Emulator condition: https://github.com/googleapis/python-spanner/blob/main/google/cloud/spanner_v1/client.py#L257

NoCredential user condition: https://github.com/googleapis/python-spanner/blob/main/google/cloud/spanner_v1/client.py#L224-L225
This sets emulator setting for NoCredentials users making them act like emulator which means disabled metrics.



class Client(ClientWithProject):
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
):
Expand Down
Loading