Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
36 changes: 36 additions & 0 deletions google/cloud/spanner_v1/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
_CLIENT_INFO = client_info.ClientInfo(client_library_version=__version__)
EMULATOR_ENV_VAR = "SPANNER_EMULATOR_HOST"
SPANNER_DISABLE_BUILTIN_METRICS_ENV_VAR = "SPANNER_DISABLE_BUILTIN_METRICS"
LOG_CLIENT_OPTIONS_ENV_VAR = "GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS"
_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 Down Expand Up @@ -104,6 +105,10 @@ def _get_spanner_enable_builtin_metrics_env():
return os.getenv(SPANNER_DISABLE_BUILTIN_METRICS_ENV_VAR) != "true"


def _get_spanner_log_client_options_env():
return os.getenv(LOG_CLIENT_OPTIONS_ENV_VAR, "false").lower() == "true"


class Client(ClientWithProject):
"""Client for interacting with Cloud Spanner API.

Expand Down Expand Up @@ -292,6 +297,37 @@ def __init__(
self._nth_client_id = Client.NTH_CLIENT.increment()
self._nth_request = AtomicCounter(0)

self.host = "spanner.googleapis.com"
if self._emulator_host:
self.host = self._emulator_host
elif self._experimental_host:
self.host = self._experimental_host
elif self._client_options and self._client_options.api_endpoint:
self.host = self._client_options.api_endpoint

if _get_spanner_log_client_options_env():
self._log_spanner_options()

def _log_spanner_options(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

We have decided not to enable it by default in Go and Node. We should follow the similar approach here. It can enabled through ENV.

@surbhigarg92 your thoughts here?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, lets do the same. We have already received customer issues in Node and Go. Add it behind a ENV same as Go. I feel only one ENV is enough GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added new env variable as a control to client options logging.

"""Logs Spanner client options."""
log.info(
"Spanner options: \n"
" Project ID: %s\n"
" Host: %s\n"
" Route to leader enabled: %s\n"
" Directed read options: %s\n"
" Default transaction options: %s\n"
" Observability options: %s\n"
" Built-in metrics enabled: %s",
self.project,
self.host,
self.route_to_leader_enabled,
self._directed_read_options,
self._default_transaction_options,
self._observability_options,
_get_spanner_enable_builtin_metrics_env(),
)

@property
def _next_nth_request(self):
return self._nth_request.increment()
Expand Down
110 changes: 109 additions & 1 deletion tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def _constructor_test_helper(
query_options=query_options,
directed_read_options=directed_read_options,
default_transaction_options=default_transaction_options,
**kwargs
**kwargs,
)

expected_creds = expected_creds or creds.with_scopes.return_value
Expand Down Expand Up @@ -329,6 +329,114 @@ def test_constructor_w_default_transaction_options(self):
default_transaction_options=self.DEFAULT_TRANSACTION_OPTIONS,
)

@mock.patch.dict(
os.environ, {"GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS": "false"}
)
def test_constructor_logs_options_disabled_by_default(self):
from google.cloud.spanner_v1 import client as MUT

logger = MUT.log
creds = build_scoped_credentials()

with mock.patch.object(logger, "info") as info_logger:
client = self._make_one(
project=self.PROJECT,
credentials=creds,
)
self.assertIsNotNone(client)
# Assert that no logs are emitted when the environment variable is explicitly false
info_logger.assert_not_called()

# Also test when the environment variable is not set at all
with mock.patch.dict(os.environ, {}, clear=True):
with mock.patch.object(logger, "info") as info_logger:
client = self._make_one(project=self.PROJECT, credentials=creds)
self.assertIsNotNone(client)
# Assert that no logs are emitted when the environment variable is not set
info_logger.assert_not_called()

def test_constructor_logs_options(self):
from google.cloud.spanner_v1 import client as MUT

creds = build_scoped_credentials()
observability_options = {"enable_extended_tracing": True}
with self.assertLogs(MUT.__name__, level="INFO") as cm:
with mock.patch.dict(
os.environ, {"GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS": "true"}
):
client = self._make_one(
project=self.PROJECT,
credentials=creds,
route_to_leader_enabled=False,
directed_read_options=self.DIRECTED_READ_OPTIONS,
default_transaction_options=self.DEFAULT_TRANSACTION_OPTIONS,
observability_options=observability_options,
)
self.assertIsNotNone(client)

# Assert that logs are emitted when the environment variable is true
# and verify their content.
self.assertEqual(len(cm.output), 1)
log_output = cm.output[0]
self.assertIn("Spanner options:", log_output)
self.assertIn(f"\n Project ID: {self.PROJECT}", log_output)
self.assertIn("\n Host: spanner.googleapis.com", log_output)
self.assertIn("\n Route to leader enabled: False", log_output)
self.assertIn(
f"\n Directed read options: {self.DIRECTED_READ_OPTIONS}", log_output
)
self.assertIn(
f"\n Default transaction options: {self.DEFAULT_TRANSACTION_OPTIONS}",
log_output,
)
self.assertIn(f"\n Observability options: {observability_options}", log_output)
# SPANNER_DISABLE_BUILTIN_METRICS is "true" from class-level patch
self.assertIn("\n Built-in metrics enabled: False", log_output)
# Test with custom host
endpoint = "test.googleapis.com"
with self.assertLogs(MUT.__name__, level="INFO") as cm:
with mock.patch.dict(
os.environ, {"GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS": "true"}
):
self._make_one(
project=self.PROJECT,
credentials=creds,
client_options={"api_endpoint": endpoint},
)
self.assertEqual(len(cm.output), 1)
log_output = cm.output[0]
self.assertIn(f"\n Host: {endpoint}", log_output)

# Test with emulator host
emulator_host = "localhost:9010"
with mock.patch.dict(
os.environ,
{
MUT.EMULATOR_ENV_VAR: emulator_host,
"GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS": "true",
},
):
with self.assertLogs(MUT.__name__, level="INFO") as cm:
self._make_one(project=self.PROJECT, credentials=creds)
self.assertEqual(len(cm.output), 1)
log_output = cm.output[0]
self.assertIn(f"\n Host: {emulator_host}", log_output)

# Test with experimental host
experimental_host = "exp.googleapis.com"
with mock.patch.dict(
os.environ, {"GOOGLE_CLOUD_SPANNER_ENABLE_LOG_CLIENT_OPTIONS": "true"}
):
with self.assertLogs(MUT.__name__, level="INFO") as cm:
self._make_one(
project=self.PROJECT,
credentials=creds,
experimental_host=experimental_host,
)
self.assertEqual(len(cm.output), 1)
log_output = cm.output[0]
self.assertIn(f"\n Host: {experimental_host}", log_output)

@mock.patch("google.cloud.spanner_v1.client._get_spanner_emulator_host")
def test_instance_admin_api(self, mock_em):
from google.cloud.spanner_v1.client import SPANNER_ADMIN_SCOPE
Expand Down