From 8f3801fbf9b67d76ed6f385394645b6f58567878 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Wed, 23 Sep 2026 08:19:53 +0000 Subject: [PATCH 1/5] feat(storage): integrate OpenTelemetry metrics gating across clients and transfer_manager - Add enable_metrics and enable_debug_metrics to Client, GrpcClient, and AsyncGrpcClient - Add metrics_enabled and debug_metrics_enabled properties across all client classes - Preserve metrics configuration across process boundaries in transfer_manager - Add unit tests in test_client.py, test_grpc_client.py, test_async_grpc_client.py, and test_transfer_manager.py --- .../storage/asyncio/async_grpc_client.py | 32 +++++++- .../google/cloud/storage/client.py | 30 +++++++ .../google/cloud/storage/grpc_client.py | 31 +++++++ .../google/cloud/storage/transfer_manager.py | 8 ++ .../unit/asyncio/test_async_grpc_client.py | 39 +++++++++ .../tests/unit/test_client.py | 80 +++++++++++++++++++ .../tests/unit/test_grpc_client.py | 40 ++++++++++ .../tests/unit/test_transfer_manager.py | 15 +++- 8 files changed, 271 insertions(+), 4 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py index 3b1487c8c7bb..e5e01e020794 100644 --- a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py @@ -21,7 +21,7 @@ from google.cloud._storage_v2.services.storage.transports.base import ( DEFAULT_CLIENT_INFO, ) -from google.cloud.storage import __version__ +from google.cloud.storage import __version__, _opentelemetry_metrics from google.cloud.storage.grpc_client import ( _DEFAULT_HOST, _resolve_direct_path_interconnect, @@ -73,6 +73,19 @@ class AsyncGrpcClient: using xDS and standard TLS. Can also be overridden via the ``GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS_OVER_INTERCONNECT`` environment variable (``"true"`` or ``"false"``). Defaults to ``False``. + + :type enable_metrics: bool or None + :param enable_metrics: + (Optional, Experimental) Whether to enable OpenTelemetry metrics. If + None, falls back to the GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS + environment variable, or False if unset. + + :type enable_debug_metrics: bool or None + :param enable_debug_metrics: + (Optional, Experimental) Whether to enable debug OpenTelemetry + metrics. If None, falls back to the + GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS environment variable, or False + if unset. """ def __init__( @@ -83,7 +96,12 @@ def __init__( *, attempt_direct_path=True, attempt_direct_path_xds_over_interconnect=False, + enable_metrics=None, + enable_debug_metrics=None, ): + self._enable_metrics = enable_metrics + self._enable_debug_metrics = enable_debug_metrics + if isinstance(credentials, auth_credentials.AnonymousCredentials): if client_options is None or client_options.api_endpoint is None: raise ValueError( @@ -115,6 +133,18 @@ def __init__( attempt_direct_path_xds_over_interconnect=use_dp_interconnect, ) + @property + def metrics_enabled(self) -> bool: + """Returns True if metrics recording is active for this client.""" + return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) + + @property + def debug_metrics_enabled(self) -> bool: + """Returns True if debug metrics recording is active for this client.""" + return _opentelemetry_metrics.is_debug_metrics_enabled( + self._enable_debug_metrics + ) + def _create_anonymous_client(self, client_options, credentials): channel = grpc.aio.insecure_channel(client_options.api_endpoint) transport = storage_v2.services.storage.transports.StorageGrpcAsyncIOTransport( diff --git a/packages/google-cloud-storage/google/cloud/storage/client.py b/packages/google-cloud-storage/google/cloud/storage/client.py index 5559dbc7d763..501234c2ac54 100644 --- a/packages/google-cloud-storage/google/cloud/storage/client.py +++ b/packages/google-cloud-storage/google/cloud/storage/client.py @@ -32,6 +32,7 @@ from google.cloud.client import ClientWithProject from google.cloud.exceptions import NotFound +from google.cloud.storage import _opentelemetry_metrics from google.cloud.storage._bucket_metadata_cache import BucketMetadataCache from google.cloud.storage._helpers import ( _DEFAULT_SCHEME, @@ -126,6 +127,19 @@ class Client(ClientWithProject): (Optional) An API key. Mutually exclusive with any other credentials. This parameter is an alias for setting `client_options.api_key` and will supercede any api key set in the `client_options` parameter. + + :type enable_metrics: bool or None + :param enable_metrics: + (Optional, Experimental) Whether to enable OpenTelemetry metrics. If + None, falls back to the GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS + environment variable, or False if unset. + + :type enable_debug_metrics: bool or None + :param enable_debug_metrics: + (Optional, Experimental) Whether to enable debug OpenTelemetry + metrics. If None, falls back to the + GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS environment variable, or False + if unset. """ SCOPE = ( @@ -146,6 +160,8 @@ def __init__( extra_headers={}, *, api_key=None, + enable_metrics=None, + enable_debug_metrics=None, ): self._base_connection = None @@ -293,6 +309,20 @@ def __init__( self._connection = connection self._batch_stack = _LocalStack() self._bucket_metadata_cache = BucketMetadataCache(self) + self._enable_metrics = enable_metrics + self._enable_debug_metrics = enable_debug_metrics + + @property + def metrics_enabled(self) -> bool: + """Returns True if metrics recording is active for this client.""" + return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) + + @property + def debug_metrics_enabled(self) -> bool: + """Returns True if debug metrics recording is active for this client.""" + return _opentelemetry_metrics.is_debug_metrics_enabled( + self._enable_debug_metrics + ) def close(self): """Close the client and clear any cached metadata or active connections.""" diff --git a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py index b3d8b261e77f..8125cfd1a58d 100644 --- a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py @@ -19,6 +19,7 @@ from google.cloud.client import ClientWithProject from google.cloud import _storage_v2 as storage_v2 +from google.cloud.storage import _opentelemetry_metrics _marker = object() _DEFAULT_HOST = "storage.googleapis.com" @@ -103,6 +104,19 @@ class GrpcClient(ClientWithProject): using xDS and standard TLS. Can also be overridden via the ``GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS_OVER_INTERCONNECT`` environment variable (``"true"`` or ``"false"``). Defaults to ``False``. + + :type enable_metrics: bool or None + :param enable_metrics: + (Optional, Experimental) Whether to enable OpenTelemetry metrics. If + None, falls back to the GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS + environment variable, or False if unset. + + :type enable_debug_metrics: bool or None + :param enable_debug_metrics: + (Optional, Experimental) Whether to enable debug OpenTelemetry + metrics. If None, falls back to the + GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS environment variable, or False + if unset. """ def __init__( @@ -115,9 +129,14 @@ def __init__( api_key=None, attempt_direct_path=True, attempt_direct_path_xds_over_interconnect=False, + enable_metrics=None, + enable_debug_metrics=None, ): super(GrpcClient, self).__init__(project=project, credentials=credentials) + self._enable_metrics = enable_metrics + self._enable_debug_metrics = enable_debug_metrics + if isinstance(client_options, dict): if api_key: client_options["api_key"] = api_key @@ -138,6 +157,18 @@ def __init__( attempt_direct_path_xds_over_interconnect=use_dp_interconnect, ) + @property + def metrics_enabled(self) -> bool: + """Returns True if metrics recording is active for this client.""" + return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) + + @property + def debug_metrics_enabled(self) -> bool: + """Returns True if debug metrics recording is active for this client.""" + return _opentelemetry_metrics.is_debug_metrics_enabled( + self._enable_debug_metrics + ) + def _create_gapic_client( self, credentials=None, diff --git a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py index 6486696021fc..3ca93a1b7745 100644 --- a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py +++ b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py @@ -1383,6 +1383,8 @@ def _reduce_client(cl): client_info = cl._initial_client_info client_options = cl._initial_client_options extra_headers = getattr(cl, "_extra_headers", {}) + enable_metrics = getattr(cl, "_enable_metrics", None) + enable_debug_metrics = getattr(cl, "_enable_debug_metrics", None) return _LazyClient, ( client_object_id, @@ -1392,6 +1394,8 @@ def _reduce_client(cl): client_info, client_options, extra_headers, + enable_metrics, + enable_debug_metrics, ) @@ -1461,6 +1465,10 @@ def __new__(cls, id, *args, **kwargs): if cached_client: return cached_client else: + if len(args) >= 8: + kwargs.setdefault("enable_metrics", args[6]) + kwargs.setdefault("enable_debug_metrics", args[7]) + args = args[:6] cached_client = Client(*args, **kwargs) _cached_clients[id] = cached_client return cached_client diff --git a/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py b/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py index 9dffa6b36c86..73a11dd8f6b0 100644 --- a/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py @@ -149,6 +149,45 @@ def test_constructor_disables_directpath(self, mock_async_storage_client): mock_channel = mock_transport_cls.create_channel.return_value mock_transport_cls.assert_called_once_with(channel=mock_channel) + @mock.patch("google.cloud._storage_v2.StorageAsyncClient") + def test_metrics_properties(self, mock_async_storage_client): + from google.cloud.storage import _opentelemetry_metrics + + mock_transport_cls = mock.MagicMock() + mock_async_storage_client.get_transport_class.return_value = mock_transport_cls + mock_creds = _make_credentials() + + client = async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_metrics=True, + enable_debug_metrics=True, + ) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + assert client.metrics_enabled is True + assert client.debug_metrics_enabled is True + + client._enable_metrics = False + assert client.metrics_enabled is False + assert client.debug_metrics_enabled is True + + client._enable_debug_metrics = False + assert client.debug_metrics_enabled is False + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=False, + ): + client._enable_metrics = True + client._enable_debug_metrics = True + assert client.metrics_enabled is False + assert client.debug_metrics_enabled is False + @mock.patch("google.cloud._storage_v2.StorageAsyncClient") def test_grpc_client_property(self, mock_grpc_gapic_client): # Arrange diff --git a/packages/google-cloud-storage/tests/unit/test_client.py b/packages/google-cloud-storage/tests/unit/test_client.py index 87d82581490d..14f7061534f4 100644 --- a/packages/google-cloud-storage/tests/unit/test_client.py +++ b/packages/google-cloud-storage/tests/unit/test_client.py @@ -259,6 +259,86 @@ def test_ctor_w_universe_domain_and_matched_credentials(self): self.assertEqual(client.api_endpoint, expected_api_endpoint) self.assertEqual(client.universe_domain, universe_domain) + def test_ctor_w_enable_metrics(self): + PROJECT = "PROJECT" + credentials = _make_credentials() + client = self._make_one( + project=PROJECT, credentials=credentials, enable_metrics=True + ) + self.assertTrue(client._enable_metrics) + + def test_ctor_w_enable_debug_metrics(self): + PROJECT = "PROJECT" + credentials = _make_credentials() + client = self._make_one( + project=PROJECT, credentials=credentials, enable_debug_metrics=True + ) + self.assertTrue(client._enable_debug_metrics) + + def test_client_metrics_enabled_property(self): + from google.cloud.storage import _opentelemetry_metrics + + PROJECT = "PROJECT" + credentials = _make_credentials() + client = self._make_one( + project=PROJECT, credentials=credentials, enable_metrics=True + ) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + self.assertTrue(client.metrics_enabled) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=False, + ): + self.assertFalse(client.metrics_enabled) + + def test_client_debug_metrics_enabled_property(self): + from google.cloud.storage import _opentelemetry_metrics + + PROJECT = "PROJECT" + credentials = _make_credentials() + client = self._make_one( + project=PROJECT, + credentials=credentials, + enable_metrics=True, + enable_debug_metrics=True, + ) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + self.assertTrue(client.debug_metrics_enabled) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=False, + ): + self.assertFalse(client.debug_metrics_enabled) + + # Verify that debug metrics are independent when base metrics are disabled + client_disabled_base = self._make_one( + project=PROJECT, + credentials=credentials, + enable_metrics=False, + enable_debug_metrics=True, + ) + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + self.assertFalse(client_disabled_base.metrics_enabled) + self.assertTrue(client_disabled_base.debug_metrics_enabled) + def test_ctor_w_universe_domain_and_mismatched_credentials(self): PROJECT = "PROJECT" universe_domain = "example.com" diff --git a/packages/google-cloud-storage/tests/unit/test_grpc_client.py b/packages/google-cloud-storage/tests/unit/test_grpc_client.py index 73a8e5b273e4..b243fcf4e7a8 100644 --- a/packages/google-cloud-storage/tests/unit/test_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/test_grpc_client.py @@ -360,3 +360,43 @@ def test_rewrite_host_for_interconnect_delimiters(self): grpc_client._rewrite_host_for_interconnect(original_host), expected_host, ) + + @mock.patch("google.cloud.storage.grpc_client.ClientWithProject") + @mock.patch("google.cloud._storage_v2.StorageClient") + def test_metrics_properties(self, mock_storage_client, mock_base_client): + from google.cloud.storage import _opentelemetry_metrics + + mock_creds = _make_credentials() + mock_base_client.return_value._credentials = mock_creds + + client = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_metrics=True, + enable_debug_metrics=True, + ) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + self.assertTrue(client.metrics_enabled) + self.assertTrue(client.debug_metrics_enabled) + + client._enable_metrics = False + self.assertFalse(client.metrics_enabled) + self.assertTrue(client.debug_metrics_enabled) + + client._enable_debug_metrics = False + self.assertFalse(client.debug_metrics_enabled) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=False, + ): + client._enable_metrics = True + client._enable_debug_metrics = True + self.assertFalse(client.metrics_enabled) + self.assertFalse(client.debug_metrics_enabled) diff --git a/packages/google-cloud-storage/tests/unit/test_transfer_manager.py b/packages/google-cloud-storage/tests/unit/test_transfer_manager.py index b9d6f11a742d..90aec859a44c 100644 --- a/packages/google-cloud-storage/tests/unit/test_transfer_manager.py +++ b/packages/google-cloud-storage/tests/unit/test_transfer_manager.py @@ -1405,16 +1405,25 @@ def test__reduce_client(): "x-goog-custom-audit-foo": "bar", } client._extra_headers = custom_headers + client._enable_metrics = True + client._enable_debug_metrics = True with ( mock.patch( "google.cloud.storage.transfer_manager._cached_clients", new=fake_cache ), - mock.patch("google.cloud.storage.transfer_manager.Client"), + mock.patch("google.cloud.storage.transfer_manager.Client") as mock_client_cls, ): - replicated_client, kwargs = transfer_manager._reduce_client(client) + replicated_client, args = transfer_manager._reduce_client(client) assert replicated_client is not None - assert custom_headers in kwargs + assert custom_headers in args + assert args[-2:] == (True, True) + replicated_client(*args) + mock_client_cls.assert_called_once_with( + *args[1:7], + enable_metrics=True, + enable_debug_metrics=True, + ) def test__call_method_on_maybe_pickled_blob(): From d04e06804016013f8890143222ad8062a35e50d7 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Thu, 8 Oct 2026 17:04:15 +0000 Subject: [PATCH 2/5] fix(storage): handle intermediate positional arg counts in _LazyClient.__new__ --- .../google/cloud/storage/transfer_manager.py | 5 +++-- .../tests/unit/test_transfer_manager.py | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py index 3ca93a1b7745..f50912048b84 100644 --- a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py +++ b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py @@ -1465,9 +1465,10 @@ def __new__(cls, id, *args, **kwargs): if cached_client: return cached_client else: - if len(args) >= 8: + if len(args) > 6: kwargs.setdefault("enable_metrics", args[6]) - kwargs.setdefault("enable_debug_metrics", args[7]) + if len(args) > 7: + kwargs.setdefault("enable_debug_metrics", args[7]) args = args[:6] cached_client = Client(*args, **kwargs) _cached_clients[id] = cached_client diff --git a/packages/google-cloud-storage/tests/unit/test_transfer_manager.py b/packages/google-cloud-storage/tests/unit/test_transfer_manager.py index 90aec859a44c..b008b6c7e74d 100644 --- a/packages/google-cloud-storage/tests/unit/test_transfer_manager.py +++ b/packages/google-cloud-storage/tests/unit/test_transfer_manager.py @@ -1425,6 +1425,15 @@ def test__reduce_client(): enable_debug_metrics=True, ) + # Verify compatibility when only enable_metrics is present (len(args) == 7 after id) + fake_cache.clear() + mock_client_cls.reset_mock() + replicated_client(*args[:-1]) + mock_client_cls.assert_called_once_with( + *args[1:7], + enable_metrics=True, + ) + def test__call_method_on_maybe_pickled_blob(): blob = mock.Mock(spec=Blob) From 507dbaf8492cadcc2fa0454993a85b5cbcfb17af Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Mon, 12 Oct 2026 08:02:08 +0000 Subject: [PATCH 3/5] fix(storage): refine OpenTelemetry metrics client initialization, docstrings, and unit tests - Initialize _enable_metrics and _enable_debug_metrics earlier in Client.__init__ before Connection creation. - Extract _REDUCE_CLIENT_POS_ARGS = 6 constant in _LazyClient.__new__ to document positional argument slicing. - Add unit tests for default None metrics arguments and environment variable fallback across Client, GrpcClient, and AsyncGrpcClient. - Remove unused ClientWithProject mock patch from TestGrpcClient.test_metrics_properties. - Format Sphinx docstring literals and environment variable names with double backticks and mark properties as (Experimental). [Generated-by: AI] --- .../storage/asyncio/async_grpc_client.py | 14 +++---- .../google/cloud/storage/client.py | 18 ++++----- .../google/cloud/storage/grpc_client.py | 14 +++---- .../google/cloud/storage/transfer_manager.py | 19 +++++++--- .../unit/asyncio/test_async_grpc_client.py | 33 +++++++++++++++++ .../tests/unit/test_client.py | 30 +++++++++++++++ .../tests/unit/test_grpc_client.py | 37 +++++++++++++++++-- 7 files changed, 134 insertions(+), 31 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py index e5e01e020794..3613e4eb83ae 100644 --- a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py @@ -77,15 +77,15 @@ class AsyncGrpcClient: :type enable_metrics: bool or None :param enable_metrics: (Optional, Experimental) Whether to enable OpenTelemetry metrics. If - None, falls back to the GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS - environment variable, or False if unset. + ``None``, falls back to the ``GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS`` + environment variable, or ``False`` if unset. :type enable_debug_metrics: bool or None :param enable_debug_metrics: (Optional, Experimental) Whether to enable debug OpenTelemetry - metrics. If None, falls back to the - GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS environment variable, or False - if unset. + metrics. If ``None``, falls back to the + ``GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS`` environment variable, + or ``False`` if unset. """ def __init__( @@ -135,12 +135,12 @@ def __init__( @property def metrics_enabled(self) -> bool: - """Returns True if metrics recording is active for this client.""" + """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) @property def debug_metrics_enabled(self) -> bool: - """Returns True if debug metrics recording is active for this client.""" + """(Experimental) Returns ``True`` if debug metrics recording is active for this client.""" return _opentelemetry_metrics.is_debug_metrics_enabled( self._enable_debug_metrics ) diff --git a/packages/google-cloud-storage/google/cloud/storage/client.py b/packages/google-cloud-storage/google/cloud/storage/client.py index 501234c2ac54..07cb2af468ef 100644 --- a/packages/google-cloud-storage/google/cloud/storage/client.py +++ b/packages/google-cloud-storage/google/cloud/storage/client.py @@ -131,15 +131,15 @@ class Client(ClientWithProject): :type enable_metrics: bool or None :param enable_metrics: (Optional, Experimental) Whether to enable OpenTelemetry metrics. If - None, falls back to the GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS - environment variable, or False if unset. + ``None``, falls back to the ``GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS`` + environment variable, or ``False`` if unset. :type enable_debug_metrics: bool or None :param enable_debug_metrics: (Optional, Experimental) Whether to enable debug OpenTelemetry - metrics. If None, falls back to the - GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS environment variable, or False - if unset. + metrics. If ``None``, falls back to the + ``GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS`` environment variable, + or ``False`` if unset. """ SCOPE = ( @@ -179,6 +179,8 @@ def __init__( self._initial_client_info = client_info self._initial_client_options = client_options self._extra_headers = extra_headers + self._enable_metrics = enable_metrics + self._enable_debug_metrics = enable_debug_metrics connection_kw_args = {"client_info": client_info} @@ -309,17 +311,15 @@ def __init__( self._connection = connection self._batch_stack = _LocalStack() self._bucket_metadata_cache = BucketMetadataCache(self) - self._enable_metrics = enable_metrics - self._enable_debug_metrics = enable_debug_metrics @property def metrics_enabled(self) -> bool: - """Returns True if metrics recording is active for this client.""" + """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) @property def debug_metrics_enabled(self) -> bool: - """Returns True if debug metrics recording is active for this client.""" + """(Experimental) Returns ``True`` if debug metrics recording is active for this client.""" return _opentelemetry_metrics.is_debug_metrics_enabled( self._enable_debug_metrics ) diff --git a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py index 8125cfd1a58d..988109be9122 100644 --- a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py @@ -108,15 +108,15 @@ class GrpcClient(ClientWithProject): :type enable_metrics: bool or None :param enable_metrics: (Optional, Experimental) Whether to enable OpenTelemetry metrics. If - None, falls back to the GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS - environment variable, or False if unset. + ``None``, falls back to the ``GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS`` + environment variable, or ``False`` if unset. :type enable_debug_metrics: bool or None :param enable_debug_metrics: (Optional, Experimental) Whether to enable debug OpenTelemetry - metrics. If None, falls back to the - GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS environment variable, or False - if unset. + metrics. If ``None``, falls back to the + ``GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS`` environment variable, + or ``False`` if unset. """ def __init__( @@ -159,12 +159,12 @@ def __init__( @property def metrics_enabled(self) -> bool: - """Returns True if metrics recording is active for this client.""" + """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) @property def debug_metrics_enabled(self) -> bool: - """Returns True if debug metrics recording is active for this client.""" + """(Experimental) Returns ``True`` if debug metrics recording is active for this client.""" return _opentelemetry_metrics.is_debug_metrics_enabled( self._enable_debug_metrics ) diff --git a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py index f50912048b84..974f0aac5a8c 100644 --- a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py +++ b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py @@ -1457,6 +1457,13 @@ def _digest_ordered_checksum_and_size_pairs(checksum_and_size_pairs): return crc_digest +# Number of positional Client constructor arguments passed after `id` by +# `_reduce_client`: (project, credentials, _http, client_info, client_options, +# extra_headers). Trailing items in `args` correspond to keyword-only +# `enable_metrics` and `enable_debug_metrics` parameters. +_REDUCE_CLIENT_POS_ARGS = 6 + + class _LazyClient: """An object that will transform into either a cached or a new Client""" @@ -1465,11 +1472,13 @@ def __new__(cls, id, *args, **kwargs): if cached_client: return cached_client else: - if len(args) > 6: - kwargs.setdefault("enable_metrics", args[6]) - if len(args) > 7: - kwargs.setdefault("enable_debug_metrics", args[7]) - args = args[:6] + if len(args) > _REDUCE_CLIENT_POS_ARGS: + kwargs.setdefault("enable_metrics", args[_REDUCE_CLIENT_POS_ARGS]) + if len(args) > _REDUCE_CLIENT_POS_ARGS + 1: + kwargs.setdefault( + "enable_debug_metrics", args[_REDUCE_CLIENT_POS_ARGS + 1] + ) + args = args[:_REDUCE_CLIENT_POS_ARGS] cached_client = Client(*args, **kwargs) _cached_clients[id] = cached_client return cached_client diff --git a/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py b/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py index 73a11dd8f6b0..494746266647 100644 --- a/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py @@ -188,6 +188,39 @@ def test_metrics_properties(self, mock_async_storage_client): assert client.metrics_enabled is False assert client.debug_metrics_enabled is False + @mock.patch("google.cloud._storage_v2.StorageAsyncClient") + def test_ctor_metrics_defaults_and_env_fallback(self, mock_async_storage_client): + from google.cloud.storage import _opentelemetry_metrics + + mock_transport_cls = mock.MagicMock() + mock_async_storage_client.get_transport_class.return_value = mock_transport_cls + mock_creds = _make_credentials() + + client = async_grpc_client.AsyncGrpcClient(credentials=mock_creds) + + assert client._enable_metrics is None + assert client._enable_debug_metrics is None + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + with mock.patch.dict("os.environ", {}, clear=True): + assert client.metrics_enabled is False + assert client.debug_metrics_enabled is False + + with mock.patch.dict( + "os.environ", + { + "GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS": "true", + "GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS": "true", + }, + clear=True, + ): + assert client.metrics_enabled is True + assert client.debug_metrics_enabled is True + @mock.patch("google.cloud._storage_v2.StorageAsyncClient") def test_grpc_client_property(self, mock_grpc_gapic_client): # Arrange diff --git a/packages/google-cloud-storage/tests/unit/test_client.py b/packages/google-cloud-storage/tests/unit/test_client.py index 14f7061534f4..6ba7a6d51e91 100644 --- a/packages/google-cloud-storage/tests/unit/test_client.py +++ b/packages/google-cloud-storage/tests/unit/test_client.py @@ -339,6 +339,36 @@ def test_client_debug_metrics_enabled_property(self): self.assertFalse(client_disabled_base.metrics_enabled) self.assertTrue(client_disabled_base.debug_metrics_enabled) + def test_ctor_metrics_defaults_and_env_fallback(self): + from google.cloud.storage import _opentelemetry_metrics + + PROJECT = "PROJECT" + credentials = _make_credentials() + client = self._make_one(project=PROJECT, credentials=credentials) + + self.assertIsNone(client._enable_metrics) + self.assertIsNone(client._enable_debug_metrics) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + with mock.patch.dict("os.environ", {}, clear=True): + self.assertFalse(client.metrics_enabled) + self.assertFalse(client.debug_metrics_enabled) + + with mock.patch.dict( + "os.environ", + { + "GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS": "true", + "GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS": "true", + }, + clear=True, + ): + self.assertTrue(client.metrics_enabled) + self.assertTrue(client.debug_metrics_enabled) + def test_ctor_w_universe_domain_and_mismatched_credentials(self): PROJECT = "PROJECT" universe_domain = "example.com" diff --git a/packages/google-cloud-storage/tests/unit/test_grpc_client.py b/packages/google-cloud-storage/tests/unit/test_grpc_client.py index b243fcf4e7a8..8c367d80e77d 100644 --- a/packages/google-cloud-storage/tests/unit/test_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/test_grpc_client.py @@ -361,13 +361,11 @@ def test_rewrite_host_for_interconnect_delimiters(self): expected_host, ) - @mock.patch("google.cloud.storage.grpc_client.ClientWithProject") @mock.patch("google.cloud._storage_v2.StorageClient") - def test_metrics_properties(self, mock_storage_client, mock_base_client): + def test_metrics_properties(self, mock_storage_client): from google.cloud.storage import _opentelemetry_metrics mock_creds = _make_credentials() - mock_base_client.return_value._credentials = mock_creds client = grpc_client.GrpcClient( project="test-project", @@ -400,3 +398,36 @@ def test_metrics_properties(self, mock_storage_client, mock_base_client): client._enable_debug_metrics = True self.assertFalse(client.metrics_enabled) self.assertFalse(client.debug_metrics_enabled) + + @mock.patch("google.cloud._storage_v2.StorageClient") + def test_ctor_metrics_defaults_and_env_fallback(self, mock_storage_client): + from google.cloud.storage import _opentelemetry_metrics + + mock_creds = _make_credentials() + client = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + ) + + self.assertIsNone(client._enable_metrics) + self.assertIsNone(client._enable_debug_metrics) + + with mock.patch.multiple( + _opentelemetry_metrics, + HAS_OPENTELEMETRY_METRICS=True, + _ENABLE_METRICS_DEV_GATE=True, + ): + with mock.patch.dict("os.environ", {}, clear=True): + self.assertFalse(client.metrics_enabled) + self.assertFalse(client.debug_metrics_enabled) + + with mock.patch.dict( + "os.environ", + { + "GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS": "true", + "GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS": "true", + }, + clear=True, + ): + self.assertTrue(client.metrics_enabled) + self.assertTrue(client.debug_metrics_enabled) From e94ee0adbfaa7e458b713f1c961445f2b152759a Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Thu, 15 Oct 2026 19:31:48 +0000 Subject: [PATCH 4/5] fix(storage): evaluate metrics gating in client __init__ and pass extra_headers via kwargs in _LazyClient [Generated-by: AI] --- .../storage/asyncio/async_grpc_client.py | 14 +++- .../google/cloud/storage/client.py | 14 +++- .../google/cloud/storage/grpc_client.py | 14 +++- .../google/cloud/storage/transfer_manager.py | 14 ++-- .../unit/asyncio/test_async_grpc_client.py | 65 +++++++++++----- .../tests/unit/test_client.py | 61 ++++++++++----- .../tests/unit/test_grpc_client.py | 77 ++++++++++++++----- .../tests/unit/test_transfer_manager.py | 15 +++- 8 files changed, 197 insertions(+), 77 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py index 3613e4eb83ae..1cc42fbfe1fe 100644 --- a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py @@ -101,6 +101,14 @@ def __init__( ): self._enable_metrics = enable_metrics self._enable_debug_metrics = enable_debug_metrics + self._metrics_enabled = _opentelemetry_metrics.is_metrics_enabled( + enable_metrics + ) + self._debug_metrics_enabled = ( + _opentelemetry_metrics.is_debug_metrics_enabled( + enable_debug_metrics + ) + ) if isinstance(credentials, auth_credentials.AnonymousCredentials): if client_options is None or client_options.api_endpoint is None: @@ -136,14 +144,12 @@ def __init__( @property def metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" - return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) + return self._metrics_enabled @property def debug_metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if debug metrics recording is active for this client.""" - return _opentelemetry_metrics.is_debug_metrics_enabled( - self._enable_debug_metrics - ) + return self._debug_metrics_enabled def _create_anonymous_client(self, client_options, credentials): channel = grpc.aio.insecure_channel(client_options.api_endpoint) diff --git a/packages/google-cloud-storage/google/cloud/storage/client.py b/packages/google-cloud-storage/google/cloud/storage/client.py index 07cb2af468ef..4a5feb198456 100644 --- a/packages/google-cloud-storage/google/cloud/storage/client.py +++ b/packages/google-cloud-storage/google/cloud/storage/client.py @@ -181,6 +181,14 @@ def __init__( self._extra_headers = extra_headers self._enable_metrics = enable_metrics self._enable_debug_metrics = enable_debug_metrics + self._metrics_enabled = _opentelemetry_metrics.is_metrics_enabled( + enable_metrics + ) + self._debug_metrics_enabled = ( + _opentelemetry_metrics.is_debug_metrics_enabled( + enable_debug_metrics + ) + ) connection_kw_args = {"client_info": client_info} @@ -315,14 +323,12 @@ def __init__( @property def metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" - return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) + return self._metrics_enabled @property def debug_metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if debug metrics recording is active for this client.""" - return _opentelemetry_metrics.is_debug_metrics_enabled( - self._enable_debug_metrics - ) + return self._debug_metrics_enabled def close(self): """Close the client and clear any cached metadata or active connections.""" diff --git a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py index 988109be9122..160a1e597168 100644 --- a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py @@ -136,6 +136,14 @@ def __init__( self._enable_metrics = enable_metrics self._enable_debug_metrics = enable_debug_metrics + self._metrics_enabled = _opentelemetry_metrics.is_metrics_enabled( + enable_metrics + ) + self._debug_metrics_enabled = ( + _opentelemetry_metrics.is_debug_metrics_enabled( + enable_debug_metrics + ) + ) if isinstance(client_options, dict): if api_key: @@ -160,14 +168,12 @@ def __init__( @property def metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" - return _opentelemetry_metrics.is_metrics_enabled(self._enable_metrics) + return self._metrics_enabled @property def debug_metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if debug metrics recording is active for this client.""" - return _opentelemetry_metrics.is_debug_metrics_enabled( - self._enable_debug_metrics - ) + return self._debug_metrics_enabled def _create_gapic_client( self, diff --git a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py index 974f0aac5a8c..b376c5138d72 100644 --- a/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py +++ b/packages/google-cloud-storage/google/cloud/storage/transfer_manager.py @@ -1458,10 +1458,10 @@ def _digest_ordered_checksum_and_size_pairs(checksum_and_size_pairs): # Number of positional Client constructor arguments passed after `id` by -# `_reduce_client`: (project, credentials, _http, client_info, client_options, -# extra_headers). Trailing items in `args` correspond to keyword-only +# `_reduce_client`: (project, credentials, _http, client_info, client_options). +# Trailing items in `args` correspond to `extra_headers` and keyword-only # `enable_metrics` and `enable_debug_metrics` parameters. -_REDUCE_CLIENT_POS_ARGS = 6 +_REDUCE_CLIENT_POS_ARGS = 5 class _LazyClient: @@ -1473,10 +1473,14 @@ def __new__(cls, id, *args, **kwargs): return cached_client else: if len(args) > _REDUCE_CLIENT_POS_ARGS: - kwargs.setdefault("enable_metrics", args[_REDUCE_CLIENT_POS_ARGS]) + kwargs.setdefault("extra_headers", args[_REDUCE_CLIENT_POS_ARGS]) if len(args) > _REDUCE_CLIENT_POS_ARGS + 1: kwargs.setdefault( - "enable_debug_metrics", args[_REDUCE_CLIENT_POS_ARGS + 1] + "enable_metrics", args[_REDUCE_CLIENT_POS_ARGS + 1] + ) + if len(args) > _REDUCE_CLIENT_POS_ARGS + 2: + kwargs.setdefault( + "enable_debug_metrics", args[_REDUCE_CLIENT_POS_ARGS + 2] ) args = args[:_REDUCE_CLIENT_POS_ARGS] cached_client = Client(*args, **kwargs) diff --git a/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py b/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py index 494746266647..0b96a92eb055 100644 --- a/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/asyncio/test_async_grpc_client.py @@ -157,37 +157,67 @@ def test_metrics_properties(self, mock_async_storage_client): mock_async_storage_client.get_transport_class.return_value = mock_transport_cls mock_creds = _make_credentials() - client = async_grpc_client.AsyncGrpcClient( - credentials=mock_creds, - enable_metrics=True, - enable_debug_metrics=True, - ) - with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=True, ): + client = async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_metrics=True, + enable_debug_metrics=True, + ) assert client.metrics_enabled is True assert client.debug_metrics_enabled is True - client._enable_metrics = False - assert client.metrics_enabled is False - assert client.debug_metrics_enabled is True + client_base_false = async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_metrics=False, + enable_debug_metrics=True, + ) + assert client_base_false.metrics_enabled is False + assert client_base_false.debug_metrics_enabled is True - client._enable_debug_metrics = False - assert client.debug_metrics_enabled is False + client_both_false = async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_metrics=False, + enable_debug_metrics=False, + ) + assert client_both_false.metrics_enabled is False + assert client_both_false.debug_metrics_enabled is False with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=False, ): - client._enable_metrics = True - client._enable_debug_metrics = True + client = async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_metrics=True, + enable_debug_metrics=True, + ) assert client.metrics_enabled is False assert client.debug_metrics_enabled is False + @mock.patch("google.cloud._storage_v2.StorageAsyncClient") + def test_ctor_w_invalid_enable_metrics_raises_type_error( + self, mock_async_storage_client + ): + mock_transport_cls = mock.MagicMock() + mock_async_storage_client.get_transport_class.return_value = mock_transport_cls + mock_creds = _make_credentials() + + with pytest.raises(TypeError): + async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_metrics="true", + ) + with pytest.raises(TypeError): + async_grpc_client.AsyncGrpcClient( + credentials=mock_creds, + enable_debug_metrics="true", + ) + @mock.patch("google.cloud._storage_v2.StorageAsyncClient") def test_ctor_metrics_defaults_and_env_fallback(self, mock_async_storage_client): from google.cloud.storage import _opentelemetry_metrics @@ -196,17 +226,15 @@ def test_ctor_metrics_defaults_and_env_fallback(self, mock_async_storage_client) mock_async_storage_client.get_transport_class.return_value = mock_transport_cls mock_creds = _make_credentials() - client = async_grpc_client.AsyncGrpcClient(credentials=mock_creds) - - assert client._enable_metrics is None - assert client._enable_debug_metrics is None - with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=True, ): with mock.patch.dict("os.environ", {}, clear=True): + client = async_grpc_client.AsyncGrpcClient(credentials=mock_creds) + assert client._enable_metrics is None + assert client._enable_debug_metrics is None assert client.metrics_enabled is False assert client.debug_metrics_enabled is False @@ -218,6 +246,7 @@ def test_ctor_metrics_defaults_and_env_fallback(self, mock_async_storage_client) }, clear=True, ): + client = async_grpc_client.AsyncGrpcClient(credentials=mock_creds) assert client.metrics_enabled is True assert client.debug_metrics_enabled is True diff --git a/packages/google-cloud-storage/tests/unit/test_client.py b/packages/google-cloud-storage/tests/unit/test_client.py index 6ba7a6d51e91..672bde151053 100644 --- a/packages/google-cloud-storage/tests/unit/test_client.py +++ b/packages/google-cloud-storage/tests/unit/test_client.py @@ -275,20 +275,34 @@ def test_ctor_w_enable_debug_metrics(self): ) self.assertTrue(client._enable_debug_metrics) + def test_ctor_w_invalid_enable_metrics_raises_type_error(self): + PROJECT = "PROJECT" + credentials = _make_credentials() + with self.assertRaises(TypeError): + self._make_one( + project=PROJECT, credentials=credentials, enable_metrics="true" + ) + with self.assertRaises(TypeError): + self._make_one( + project=PROJECT, + credentials=credentials, + enable_debug_metrics="true", + ) + def test_client_metrics_enabled_property(self): from google.cloud.storage import _opentelemetry_metrics PROJECT = "PROJECT" credentials = _make_credentials() - client = self._make_one( - project=PROJECT, credentials=credentials, enable_metrics=True - ) with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=True, ): + client = self._make_one( + project=PROJECT, credentials=credentials, enable_metrics=True + ) self.assertTrue(client.metrics_enabled) with mock.patch.multiple( @@ -296,6 +310,9 @@ def test_client_metrics_enabled_property(self): HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=False, ): + client = self._make_one( + project=PROJECT, credentials=credentials, enable_metrics=True + ) self.assertFalse(client.metrics_enabled) def test_client_debug_metrics_enabled_property(self): @@ -303,18 +320,18 @@ def test_client_debug_metrics_enabled_property(self): PROJECT = "PROJECT" credentials = _make_credentials() - client = self._make_one( - project=PROJECT, - credentials=credentials, - enable_metrics=True, - enable_debug_metrics=True, - ) with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=True, ): + client = self._make_one( + project=PROJECT, + credentials=credentials, + enable_metrics=True, + enable_debug_metrics=True, + ) self.assertTrue(client.debug_metrics_enabled) with mock.patch.multiple( @@ -322,20 +339,26 @@ def test_client_debug_metrics_enabled_property(self): HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=False, ): + client = self._make_one( + project=PROJECT, + credentials=credentials, + enable_metrics=True, + enable_debug_metrics=True, + ) self.assertFalse(client.debug_metrics_enabled) # Verify that debug metrics are independent when base metrics are disabled - client_disabled_base = self._make_one( - project=PROJECT, - credentials=credentials, - enable_metrics=False, - enable_debug_metrics=True, - ) with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=True, ): + client_disabled_base = self._make_one( + project=PROJECT, + credentials=credentials, + enable_metrics=False, + enable_debug_metrics=True, + ) self.assertFalse(client_disabled_base.metrics_enabled) self.assertTrue(client_disabled_base.debug_metrics_enabled) @@ -344,10 +367,6 @@ def test_ctor_metrics_defaults_and_env_fallback(self): PROJECT = "PROJECT" credentials = _make_credentials() - client = self._make_one(project=PROJECT, credentials=credentials) - - self.assertIsNone(client._enable_metrics) - self.assertIsNone(client._enable_debug_metrics) with mock.patch.multiple( _opentelemetry_metrics, @@ -355,6 +374,9 @@ def test_ctor_metrics_defaults_and_env_fallback(self): _ENABLE_METRICS_DEV_GATE=True, ): with mock.patch.dict("os.environ", {}, clear=True): + client = self._make_one(project=PROJECT, credentials=credentials) + self.assertIsNone(client._enable_metrics) + self.assertIsNone(client._enable_debug_metrics) self.assertFalse(client.metrics_enabled) self.assertFalse(client.debug_metrics_enabled) @@ -366,6 +388,7 @@ def test_ctor_metrics_defaults_and_env_fallback(self): }, clear=True, ): + client = self._make_one(project=PROJECT, credentials=credentials) self.assertTrue(client.metrics_enabled) self.assertTrue(client.debug_metrics_enabled) diff --git a/packages/google-cloud-storage/tests/unit/test_grpc_client.py b/packages/google-cloud-storage/tests/unit/test_grpc_client.py index 8c367d80e77d..f4ae7a21020c 100644 --- a/packages/google-cloud-storage/tests/unit/test_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/test_grpc_client.py @@ -367,50 +367,75 @@ def test_metrics_properties(self, mock_storage_client): mock_creds = _make_credentials() - client = grpc_client.GrpcClient( - project="test-project", - credentials=mock_creds, - enable_metrics=True, - enable_debug_metrics=True, - ) - with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=True, ): + client = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_metrics=True, + enable_debug_metrics=True, + ) self.assertTrue(client.metrics_enabled) self.assertTrue(client.debug_metrics_enabled) - client._enable_metrics = False - self.assertFalse(client.metrics_enabled) - self.assertTrue(client.debug_metrics_enabled) + client_base_false = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_metrics=False, + enable_debug_metrics=True, + ) + self.assertFalse(client_base_false.metrics_enabled) + self.assertTrue(client_base_false.debug_metrics_enabled) - client._enable_debug_metrics = False - self.assertFalse(client.debug_metrics_enabled) + client_both_false = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_metrics=False, + enable_debug_metrics=False, + ) + self.assertFalse(client_both_false.metrics_enabled) + self.assertFalse(client_both_false.debug_metrics_enabled) with mock.patch.multiple( _opentelemetry_metrics, HAS_OPENTELEMETRY_METRICS=True, _ENABLE_METRICS_DEV_GATE=False, ): - client._enable_metrics = True - client._enable_debug_metrics = True + client = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_metrics=True, + enable_debug_metrics=True, + ) self.assertFalse(client.metrics_enabled) self.assertFalse(client.debug_metrics_enabled) + @mock.patch("google.cloud._storage_v2.StorageClient") + def test_ctor_w_invalid_enable_metrics_raises_type_error( + self, mock_storage_client + ): + mock_creds = _make_credentials() + with self.assertRaises(TypeError): + grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_metrics="true", + ) + with self.assertRaises(TypeError): + grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + enable_debug_metrics="true", + ) + @mock.patch("google.cloud._storage_v2.StorageClient") def test_ctor_metrics_defaults_and_env_fallback(self, mock_storage_client): from google.cloud.storage import _opentelemetry_metrics mock_creds = _make_credentials() - client = grpc_client.GrpcClient( - project="test-project", - credentials=mock_creds, - ) - - self.assertIsNone(client._enable_metrics) - self.assertIsNone(client._enable_debug_metrics) with mock.patch.multiple( _opentelemetry_metrics, @@ -418,6 +443,12 @@ def test_ctor_metrics_defaults_and_env_fallback(self, mock_storage_client): _ENABLE_METRICS_DEV_GATE=True, ): with mock.patch.dict("os.environ", {}, clear=True): + client = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + ) + self.assertIsNone(client._enable_metrics) + self.assertIsNone(client._enable_debug_metrics) self.assertFalse(client.metrics_enabled) self.assertFalse(client.debug_metrics_enabled) @@ -429,5 +460,9 @@ def test_ctor_metrics_defaults_and_env_fallback(self, mock_storage_client): }, clear=True, ): + client = grpc_client.GrpcClient( + project="test-project", + credentials=mock_creds, + ) self.assertTrue(client.metrics_enabled) self.assertTrue(client.debug_metrics_enabled) diff --git a/packages/google-cloud-storage/tests/unit/test_transfer_manager.py b/packages/google-cloud-storage/tests/unit/test_transfer_manager.py index b008b6c7e74d..c370f2eede2a 100644 --- a/packages/google-cloud-storage/tests/unit/test_transfer_manager.py +++ b/packages/google-cloud-storage/tests/unit/test_transfer_manager.py @@ -1420,7 +1420,8 @@ def test__reduce_client(): assert args[-2:] == (True, True) replicated_client(*args) mock_client_cls.assert_called_once_with( - *args[1:7], + *args[1:6], + extra_headers=custom_headers, enable_metrics=True, enable_debug_metrics=True, ) @@ -1430,10 +1431,20 @@ def test__reduce_client(): mock_client_cls.reset_mock() replicated_client(*args[:-1]) mock_client_cls.assert_called_once_with( - *args[1:7], + *args[1:6], + extra_headers=custom_headers, enable_metrics=True, ) + # Verify compatibility when only extra_headers is present (len(args) == 6 after id) + fake_cache.clear() + mock_client_cls.reset_mock() + replicated_client(*args[:-2]) + mock_client_cls.assert_called_once_with( + *args[1:6], + extra_headers=custom_headers, + ) + def test__call_method_on_maybe_pickled_blob(): blob = mock.Mock(spec=Blob) From f212c67d64d2d0abca16ad56e57b1025717a0823 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Fri, 16 Oct 2026 08:25:53 +0000 Subject: [PATCH 5/5] style(storage): format client and test files with ruff [Generated-by: AI] --- .../google/cloud/storage/asyncio/async_grpc_client.py | 6 ++---- .../google-cloud-storage/google/cloud/storage/client.py | 6 ++---- .../google/cloud/storage/grpc_client.py | 6 ++---- .../google-cloud-storage/tests/unit/test_grpc_client.py | 4 +--- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py index 1cc42fbfe1fe..21278cd624bf 100644 --- a/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/asyncio/async_grpc_client.py @@ -104,10 +104,8 @@ def __init__( self._metrics_enabled = _opentelemetry_metrics.is_metrics_enabled( enable_metrics ) - self._debug_metrics_enabled = ( - _opentelemetry_metrics.is_debug_metrics_enabled( - enable_debug_metrics - ) + self._debug_metrics_enabled = _opentelemetry_metrics.is_debug_metrics_enabled( + enable_debug_metrics ) if isinstance(credentials, auth_credentials.AnonymousCredentials): diff --git a/packages/google-cloud-storage/google/cloud/storage/client.py b/packages/google-cloud-storage/google/cloud/storage/client.py index 4a5feb198456..f9d354a4d269 100644 --- a/packages/google-cloud-storage/google/cloud/storage/client.py +++ b/packages/google-cloud-storage/google/cloud/storage/client.py @@ -184,10 +184,8 @@ def __init__( self._metrics_enabled = _opentelemetry_metrics.is_metrics_enabled( enable_metrics ) - self._debug_metrics_enabled = ( - _opentelemetry_metrics.is_debug_metrics_enabled( - enable_debug_metrics - ) + self._debug_metrics_enabled = _opentelemetry_metrics.is_debug_metrics_enabled( + enable_debug_metrics ) connection_kw_args = {"client_info": client_info} diff --git a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py index 160a1e597168..79bcb4f0d8b1 100644 --- a/packages/google-cloud-storage/google/cloud/storage/grpc_client.py +++ b/packages/google-cloud-storage/google/cloud/storage/grpc_client.py @@ -139,10 +139,8 @@ def __init__( self._metrics_enabled = _opentelemetry_metrics.is_metrics_enabled( enable_metrics ) - self._debug_metrics_enabled = ( - _opentelemetry_metrics.is_debug_metrics_enabled( - enable_debug_metrics - ) + self._debug_metrics_enabled = _opentelemetry_metrics.is_debug_metrics_enabled( + enable_debug_metrics ) if isinstance(client_options, dict): diff --git a/packages/google-cloud-storage/tests/unit/test_grpc_client.py b/packages/google-cloud-storage/tests/unit/test_grpc_client.py index f4ae7a21020c..5e3789549696 100644 --- a/packages/google-cloud-storage/tests/unit/test_grpc_client.py +++ b/packages/google-cloud-storage/tests/unit/test_grpc_client.py @@ -414,9 +414,7 @@ def test_metrics_properties(self, mock_storage_client): self.assertFalse(client.debug_metrics_enabled) @mock.patch("google.cloud._storage_v2.StorageClient") - def test_ctor_w_invalid_enable_metrics_raises_type_error( - self, mock_storage_client - ): + def test_ctor_w_invalid_enable_metrics_raises_type_error(self, mock_storage_client): mock_creds = _make_credentials() with self.assertRaises(TypeError): grpc_client.GrpcClient(