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
Original file line number Diff line number Diff line change
Expand Up @@ -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

_DEFAULT_HOST = "storage.googleapis.com"

Expand Down Expand Up @@ -63,6 +63,19 @@ class AsyncGrpcClient:
:param attempt_direct_path:
(Optional) Whether to attempt to use DirectPath for gRPC connections.
Defaults to ``True``.

: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__(
Expand All @@ -72,7 +85,18 @@ def __init__(
client_options=None,
*,
attempt_direct_path=True,
enable_metrics=None,
enable_debug_metrics=None,
):
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:
raise ValueError(
Expand All @@ -99,6 +123,16 @@ def __init__(
attempt_direct_path=attempt_direct_path,
)

@property
def metrics_enabled(self) -> bool:
"""(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client."""
return self._metrics_enabled

@property
def debug_metrics_enabled(self) -> bool:
"""(Experimental) Returns ``True`` if debug metrics recording is active for this client."""
return self._debug_metrics_enabled

def _create_anonymous_client(self, client_options, credentials):
channel = grpc.aio.insecure_channel(client_options.api_endpoint)
transport = storage_v2.services.storage.transports.StorageGrpcAsyncIOTransport(
Expand Down
34 changes: 34 additions & 0 deletions packages/google-cloud-storage/google/cloud/storage/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 = (
Expand All @@ -146,6 +160,8 @@ def __init__(
extra_headers={},
*,
api_key=None,
enable_metrics=None,
enable_debug_metrics=None,
):
self._base_connection = None

Expand All @@ -163,6 +179,14 @@ def __init__(
self._initial_client_info = client_info
self._initial_client_options = client_options
self._extra_headers = extra_headers
self._enable_metrics = enable_metrics

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since is_metrics_enabled() is only called lazily inside the properties, Client(enable_metrics="false") doesn't raise TypeError at construction time, and every property access re-parses os.environ. Can we evaluate self._metrics_enabled and self._debug_metrics_enabled once in __init__ (same in GrpcClient and AsyncGrpcClient), while keeping self._enable_metrics / self._enable_debug_metrics for _reduce_client?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Co-authored by AI Agent

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}

Expand Down Expand Up @@ -294,6 +318,16 @@ def __init__(
self._batch_stack = _LocalStack()
self._bucket_metadata_cache = BucketMetadataCache(self)

@property
def metrics_enabled(self) -> bool:
"""(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client."""
return self._metrics_enabled

@property
def debug_metrics_enabled(self) -> bool:
"""(Experimental) Returns ``True`` if debug metrics recording is active for this client."""
return self._debug_metrics_enabled

def close(self):
"""Close the client and clear any cached metadata or active connections."""
if hasattr(self, "_bucket_metadata_cache") and self._bucket_metadata_cache:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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()

Expand Down Expand Up @@ -58,6 +59,19 @@ class GrpcClient(ClientWithProject):
This provides a direct, unproxied connection to GCS for lower latency
and higher throughput, and is highly recommended when running on Google
Cloud infrastructure. Defaults to ``True``.

: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__(
Expand All @@ -69,9 +83,20 @@ def __init__(
*,
api_key=None,
attempt_direct_path=True,
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
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:
client_options["api_key"] = api_key
Expand All @@ -87,6 +112,16 @@ def __init__(
attempt_direct_path=attempt_direct_path,
)

@property
def metrics_enabled(self) -> bool:
"""(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client."""
return self._metrics_enabled

@property
def debug_metrics_enabled(self) -> bool:
"""(Experimental) Returns ``True`` if debug metrics recording is active for this client."""
return self._debug_metrics_enabled

def _create_gapic_client(
self,
credentials=None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1392,6 +1394,8 @@ def _reduce_client(cl):
client_info,
client_options,
extra_headers,
enable_metrics,
enable_debug_metrics,
)


Expand Down Expand Up @@ -1453,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).
# Trailing items in `args` correspond to `extra_headers` and keyword-only
# `enable_metrics` and `enable_debug_metrics` parameters.
_REDUCE_CLIENT_POS_ARGS = 5


class _LazyClient:
"""An object that will transform into either a cached or a new Client"""

Expand All @@ -1461,6 +1472,17 @@ def __new__(cls, id, *args, **kwargs):
if cached_client:
return cached_client
else:
if len(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_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)
_cached_clients[id] = cached_client
return cached_client
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,107 @@ 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()

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_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_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 = 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

mock_transport_cls = mock.MagicMock()
mock_async_storage_client.get_transport_class.return_value = mock_transport_cls
mock_creds = _make_credentials()

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

with mock.patch.dict(
"os.environ",
{
"GCP_STORAGE_PYTHON_ENABLE_OTEL_METRICS": "true",
"GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS": "true",
},
clear=True,
):
client = async_grpc_client.AsyncGrpcClient(credentials=mock_creds)
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
Expand Down
Loading
Loading