From 43d1e94caf36dce78162e9fe38a7c6c2fd1dd47f Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Thu, 15 Oct 2026 15:44:24 +0000 Subject: [PATCH 1/7] feat(storage): add OpenTelemetry standard metrics recorder, views, and helpers Implement core OpenTelemetry standard metrics infrastructure for GCS: - Add _MetricsRecorder with the 9 standard metric instruments - Add custom latency (165 buckets) and byte-size (45 buckets) histogram views - Add classify_error_type for low-cardinality error.type mapping - Add normalize_url_template for bounded HTTP url.template routing - Add start_operation ContextVar for outermost logical operation tracking and nested call suppression [Generated-by: AI] --- .../cloud/storage/_opentelemetry_metrics.py | 959 ++++++++++++++++- .../tests/unit/test__opentelemetry_metrics.py | 981 +++++++++++++++++- 2 files changed, 1936 insertions(+), 4 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py index 8592cc24c65d..c357a968407f 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py +++ b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py @@ -14,9 +14,21 @@ """Manages OpenTelemetry metrics instruments and gating for GCS client.""" +import asyncio +from collections import deque +import concurrent.futures +import contextlib +from contextvars import ContextVar +from dataclasses import dataclass +import http.client import logging import os -from typing import Any, Dict, Optional +import re +import socket +import ssl +import time +from typing import Any, Deque, Dict, Iterator, List, Optional, Set, Tuple +from urllib.parse import urlsplit from google.cloud.storage.version import __version__ @@ -38,8 +50,35 @@ _DEFAULT_ENABLE_METRICS = False _DEFAULT_ENABLE_DEBUG_METRICS = False +DEFAULT_SERVER_ADDRESS = "storage.googleapis.com" + +# --------------------------------------------------------------------------- +# 3. Standard Metric Instrument Names # --------------------------------------------------------------------------- -# 3. Optional OpenTelemetry Dependency Check +METRIC_RPC_CLIENT_CALL_DURATION = "rpc.client.call.duration" +METRIC_HTTP_CLIENT_REQUEST_DURATION = "http.client.request.duration" +METRIC_GCP_CLIENT_REQUEST_DURATION = "gcp.client.request.duration" +METRIC_STORAGE_CLIENT_OPERATIONS = "gcp.storage.client.operations" +METRIC_STORAGE_CLIENT_ATTEMPTS = "gcp.storage.client.attempts" +METRIC_STORAGE_REQUEST_BODY_SIZE = "gcp.storage.client.request.body.size" +METRIC_STORAGE_RESPONSE_BODY_SIZE = "gcp.storage.client.response.body.size" +METRIC_STORAGE_OPERATION_TTFB = "gcp.storage.client.operation.ttfb" +METRIC_STORAGE_CLIENT_ERRORS = "gcp.storage.client.errors" + +_LATENCY_METRIC_NAMES = ( + METRIC_RPC_CLIENT_CALL_DURATION, + METRIC_HTTP_CLIENT_REQUEST_DURATION, + METRIC_GCP_CLIENT_REQUEST_DURATION, + METRIC_STORAGE_OPERATION_TTFB, +) + +_SIZE_METRIC_NAMES = ( + METRIC_STORAGE_REQUEST_BODY_SIZE, + METRIC_STORAGE_RESPONSE_BODY_SIZE, +) + +# --------------------------------------------------------------------------- +# 4. Optional OpenTelemetry Dependency Check # --------------------------------------------------------------------------- try: from opentelemetry import metrics @@ -134,7 +173,7 @@ def is_debug_metrics_enabled( # --------------------------------------------------------------------------- -# 4. Standard Common Attributes & Meter Provider +# 5. Standard Common Attributes & Meter Provider # --------------------------------------------------------------------------- _COMMON_ATTRIBUTES: Dict[str, Any] = { "gcp.client.service": "storage", @@ -178,3 +217,917 @@ def get_meter( __version__, meter_provider=meter_provider, ) + + +# --------------------------------------------------------------------------- +# 6. Custom Histogram Bucket Boundaries & Views +# --------------------------------------------------------------------------- +def _build_latency_buckets() -> Tuple[float, ...]: + """Builds the custom latency histogram bucket boundaries in seconds. + + Uses 2ms resolution for the first 50 buckets (0.000s to 0.098s), then starts + at 0.100s with 10ms resolution and doubles the increment every 10 buckets + while boundary < 300.0s. + """ + boundaries: List[float] = [] + boundary = 0.0 + increment = 0.002 + + for _ in range(50): + boundaries.append(round(boundary, 6)) + boundary += increment + + increment = 0.010 + for i in range(150): + if boundary >= 300.0: + break + if i != 0 and i % 10 == 0: + increment *= 2 + boundaries.append(round(boundary, 6)) + boundary += increment + + return tuple(boundaries) + + +def _build_size_buckets() -> Tuple[int, ...]: + """Builds the custom byte-size histogram bucket boundaries in bytes. + + Uses 128 KiB resolution from 0 up to 4 MiB (33 buckets), then doubles every + bucket up to 16 GiB (12 buckets, 45 total buckets). + """ + kb = 1024 + mb = 1024 * kb + gb = 1024 * mb + + boundaries: List[int] = [] + boundary = 0 + increment = 128 * kb + + while boundary <= 16 * gb: + boundaries.append(boundary) + if boundary < 4 * mb: + boundary += increment + else: + boundary *= 2 + + return tuple(boundaries) + + +LATENCY_HISTOGRAM_BOUNDARIES: Tuple[float, ...] = _build_latency_buckets() +SIZE_HISTOGRAM_BOUNDARIES: Tuple[int, ...] = _build_size_buckets() + + +def get_histogram_views() -> List[Any]: + """Returns OpenTelemetry SDK Views for custom latency and size histograms. + + If ``opentelemetry-sdk`` is not installed, returns an empty list. + """ + try: + from opentelemetry.sdk.metrics.view import ( + ExplicitBucketHistogramAggregation, + View, + ) + except ImportError: + return [] + + views: List[Any] = [] + for name in _LATENCY_METRIC_NAMES: + views.append( + View( + instrument_name=name, + aggregation=ExplicitBucketHistogramAggregation( + boundaries=LATENCY_HISTOGRAM_BOUNDARIES + ), + ) + ) + for name in _SIZE_METRIC_NAMES: + views.append( + View( + instrument_name=name, + aggregation=ExplicitBucketHistogramAggregation( + boundaries=SIZE_HISTOGRAM_BOUNDARIES + ), + ) + ) + return views + + +# --------------------------------------------------------------------------- +# 7. Canonical Error Classification (`error.type`) +# --------------------------------------------------------------------------- +_HTTP_STATUS_TO_ERROR_TYPE: Dict[int, str] = { + 400: "INVALID_ARGUMENT", + 401: "UNAUTHENTICATED", + 403: "PERMISSION_DENIED", + 404: "NOT_FOUND", + 408: "DEADLINE_EXCEEDED", + 409: "ABORTED", + 412: "FAILED_PRECONDITION", + 416: "OUT_OF_RANGE", + 429: "RESOURCE_EXHAUSTED", + 499: "CANCELLED", + 500: "INTERNAL", + 501: "UNIMPLEMENTED", + 503: "UNAVAILABLE", + 504: "DEADLINE_EXCEEDED", +} + + +def _is_valid_status_code(val: Any) -> bool: + """Returns True if ``val`` is a valid HTTP error status code (int in 400..599).""" + return isinstance(val, int) and not isinstance(val, bool) and 400 <= val < 600 + + +def _iter_exception_chain(exc: BaseException) -> Iterator[BaseException]: + """Yields ``exc`` and any nested causes, contexts, reasons, or exception args.""" + visited: Set[int] = {id(exc)} + queue: Deque[BaseException] = deque([exc]) + while queue: + current = queue.popleft() + yield current + + for attr_name in ( + "__cause__", + "__context__", + "cause", + "reason", + "original_exception", + ): + nested = getattr(current, attr_name, None) + if isinstance(nested, BaseException) and id(nested) not in visited: + visited.add(id(nested)) + queue.append(nested) + + for arg in getattr(current, "args", ()): + if isinstance(arg, BaseException) and id(arg) not in visited: + visited.add(id(arg)) + queue.append(arg) + + +def _is_subclass_by_module( + cls: type, module_prefix: str, class_names: Optional[Set[str]] = None +) -> bool: + """Checks whether ``cls`` or any of its base classes match module/name criteria.""" + for base in getattr(cls, "__mro__", ()): + mod = getattr(base, "__module__", "") or "" + if mod.startswith(module_prefix): + if class_names is None or base.__name__ in class_names: + return True + return False + + +def classify_error_type( + exc: Optional[BaseException] = None, + status_code: Optional[int] = None, +) -> str: + """Maps an exception or HTTP status code to a low-cardinality ``error.type``. + + Args: + exc: Optional exception raised during an operation or transport attempt. + status_code: Optional HTTP status code from the response. + + Returns: + str: Canonical low-cardinality error classification string. + """ + valid_status = _is_valid_status_code(status_code) + if exc is None: + if not valid_status or status_code is None: + return "OK" + return _HTTP_STATUS_TO_ERROR_TYPE.get(status_code, str(status_code)) + + chain = list(_iter_exception_chain(exc)) + + # 1. Checksum Mismatch (DataCorruption) + for item in chain: + if _is_subclass_by_module( + item.__class__, "google.cloud.storage", {"DataCorruption"} + ) or _is_subclass_by_module( + item.__class__, "google.resumable_media", {"DataCorruption"} + ): + return "CHECKSUM_MISMATCH" + + # 2. Authentication / Credentials Failure (google.auth.exceptions) + for item in chain: + if _is_subclass_by_module( + item.__class__, + "google.auth.exceptions", + { + "RefreshError", + "TransportError", + "DefaultCredentialsError", + "GoogleAuthError", + }, + ): + return "AUTHENTICATION_ERROR" + + # 3. Cancellation + for item in chain: + if isinstance(item, asyncio.CancelledError): + return "CANCELLED" + + # 4. HTTP response status code on exception (checked before grpc_status_code so + # HTTP 409 Conflict consistently maps to ABORTED across HTTP and Veneer metrics) + for item in chain: + resp = getattr(item, "response", None) + resp_status = getattr(resp, "status_code", None) + if _is_valid_status_code(resp_status): + return _HTTP_STATUS_TO_ERROR_TYPE.get(resp_status, str(resp_status)) + + if item.__class__.__name__ == "Conflict" and _is_subclass_by_module( + item.__class__, "google.api_core.exceptions", {"Conflict"} + ): + return _HTTP_STATUS_TO_ERROR_TYPE[409] + + # 5. gRPC / GoogleAPICallError status code + for item in chain: + grpc_code = getattr(item, "grpc_status_code", None) + if grpc_code is not None: + name = getattr(grpc_code, "name", None) + if name: + return str(name) + return str(grpc_code) + + # Direct grpc.RpcError (has callable .code()) + code_attr = getattr(item, "code", None) + if callable(code_attr): + try: + rpc_status = code_attr() + if rpc_status is not None: + name = getattr(rpc_status, "name", None) + if name: + return str(name) + except Exception: # pragma: no cover + pass + + if _is_valid_status_code(code_attr): + return _HTTP_STATUS_TO_ERROR_TYPE.get(code_attr, str(code_attr)) + + # 6. TLS / SSL Failure + for item in chain: + if ( + isinstance(item, ssl.SSLError) + or _is_subclass_by_module( + item.__class__, + "requests.exceptions", + {"SSLError"}, + ) + or _is_subclass_by_module( + item.__class__, + "urllib3.exceptions", + {"SSLError"}, + ) + ): + return "TLS_FAILURE" + + # 7. Specific Socket / Network Failures (DNS, Connection Refused, Connection Reset) + for item in chain: + if isinstance(item, socket.gaierror) or _is_subclass_by_module( + item.__class__, + "urllib3.exceptions", + {"NameResolutionError"}, + ): + return "DNS_FAILURE" + if isinstance(item, ConnectionRefusedError): + return "CONNECTION_REFUSED" + if isinstance(item, ConnectionResetError): + return "CONNECTION_RESET" + + # 8. Timeouts (Note: urllib3.exceptions.NewConnectionError subclasses ConnectTimeoutError + # even for non-timeout connection failures, so exclude NewConnectionError here.) + for item in chain: + if item.__class__.__name__ != "NewConnectionError": + if isinstance( + item, + ( + socket.timeout, + TimeoutError, + asyncio.TimeoutError, + concurrent.futures.TimeoutError, + ), + ): + return "TIMEOUT" + if _is_subclass_by_module( + item.__class__, + "requests.exceptions", + {"Timeout", "ConnectTimeout", "ReadTimeout"}, + ) or _is_subclass_by_module( + item.__class__, + "urllib3.exceptions", + {"TimeoutError", "ReadTimeoutError", "ConnectTimeoutError"}, + ): + return "TIMEOUT" + + # 9. Generic Connectivity Failure + for item in chain: + if ( + _is_subclass_by_module( + item.__class__, + "requests.exceptions", + {"ConnectionError", "ChunkedEncodingError", "RequestException"}, + ) + or _is_subclass_by_module( + item.__class__, + "urllib3.exceptions", + {"HTTPError"}, + ) + or isinstance(item, (OSError, http.client.HTTPException)) + ): + return "CONNECTIVITY" + + # 10. Explicit HTTP status_code fallback + if valid_status and status_code is not None: + return _HTTP_STATUS_TO_ERROR_TYPE.get(status_code, str(status_code)) + + # 11. Client Validation Errors + for item in chain: + if ( + isinstance(item, (ValueError, TypeError)) + or _is_subclass_by_module( + item.__class__, "google.cloud.storage", {"InvalidResponse"} + ) + or _is_subclass_by_module( + item.__class__, "google.resumable_media", {"InvalidResponse"} + ) + ): + return "CLIENT_VALIDATION_ERROR" + + return "UNKNOWN" + + +# --------------------------------------------------------------------------- +# 8. HTTP `url.template` Route Normalization +# --------------------------------------------------------------------------- +_URL_TEMPLATE_PATTERNS: Tuple[Tuple[re.Pattern[str], str], ...] = ( + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/rewriteTo/b/[^/]+/o/.+$"), + "/storage/v1/b/{bucket}/o/{object}/rewriteTo/b/{bucket}/o/{object}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/copyTo/b/[^/]+/o/.+$"), + "/storage/v1/b/{bucket}/o/{object}/copyTo/b/{bucket}/o/{object}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/moveTo/o/.+$"), + "/storage/v1/b/{bucket}/o/{object}/moveTo/o/{object}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/compose$"), + "/storage/v1/b/{bucket}/o/{object}/compose", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/restore$"), + "/storage/v1/b/{bucket}/o/{object}/restore", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/acl/[^/]+$"), + "/storage/v1/b/{bucket}/o/{object}/acl/{entity}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/acl$"), + "/storage/v1/b/{bucket}/o/{object}/acl", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/[^/]+/iam(?:/.*)?$"), + "/storage/v1/b/{bucket}/o/{object}/iam", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o/.+$"), + "/storage/v1/b/{bucket}/o/{object}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/o$"), + "/storage/v1/b/{bucket}/o", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/acl/[^/]+$"), + "/storage/v1/b/{bucket}/acl/{entity}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/acl$"), + "/storage/v1/b/{bucket}/acl", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/defaultObjectAcl/[^/]+$"), + "/storage/v1/b/{bucket}/defaultObjectAcl/{entity}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/defaultObjectAcl$"), + "/storage/v1/b/{bucket}/defaultObjectAcl", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/iam(?:/.*)?$"), + "/storage/v1/b/{bucket}/iam", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/notificationConfigs/[^/]+$"), + "/storage/v1/b/{bucket}/notificationConfigs/{notification}", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/notificationConfigs$"), + "/storage/v1/b/{bucket}/notificationConfigs", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/lockRetentionPolicy$"), + "/storage/v1/b/{bucket}/lockRetentionPolicy", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+/restore$"), + "/storage/v1/b/{bucket}/restore", + ), + ( + re.compile(r"^/storage/v1/b/[^/]+$"), + "/storage/v1/b/{bucket}", + ), + ( + re.compile(r"^/storage/v1/b$"), + "/storage/v1/b", + ), + ( + re.compile(r"^/storage/v1/projects/[^/]+/serviceAccount$"), + "/storage/v1/projects/{project}/serviceAccount", + ), + ( + re.compile(r"^/storage/v1/projects/[^/]+/hmacKeys/[^/]+$"), + "/storage/v1/projects/{project}/hmacKeys/{access_id}", + ), + ( + re.compile(r"^/storage/v1/projects/[^/]+/hmacKeys$"), + "/storage/v1/projects/{project}/hmacKeys", + ), + ( + re.compile(r"^/upload/storage/v1/b/[^/]+/o(?:/[^/]+)?$"), + "/upload/storage/v1/b/{bucket}/o", + ), + ( + re.compile(r"^/download/storage/v1/b/[^/]+/o/.+$"), + "/download/storage/v1/b/{bucket}/o/{object}", + ), + ( + re.compile(r"^/batch/storage/v1$"), + "/batch/storage/v1", + ), +) + +_XML_MPU_RESERVED_PREFIXES = {"storage", "upload", "download", "batch", "b", "projects"} +_XML_MPU_QUERY_KEYS = {"uploads", "uploadId", "partNumber"} +_XML_MULTI_SEGMENT_RE = re.compile(r"^/([^/]+)/(.+)$") + + +def normalize_url_template(url_or_path: Optional[str]) -> str: + """Normalizes a raw HTTP URL or path into a bounded ``url.template`` string. + + Bucket names, object keys, project IDs, and query strings are stripped to + prevent cardinality explosion. Unrecognized paths fall back to ``"UNKNOWN"``. + + Args: + url_or_path: Full request URL or path string. + + Returns: + str: Low-cardinality ``url.template`` value. + """ + if not url_or_path or not isinstance(url_or_path, str): + return "UNKNOWN" + + parsed = urlsplit(url_or_path) + path = parsed.path or "" + query = parsed.query or "" + + path = "/" + path.strip("/") + if path == "/": + return "UNKNOWN" + + # Normalize relative paths passed to Connection.api_request before /storage/v1 prefixing + if path == "/b" or path.startswith(("/b/", "/projects/")): + path = "/storage/v1" + path + + for pattern, template in _URL_TEMPLATE_PATTERNS: + if pattern.match(path): + return template + + # XML API Multipart Upload routes: /{bucket}/{object} with XML MPU query params + if query: + query_keys = {param.partition("=")[0] for param in query.split("&") if param} + if query_keys & _XML_MPU_QUERY_KEYS: + match_multi = _XML_MULTI_SEGMENT_RE.match(path) + if match_multi and match_multi.group(1) not in _XML_MPU_RESERVED_PREFIXES: + return "/{bucket}/{object}" + + return "UNKNOWN" + + +# --------------------------------------------------------------------------- +# 9. Logical Operation Context & Suppression (`ContextVar`) +# --------------------------------------------------------------------------- +@dataclass(frozen=True) +class _OperationState: + """Represents the active logical GCS Veneer operation in the current context.""" + + client_method: str + rpc_method: str + suppressed: bool = False + + +_OP_STATE: ContextVar[Optional[_OperationState]] = ContextVar( + "_gcs_otel_op_state", default=None +) + + +def get_current_operation() -> Optional[_OperationState]: + """Returns the current logical `_OperationState` in this execution context, if any.""" + return _OP_STATE.get() + + +@contextlib.contextmanager +def start_operation( + recorder: Optional["_MetricsRecorder"], + client_method: str, + rpc_method: str, + suppress: bool = False, +) -> Iterator[Optional[_OperationState]]: + """Context manager tracking a logical user-facing GCS operation. + + If a parent logical operation is already active in the current context, + nested calls become a no-op so only the outermost user-invoked method + emits ``gcp.client.request.duration`` and ``gcp.storage.client.operations``. + + Args: + recorder: Optional `_MetricsRecorder` configured on the client. + client_method: Fully qualified Python method name + (e.g. ``"storage.Blob.download_to_filename"``). + rpc_method: Canonical GCS RPC operation name + (e.g. ``"storage.objects.get"``). + suppress: If True, suppresses logical operation and payload size + emission for this context and any nested calls (used by + ``transfer_manager`` workers and ``BlobReader`` prefetches). + """ + existing = _OP_STATE.get() + if existing is not None: + if suppress and not existing.suppressed: + suppressed_child = _OperationState( + client_method=existing.client_method, + rpc_method=existing.rpc_method, + suppressed=True, + ) + child_token = _OP_STATE.set(suppressed_child) + try: + yield suppressed_child + finally: + _OP_STATE.reset(child_token) + else: + yield existing + return + + recorder_active = recorder is not None and recorder.standard_enabled + if not recorder_active and not suppress: + yield None + return + + state = _OperationState( + client_method=client_method, + rpc_method=rpc_method, + suppressed=suppress, + ) + token = _OP_STATE.set(state) + start_time = time.perf_counter() + error_type = "OK" + try: + yield state + except BaseException as exc: + error_type = classify_error_type(exc) + raise + finally: + duration = max(0.0, time.perf_counter() - start_time) + _OP_STATE.reset(token) + if recorder_active and not suppress and recorder is not None: + recorder.record_logical_operation( + duration=duration, + client_method=client_method, + rpc_method=rpc_method, + error_type=error_type, + ) + + +# --------------------------------------------------------------------------- +# 10. Server Address Parsing & `_MetricsRecorder` +# --------------------------------------------------------------------------- +def _extract_server_address(api_endpoint: Optional[str]) -> str: + """Extracts the low-cardinality hostname from ``api_endpoint``.""" + if not api_endpoint or not isinstance(api_endpoint, str): + return DEFAULT_SERVER_ADDRESS + endpoint = api_endpoint.strip() + if not endpoint: + return DEFAULT_SERVER_ADDRESS + if "://" not in endpoint: + endpoint = "https://" + endpoint.lstrip("/") + parsed = urlsplit(endpoint) + return parsed.hostname or DEFAULT_SERVER_ADDRESS + + +class _MetricsRecorder: + """Holds OpenTelemetry metric instruments and records standard GCS metrics.""" + + def __init__( + self, + enable_metrics: Optional[bool] = None, + enable_debug_metrics: Optional[bool] = None, + meter_provider: Optional[Any] = None, + api_endpoint: Optional[str] = None, + rpc_system: str = "http", + ) -> None: + self.standard_enabled: bool = is_metrics_enabled(enable_metrics) + self.debug_enabled: bool = is_debug_metrics_enabled(enable_debug_metrics) + self.enabled: bool = self.standard_enabled or self.debug_enabled + self._meter_provider: Optional[Any] = meter_provider + self.rpc_system: str = rpc_system + self.server_address: str = _extract_server_address(api_endpoint) + self.meter: Optional[Any] = get_meter( + enable_metrics=enable_metrics, + enable_debug_metrics=enable_debug_metrics, + meter_provider=meter_provider, + ) + + self._base_attributes: Dict[str, Any] = { + **_COMMON_ATTRIBUTES, + "server.address": self.server_address, + "rpc.system.name": self.rpc_system, + } + + if self.standard_enabled and self.meter is not None: + self.rpc_call_duration = self.meter.create_histogram( + METRIC_RPC_CLIENT_CALL_DURATION, + unit="s", + description="Duration of an individual gRPC attempt in seconds (excluding retries).", + ) + self.http_request_duration = self.meter.create_histogram( + METRIC_HTTP_CLIENT_REQUEST_DURATION, + unit="s", + description="Duration of an individual HTTP request attempt in seconds (excluding retries).", + ) + self.client_request_duration = self.meter.create_histogram( + METRIC_GCP_CLIENT_REQUEST_DURATION, + unit="s", + description="End-to-end latency of a logical user-facing Veneer operation in seconds.", + ) + self.operations_counter = self.meter.create_counter( + METRIC_STORAGE_CLIENT_OPERATIONS, + unit="{operation}", + description="Number of logical GCS client operations.", + ) + self.attempts_counter = self.meter.create_counter( + METRIC_STORAGE_CLIENT_ATTEMPTS, + unit="{attempt}", + description="Number of physical network transport attempts executed.", + ) + self.request_body_size = self.meter.create_histogram( + METRIC_STORAGE_REQUEST_BODY_SIZE, + unit="By", + description="Application payload bytes submitted by the caller for an upload or write operation.", + ) + self.response_body_size = self.meter.create_histogram( + METRIC_STORAGE_RESPONSE_BODY_SIZE, + unit="By", + description="Application payload bytes delivered to the caller.", + ) + self.operation_ttfb = self.meter.create_histogram( + METRIC_STORAGE_OPERATION_TTFB, + unit="s", + description="Time elapsed from attempt initiation to receipt of the first response byte.", + ) + self.errors_counter = self.meter.create_counter( + METRIC_STORAGE_CLIENT_ERRORS, + unit="{error}", + description="Number of physical network transport attempts terminating with a non-OK status.", + ) + else: + self.rpc_call_duration = None + self.http_request_duration = None + self.client_request_duration = None + self.operations_counter = None + self.attempts_counter = None + self.request_body_size = None + self.response_body_size = None + self.operation_ttfb = None + self.errors_counter = None + + def get_base_attributes(self) -> Dict[str, Any]: + """Returns a copy of the base attributes for this recorder instance.""" + return self._base_attributes.copy() + + def _build_operation_attributes( + self, + operation: Optional[str] = None, + client_method: Optional[str] = None, + current_op: Optional[_OperationState] = None, + ) -> Dict[str, Any]: + """Builds base attributes enriched with current or explicit operation metadata.""" + attrs = self._base_attributes.copy() + if current_op is None: + current_op = _OP_STATE.get() + resolved_op = operation or (current_op.rpc_method if current_op else None) + resolved_method = client_method or ( + current_op.client_method if current_op else None + ) + if resolved_op is not None: + attrs["gcp.storage.operation"] = resolved_op + if resolved_method is not None: + attrs["gcp.client.method"] = resolved_method + return attrs + + def record_logical_operation( + self, + duration: float, + client_method: str, + rpc_method: str, + error_type: str = "OK", + ) -> None: + """Records end-to-end logical operation duration and count.""" + if not self.standard_enabled or self.meter is None: + return + attrs = { + **self._base_attributes, + "gcp.storage.operation": rpc_method, + "gcp.client.method": client_method, + "error.type": error_type, + } + self.client_request_duration.record(max(0.0, duration), attributes=attrs) + self.operations_counter.add(1, attributes=attrs) + + def record_http_attempt( + self, + duration: float, + http_method: str, + url_template: str, + status_code: Optional[int] = None, + error_type: str = "OK", + operation: Optional[str] = None, + client_method: Optional[str] = None, + network_path: Optional[str] = None, + ) -> None: + """Records a single physical HTTP request attempt.""" + if not self.standard_enabled or self.meter is None: + return + current_op = _OP_STATE.get() + resolved_op = operation or (current_op.rpc_method if current_op else None) + resolved_method = client_method or ( + current_op.client_method if current_op else None + ) + valid_code = ( + isinstance(status_code, int) + and not isinstance(status_code, bool) + and 100 <= status_code < 600 + ) + + # 1. http.client.request.duration attributes + http_attrs: Dict[str, Any] = { + **self._base_attributes, + "http.request.method": http_method.upper() if http_method else "UNKNOWN", + "url.template": url_template, + "error.type": error_type, + } + if valid_code: + http_attrs["http.response.status_code"] = status_code + if network_path is not None: + http_attrs["network.path"] = network_path + + # 2. gcp.storage.client.attempts attributes + attempt_attrs: Dict[str, Any] = { + **self._base_attributes, + "error.type": error_type, + } + if resolved_op is not None: + attempt_attrs["gcp.storage.operation"] = resolved_op + if valid_code: + attempt_attrs["http.response.status_code"] = status_code + if network_path is not None: + attempt_attrs["network.path"] = network_path + + self.http_request_duration.record(max(0.0, duration), attributes=http_attrs) + self.attempts_counter.add(1, attributes=attempt_attrs) + + # 3. gcp.storage.client.errors attributes + if error_type != "OK": + error_attrs: Dict[str, Any] = { + **self._base_attributes, + "error.type": error_type, + } + if resolved_op is not None: + error_attrs["gcp.storage.operation"] = resolved_op + if resolved_method is not None: + error_attrs["gcp.client.method"] = resolved_method + self.errors_counter.add(1, attributes=error_attrs) + + def record_rpc_attempt( + self, + duration: float, + operation: str, + status_code: Optional[str] = None, + error_type: str = "OK", + client_method: Optional[str] = None, + network_path: Optional[str] = None, + ) -> None: + """Records a single physical gRPC call attempt.""" + if not self.standard_enabled or self.meter is None: + return + op_attrs = self._build_operation_attributes( + operation=operation, client_method=client_method + ) + + # 1. rpc.client.call.duration attributes + rpc_attrs: Dict[str, Any] = { + **op_attrs, + "error.type": error_type, + } + if status_code is not None: + rpc_attrs["rpc.response.status_code"] = status_code + if network_path is not None: + rpc_attrs["network.path"] = network_path + + # 2. gcp.storage.client.attempts attributes + attempt_attrs: Dict[str, Any] = { + **self._base_attributes, + "gcp.storage.operation": operation, + "error.type": error_type, + } + if status_code is not None: + attempt_attrs["rpc.response.status_code"] = status_code + if network_path is not None: + attempt_attrs["network.path"] = network_path + + self.rpc_call_duration.record(max(0.0, duration), attributes=rpc_attrs) + self.attempts_counter.add(1, attributes=attempt_attrs) + + # 3. gcp.storage.client.errors attributes + if error_type != "OK": + error_attrs: Dict[str, Any] = { + **op_attrs, + "error.type": error_type, + } + self.errors_counter.add(1, attributes=error_attrs) + + def record_request_body_size( + self, + size: int, + operation: Optional[str] = None, + client_method: Optional[str] = None, + ) -> None: + """Records the payload byte size of an upload or write operation.""" + if not self.standard_enabled or self.meter is None: + return + if not isinstance(size, int) or isinstance(size, bool): + return + current_op = _OP_STATE.get() + if current_op is not None and current_op.suppressed: + return + attrs = self._build_operation_attributes( + operation=operation, client_method=client_method, current_op=current_op + ) + self.request_body_size.record(max(0, size), attributes=attrs) + + def record_response_body_size( + self, + size: int, + operation: Optional[str] = None, + client_method: Optional[str] = None, + ) -> None: + """Records the payload byte size of a download or read operation.""" + if not self.standard_enabled or self.meter is None: + return + if not isinstance(size, int) or isinstance(size, bool): + return + current_op = _OP_STATE.get() + if current_op is not None and current_op.suppressed: + return + attrs = self._build_operation_attributes( + operation=operation, client_method=client_method, current_op=current_op + ) + self.response_body_size.record(max(0, size), attributes=attrs) + + def record_ttfb( + self, + duration: float, + operation: Optional[str] = None, + client_method: Optional[str] = None, + ) -> None: + """Records time to first response byte (TTFB) in seconds.""" + if not self.standard_enabled or self.meter is None: + return + current_op = _OP_STATE.get() + if current_op is not None and current_op.suppressed: + return + attrs = self._build_operation_attributes( + operation=operation, client_method=client_method, current_op=current_op + ) + self.operation_ttfb.record(max(0.0, duration), attributes=attrs) + + def shutdown(self) -> None: + """Flushes pending metrics on a custom MeterProvider if configured.""" + if self._meter_provider is not None and hasattr( + self._meter_provider, "force_flush" + ): + try: + self._meter_provider.force_flush() + except Exception as exc: + logger.debug( + "Failed to force_flush custom MeterProvider during client shutdown: %s", + exc, + ) diff --git a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py index 966cb8a1c78d..cb392b2f9870 100644 --- a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py +++ b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py @@ -12,13 +12,24 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio +import concurrent.futures +import http.client import importlib +import socket +import ssl import sys +import types from unittest import mock +from google.api_core import exceptions as api_exceptions +from google.auth import exceptions as auth_exceptions import pytest +import requests.exceptions +import urllib3.exceptions from google.cloud.storage import _opentelemetry_metrics +from google.cloud.storage import exceptions as storage_exceptions from google.cloud.storage.version import __version__ @@ -144,7 +155,6 @@ def test_client_setting_overrides_env_var(monkeypatch): def test_non_boolean_client_setting_raises_type_error(): """Passing a non-boolean setting such as 'false' must raise TypeError.""" - # Argument validation executes before the _ENABLE_METRICS_DEV_GATE check. with pytest.raises(TypeError, match="enable_metrics must be a boolean or None"): _opentelemetry_metrics.is_metrics_enabled(enable_metrics="false") # type: ignore[arg-type] @@ -287,3 +297,972 @@ def test_get_meter(monkeypatch): __version__, meter_provider=mock_provider, ) + + +# --------------------------------------------------------------------------- +# Histogram Bucket Boundaries & Views Tests +# --------------------------------------------------------------------------- +def test_histogram_bucket_boundaries(): + """Verify custom latency and byte-size bucket boundaries match specification.""" + latency = _opentelemetry_metrics.LATENCY_HISTOGRAM_BOUNDARIES + assert latency[0] == 0.0 + assert latency[1] == 0.002 + assert latency[49] == 0.098 + assert latency[50] == 0.1 + assert latency[60] == 0.2 + assert latency[-1] < 300.0 + assert list(latency) == sorted(latency) + + sizes = _opentelemetry_metrics.SIZE_HISTOGRAM_BOUNDARIES + assert len(sizes) == 45 + assert sizes[0] == 0 + assert sizes[1] == 128 * 1024 + assert sizes[32] == 4 * 1024 * 1024 + assert sizes[33] == 8 * 1024 * 1024 + assert sizes[-1] == 16 * 1024 * 1024 * 1024 + assert list(sizes) == sorted(sizes) + + +def test_get_histogram_views_with_and_without_sdk(monkeypatch): + """Verify get_histogram_views builds 6 Views when SDK is installed and [] otherwise.""" + fake_view_mod = types.ModuleType("opentelemetry.sdk.metrics.view") + + class FakeAggregation: + def __init__(self, boundaries): + self.boundaries = boundaries + + class FakeView: + def __init__(self, instrument_name, aggregation): + self.instrument_name = instrument_name + self.aggregation = aggregation + + fake_view_mod.ExplicitBucketHistogramAggregation = FakeAggregation + fake_view_mod.View = FakeView + + with monkeypatch.context() as m: + m.setitem(sys.modules, "opentelemetry", types.ModuleType("opentelemetry")) + m.setitem( + sys.modules, "opentelemetry.sdk", types.ModuleType("opentelemetry.sdk") + ) + m.setitem( + sys.modules, + "opentelemetry.sdk.metrics", + types.ModuleType("opentelemetry.sdk.metrics"), + ) + m.setitem(sys.modules, "opentelemetry.sdk.metrics.view", fake_view_mod) + + views = _opentelemetry_metrics.get_histogram_views() + assert len(views) == 6 + latency_views = [ + v + for v in views + if v.instrument_name in _opentelemetry_metrics._LATENCY_METRIC_NAMES + ] + size_views = [ + v + for v in views + if v.instrument_name in _opentelemetry_metrics._SIZE_METRIC_NAMES + ] + assert len(latency_views) == 4 + assert len(size_views) == 2 + assert ( + latency_views[0].aggregation.boundaries + == _opentelemetry_metrics.LATENCY_HISTOGRAM_BOUNDARIES + ) + assert ( + size_views[0].aggregation.boundaries + == _opentelemetry_metrics.SIZE_HISTOGRAM_BOUNDARIES + ) + + # Without opentelemetry-sdk installed + with monkeypatch.context() as m: + m.setitem(sys.modules, "opentelemetry.sdk.metrics.view", None) + assert _opentelemetry_metrics.get_histogram_views() == [] + + +# --------------------------------------------------------------------------- +# Error Classification (`classify_error_type`) Tests +# --------------------------------------------------------------------------- +def test_classify_error_type_none_and_status_codes(): + assert _opentelemetry_metrics.classify_error_type() == "OK" + assert _opentelemetry_metrics.classify_error_type(None, status_code=200) == "OK" + assert _opentelemetry_metrics.classify_error_type(None, status_code=206) == "OK" + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=True) == "OK" # type: ignore[arg-type] + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=400) + == "INVALID_ARGUMENT" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=401) + == "UNAUTHENTICATED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=403) + == "PERMISSION_DENIED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=404) == "NOT_FOUND" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=408) + == "DEADLINE_EXCEEDED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=409) == "ABORTED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=412) + == "FAILED_PRECONDITION" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=416) + == "OUT_OF_RANGE" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=429) + == "RESOURCE_EXHAUSTED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=499) == "CANCELLED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=500) == "INTERNAL" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=501) + == "UNIMPLEMENTED" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=503) + == "UNAVAILABLE" + ) + assert ( + _opentelemetry_metrics.classify_error_type(None, status_code=504) + == "DEADLINE_EXCEEDED" + ) + assert _opentelemetry_metrics.classify_error_type(None, status_code=502) == "502" + assert _opentelemetry_metrics.classify_error_type(None, status_code=99999) == "OK" + + +def test_classify_error_type_exceptions(): + # 1. Checksum Mismatch (including subclasses of DataCorruption) + resp_mock = mock.Mock(status_code=200) + assert ( + _opentelemetry_metrics.classify_error_type( + storage_exceptions.DataCorruption(resp_mock, "crc mismatch") + ) + == "CHECKSUM_MISMATCH" + ) + + class CustomDataCorruption(storage_exceptions.DataCorruption): + pass + + assert ( + _opentelemetry_metrics.classify_error_type( + CustomDataCorruption(resp_mock, "subclass mismatch") + ) + == "CHECKSUM_MISMATCH" + ) + + # 2. Authentication Errors + assert ( + _opentelemetry_metrics.classify_error_type( + auth_exceptions.RefreshError("token expired") + ) + == "AUTHENTICATION_ERROR" + ) + assert ( + _opentelemetry_metrics.classify_error_type( + auth_exceptions.TransportError("auth transport error") + ) + == "AUTHENTICATION_ERROR" + ) + assert ( + _opentelemetry_metrics.classify_error_type( + auth_exceptions.DefaultCredentialsError("missing adc") + ) + == "AUTHENTICATION_ERROR" + ) + + # 3. Cancellation + assert ( + _opentelemetry_metrics.classify_error_type(asyncio.CancelledError()) + == "CANCELLED" + ) + + # 4. HTTP Conflict (409) consistency with status_code=409 + conflict_exc = api_exceptions.Conflict("conflict") + assert _opentelemetry_metrics.classify_error_type(conflict_exc) == "ABORTED" + + # 5. gRPC / GoogleAPICallError status codes + not_found_exc = api_exceptions.NotFound("not found") + assert _opentelemetry_metrics.classify_error_type(not_found_exc) == "NOT_FOUND" + + custom_api_exc = api_exceptions.GoogleAPICallError("custom") + custom_api_exc.grpc_status_code = "RAW_GRPC_STATUS" # type: ignore[assignment] + assert ( + _opentelemetry_metrics.classify_error_type(custom_api_exc) == "RAW_GRPC_STATUS" + ) + + # Direct grpc.RpcError-like exception with .code() method + class FakeStatusCode: + name = "DEADLINE_EXCEEDED" + + class FakeRpcError(Exception): + def code(self): + return FakeStatusCode() + + assert ( + _opentelemetry_metrics.classify_error_type(FakeRpcError()) + == "DEADLINE_EXCEEDED" + ) + + class FakeRpcErrorNoName(Exception): + def code(self): + return object() + + assert _opentelemetry_metrics.classify_error_type(FakeRpcErrorNoName()) == "UNKNOWN" + + # 6. HTTP status code on exception + http_api_exc = api_exceptions.GoogleAPICallError("bad gateway") + http_api_exc.code = 502 + http_api_exc.grpc_status_code = None + assert _opentelemetry_metrics.classify_error_type(http_api_exc) == "502" + + invalid_resp_with_code = storage_exceptions.InvalidResponse( + mock.Mock(status_code=429), "rate limited" + ) + assert ( + _opentelemetry_metrics.classify_error_type(invalid_resp_with_code) + == "RESOURCE_EXHAUSTED" + ) + + req_http_err = requests.exceptions.HTTPError(response=mock.Mock(status_code=503)) + assert _opentelemetry_metrics.classify_error_type(req_http_err) == "UNAVAILABLE" + + # 7. Timeouts + assert ( + _opentelemetry_metrics.classify_error_type( + requests.exceptions.ConnectTimeout("connect timeout") + ) + == "TIMEOUT" + ) + assert ( + _opentelemetry_metrics.classify_error_type( + requests.exceptions.ReadTimeout("read timeout") + ) + == "TIMEOUT" + ) + assert ( + _opentelemetry_metrics.classify_error_type(socket.timeout("timed out")) + == "TIMEOUT" + ) + assert ( + _opentelemetry_metrics.classify_error_type( + urllib3.exceptions.ReadTimeoutError(None, "/", "read timed out") # type: ignore[arg-type] + ) + == "TIMEOUT" + ) + assert ( + _opentelemetry_metrics.classify_error_type(concurrent.futures.TimeoutError()) + == "TIMEOUT" + ) + retry_err = api_exceptions.RetryError( + "deadline exceeded", cause=socket.timeout("timed out") + ) + assert _opentelemetry_metrics.classify_error_type(retry_err) == "TIMEOUT" + + # 8. TLS / SSL Failure + assert ( + _opentelemetry_metrics.classify_error_type( + requests.exceptions.SSLError("cert verify failed") + ) + == "TLS_FAILURE" + ) + assert ( + _opentelemetry_metrics.classify_error_type(ssl.SSLError("ssl handshake error")) + == "TLS_FAILURE" + ) + + # 9. DNS, Connection Refused, Connection Reset (direct and wrapped in requests/urllib3) + gai_err = socket.gaierror(-2, "Name or service not known") + new_conn_err = urllib3.exceptions.NewConnectionError(None, "dns failed") # type: ignore[arg-type] + new_conn_err.__cause__ = gai_err + wrapped_dns = requests.exceptions.ConnectionError( + urllib3.exceptions.MaxRetryError( + None, # type: ignore[arg-type] + "/", + reason=new_conn_err, + ) + ) + assert _opentelemetry_metrics.classify_error_type(wrapped_dns) == "DNS_FAILURE" + + refused_err = ConnectionRefusedError(111, "Connection refused") + wrapped_refused = requests.exceptions.ConnectionError(refused_err) + assert ( + _opentelemetry_metrics.classify_error_type(wrapped_refused) + == "CONNECTION_REFUSED" + ) + + reset_err = ConnectionResetError(104, "Connection reset by peer") + wrapped_reset = requests.exceptions.ConnectionError(reset_err) + assert ( + _opentelemetry_metrics.classify_error_type(wrapped_reset) == "CONNECTION_RESET" + ) + + # 10. Generic Connectivity + assert ( + _opentelemetry_metrics.classify_error_type( + requests.exceptions.ConnectionError("connection aborted") + ) + == "CONNECTIVITY" + ) + assert ( + _opentelemetry_metrics.classify_error_type( + requests.exceptions.ChunkedEncodingError("incomplete read") + ) + == "CONNECTIVITY" + ) + assert ( + _opentelemetry_metrics.classify_error_type(OSError("network unreachable")) + == "CONNECTIVITY" + ) + assert ( + _opentelemetry_metrics.classify_error_type( + http.client.IncompleteRead(b"partial", 10) + ) + == "CONNECTIVITY" + ) + + # 11. Explicit status_code fallback when exception is generic + assert ( + _opentelemetry_metrics.classify_error_type( + RuntimeError("unexpected"), status_code=503 + ) + == "UNAVAILABLE" + ) + + # 12. Client Validation Errors + assert ( + _opentelemetry_metrics.classify_error_type(ValueError("bad arg")) + == "CLIENT_VALIDATION_ERROR" + ) + assert ( + _opentelemetry_metrics.classify_error_type(TypeError("bad type")) + == "CLIENT_VALIDATION_ERROR" + ) + invalid_resp_no_code = storage_exceptions.InvalidResponse( + mock.Mock(status_code=None), "missing header" + ) + assert ( + _opentelemetry_metrics.classify_error_type(invalid_resp_no_code) + == "CLIENT_VALIDATION_ERROR" + ) + + # 13. Cyclic exception chain and unknown exception + e1 = RuntimeError("cycle 1") + e2 = RuntimeError("cycle 2", e1) + e1.__cause__ = e2 + assert _opentelemetry_metrics.classify_error_type(e1) == "UNKNOWN" + + +# --------------------------------------------------------------------------- +# Route Normalization (`normalize_url_template`) Tests +# --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "raw_url,expected", + [ + (None, "UNKNOWN"), + ("", "UNKNOWN"), + (123, "UNKNOWN"), + ("https://storage.googleapis.com", "UNKNOWN"), + ("/", "UNKNOWN"), + ("///", "UNKNOWN"), + ( + "https://storage.googleapis.com//storage/v1/b/my-bucket//", + "/storage/v1/b/{bucket}", + ), + ( + "https://storage.googleapis.com/upload/storage/v1/b/my-bucket/o/my-obj?uploadType=resumable", + "/upload/storage/v1/b/{bucket}/o", + ), + ( + "https://storage.googleapis.com/custom/path?tag=my_uploads", + "UNKNOWN", + ), + ( + "https://storage.googleapis.com/custom/path?my_uploadId=123", + "UNKNOWN", + ), + ( + "https://storage.googleapis.com/storage/v1/b?project=my-proj", + "/storage/v1/b", + ), + ("/b?project=my-proj", "/storage/v1/b"), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket?projection=full", + "/storage/v1/b/{bucket}", + ), + ("/b/my-bucket/", "/storage/v1/b/{bucket}"), + ("b/my-bucket", "/storage/v1/b/{bucket}"), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/o?prefix=foo", + "/storage/v1/b/{bucket}/o", + ), + ("/b/my-bucket/o", "/storage/v1/b/{bucket}/o"), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/o/folder%2Ffile.txt?generation=1", + "/storage/v1/b/{bucket}/o/{object}", + ), + ("/b/my-bucket/o/file.txt", "/storage/v1/b/{bucket}/o/{object}"), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/o/dest.txt/compose", + "/storage/v1/b/{bucket}/o/{object}/compose", + ), + ( + "/b/my-bucket/o/dest.txt/compose", + "/storage/v1/b/{bucket}/o/{object}/compose", + ), + ( + "https://storage.googleapis.com/storage/v1/b/src-b/o/src-o/rewriteTo/b/dst-b/o/dst-o", + "/storage/v1/b/{bucket}/o/{object}/rewriteTo/b/{bucket}/o/{object}", + ), + ( + "/b/src-b/o/src-o/rewriteTo/b/dst-b/o/dst-o", + "/storage/v1/b/{bucket}/o/{object}/rewriteTo/b/{bucket}/o/{object}", + ), + ( + "https://storage.googleapis.com/storage/v1/b/src-b/o/src-o/copyTo/b/dst-b/o/dst-o", + "/storage/v1/b/{bucket}/o/{object}/copyTo/b/{bucket}/o/{object}", + ), + ( + "/b/src-b/o/src-o/copyTo/b/dst-b/o/dst-o", + "/storage/v1/b/{bucket}/o/{object}/copyTo/b/{bucket}/o/{object}", + ), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/o/src-o/moveTo/o/dst-o", + "/storage/v1/b/{bucket}/o/{object}/moveTo/o/{object}", + ), + ( + "/b/my-bucket/o/src-o/moveTo/o/dst-o", + "/storage/v1/b/{bucket}/o/{object}/moveTo/o/{object}", + ), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/o/my-o/restore", + "/storage/v1/b/{bucket}/o/{object}/restore", + ), + ( + "/b/my-bucket/o/my-o/restore", + "/storage/v1/b/{bucket}/o/{object}/restore", + ), + ( + "/b/my-bucket/o/my-o/acl", + "/storage/v1/b/{bucket}/o/{object}/acl", + ), + ( + "/b/my-bucket/o/my-o/acl/allUsers", + "/storage/v1/b/{bucket}/o/{object}/acl/{entity}", + ), + ( + "/b/my-bucket/o/my-o/iam", + "/storage/v1/b/{bucket}/o/{object}/iam", + ), + ( + "/b/my-bucket/o/my-o/iam/testPermissions", + "/storage/v1/b/{bucket}/o/{object}/iam", + ), + ( + "/b/my-bucket/acl", + "/storage/v1/b/{bucket}/acl", + ), + ( + "/b/my-bucket/acl/allUsers", + "/storage/v1/b/{bucket}/acl/{entity}", + ), + ( + "/b/my-bucket/defaultObjectAcl", + "/storage/v1/b/{bucket}/defaultObjectAcl", + ), + ( + "/b/my-bucket/defaultObjectAcl/allUsers", + "/storage/v1/b/{bucket}/defaultObjectAcl/{entity}", + ), + ( + "/b/my-bucket/restore", + "/storage/v1/b/{bucket}/restore", + ), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/iam/testPermissions", + "/storage/v1/b/{bucket}/iam", + ), + ("/b/my-bucket/iam", "/storage/v1/b/{bucket}/iam"), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/notificationConfigs", + "/storage/v1/b/{bucket}/notificationConfigs", + ), + ( + "/b/my-bucket/notificationConfigs", + "/storage/v1/b/{bucket}/notificationConfigs", + ), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/notificationConfigs/42", + "/storage/v1/b/{bucket}/notificationConfigs/{notification}", + ), + ( + "/b/my-bucket/notificationConfigs/42", + "/storage/v1/b/{bucket}/notificationConfigs/{notification}", + ), + ( + "https://storage.googleapis.com/storage/v1/b/my-bucket/lockRetentionPolicy", + "/storage/v1/b/{bucket}/lockRetentionPolicy", + ), + ( + "/b/my-bucket/lockRetentionPolicy", + "/storage/v1/b/{bucket}/lockRetentionPolicy", + ), + ( + "/storage/v1/projects/my-proj/serviceAccount", + "/storage/v1/projects/{project}/serviceAccount", + ), + ( + "/projects/my-proj/serviceAccount", + "/storage/v1/projects/{project}/serviceAccount", + ), + ( + "/storage/v1/projects/my-proj/hmacKeys", + "/storage/v1/projects/{project}/hmacKeys", + ), + ("/projects/my-proj/hmacKeys", "/storage/v1/projects/{project}/hmacKeys"), + ( + "/storage/v1/projects/my-proj/hmacKeys/GOOG123", + "/storage/v1/projects/{project}/hmacKeys/{access_id}", + ), + ( + "/projects/my-proj/hmacKeys/GOOG123", + "/storage/v1/projects/{project}/hmacKeys/{access_id}", + ), + ( + "https://storage.googleapis.com/upload/storage/v1/b/my-bucket/o?uploadType=resumable", + "/upload/storage/v1/b/{bucket}/o", + ), + ( + "https://storage.googleapis.com/download/storage/v1/b/my-bucket/o/my-obj?alt=media", + "/download/storage/v1/b/{bucket}/o/{object}", + ), + ("https://storage.googleapis.com/batch/storage/v1", "/batch/storage/v1"), + ( + "https://storage.googleapis.com/my-bucket/my-object?uploads", + "/{bucket}/{object}", + ), + ( + "https://storage.googleapis.com/my-bucket/nested/dir/object.bin?uploadId=xyz&partNumber=2", + "/{bucket}/{object}", + ), + ( + "https://storage.googleapis.com/unrecognized/two-segment", + "UNKNOWN", + ), + ("https://storage.googleapis.com/storage/v2", "UNKNOWN"), + ("https://storage.googleapis.com/storage/v2/other?uploadId=xyz", "UNKNOWN"), + ("https://storage.googleapis.com/custom/unrecognized/path/here", "UNKNOWN"), + ], +) +def test_normalize_url_template(raw_url, expected): + assert _opentelemetry_metrics.normalize_url_template(raw_url) == expected + + +# --------------------------------------------------------------------------- +# Logical Operation Context (`start_operation`) Tests +# --------------------------------------------------------------------------- +def test_start_operation_disabled_and_nesting(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + mock_metrics = mock.Mock() + mock_meter = mock.Mock() + mock_meter.create_histogram.side_effect = lambda *a, **kw: mock.Mock() + mock_meter.create_counter.side_effect = lambda *a, **kw: mock.Mock() + mock_metrics.get_meter.return_value = mock_meter + monkeypatch.setattr(_opentelemetry_metrics, "metrics", mock_metrics, raising=False) + + # Disabled recorder -> yields None and records nothing + disabled_recorder = _opentelemetry_metrics._MetricsRecorder(enable_metrics=False) + with _opentelemetry_metrics.start_operation( + disabled_recorder, "storage.Blob.reload", "storage.objects.get" + ) as op: + assert op is None + assert _opentelemetry_metrics.get_current_operation() is None + + # Enabled recorder -> outermost call records, nested call is suppressed + enabled_recorder = _opentelemetry_metrics._MetricsRecorder(enable_metrics=True) + with _opentelemetry_metrics.start_operation( + enabled_recorder, + "storage.Blob.download_to_filename", + "storage.objects.get", + ) as outer_op: + assert outer_op is not None + assert outer_op.client_method == "storage.Blob.download_to_filename" + assert outer_op.rpc_method == "storage.objects.get" + assert _opentelemetry_metrics.get_current_operation() == outer_op + + with _opentelemetry_metrics.start_operation( + enabled_recorder, + "storage.Blob.download_to_file", + "storage.objects.get", + ) as inner_op: + assert inner_op is outer_op + + assert _opentelemetry_metrics.get_current_operation() is None + enabled_recorder.client_request_duration.record.assert_called_once() + enabled_recorder.operations_counter.add.assert_called_once() + _, kwargs = enabled_recorder.operations_counter.add.call_args + assert ( + kwargs["attributes"]["gcp.client.method"] == "storage.Blob.download_to_filename" + ) + assert kwargs["attributes"]["gcp.storage.operation"] == "storage.objects.get" + assert kwargs["attributes"]["error.type"] == "OK" + + +def test_start_operation_exception_and_suppression(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + mock_metrics = mock.Mock() + mock_meter = mock.Mock() + mock_meter.create_histogram.side_effect = lambda *a, **kw: mock.Mock() + mock_meter.create_counter.side_effect = lambda *a, **kw: mock.Mock() + mock_metrics.get_meter.return_value = mock_meter + monkeypatch.setattr(_opentelemetry_metrics, "metrics", mock_metrics, raising=False) + + recorder = _opentelemetry_metrics._MetricsRecorder(enable_metrics=True) + + with pytest.raises(api_exceptions.NotFound): + with _opentelemetry_metrics.start_operation( + recorder, "storage.Bucket.get_blob", "storage.objects.get" + ): + raise api_exceptions.NotFound("missing") + + assert _opentelemetry_metrics.get_current_operation() is None + _, kwargs = recorder.operations_counter.add.call_args + assert kwargs["attributes"]["error.type"] == "NOT_FOUND" + + # Suppressed outer operation (e.g. transfer_manager worker) suppresses both + # logical operation metrics and body size recording + recorder.operations_counter.add.reset_mock() + recorder.client_request_duration.record.reset_mock() + with _opentelemetry_metrics.start_operation( + None, + "storage.transfer_manager.download_many_to_path", + "storage.objects.get", + suppress=True, + ) as suppressed_op: + assert suppressed_op is not None + assert suppressed_op.suppressed is True + with _opentelemetry_metrics.start_operation( + recorder, + "storage.Blob.download_to_filename", + "storage.objects.get", + ): + recorder.record_request_body_size(2048) + recorder.record_response_body_size(4096) + + assert _opentelemetry_metrics.get_current_operation() is None + recorder.operations_counter.add.assert_not_called() + recorder.client_request_duration.record.assert_not_called() + recorder.request_body_size.record.assert_not_called() + recorder.response_body_size.record.assert_not_called() + + # Nested suppress=True inside an active unsuppressed operation (e.g. BlobReader prefetch) + with _opentelemetry_metrics.start_operation( + recorder, + "storage.Blob.open", + "storage.objects.get", + ) as active_op: + assert active_op is not None and active_op.suppressed is False + with _opentelemetry_metrics.start_operation( + recorder, + "storage.Blob.download_as_bytes", + "storage.objects.get", + suppress=True, + ) as child_suppressed: + assert child_suppressed is not None + assert child_suppressed.suppressed is True + assert child_suppressed.client_method == "storage.Blob.open" + # Prefetch body size and TTFB are suppressed inside child_suppressed + recorder.record_response_body_size(40 * 1024 * 1024) + recorder.record_ttfb(0.05) + # Outside the suppressed prefetch block, actual user bytes are recorded + assert _opentelemetry_metrics.get_current_operation() == active_op + recorder.record_response_body_size(1024) + recorder.record_response_body_size(True) # type: ignore[arg-type] + recorder.record_request_body_size(False) # type: ignore[arg-type] + + recorder.response_body_size.record.assert_called_once() + assert recorder.response_body_size.record.call_args[0][0] == 1024 + recorder.operation_ttfb.record.assert_not_called() + recorder.operations_counter.add.assert_called_once() + + +def test_start_operation_asyncio_concurrency_isolation(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + mock_metrics = mock.Mock() + mock_meter = mock.Mock() + mock_meter.create_histogram.side_effect = lambda *a, **kw: mock.Mock() + mock_meter.create_counter.side_effect = lambda *a, **kw: mock.Mock() + mock_metrics.get_meter.return_value = mock_meter + monkeypatch.setattr(_opentelemetry_metrics, "metrics", mock_metrics, raising=False) + + recorder = _opentelemetry_metrics._MetricsRecorder( + enable_metrics=True, rpc_system="grpc" + ) + + async def worker(method_name: str, rpc_name: str): + with _opentelemetry_metrics.start_operation(recorder, method_name, rpc_name): + await asyncio.sleep(0.005) + current = _opentelemetry_metrics.get_current_operation() + assert current is not None + assert current.client_method == method_name + assert current.rpc_method == rpc_name + + async def run_all(): + await asyncio.gather( + worker( + "storage.AsyncMultiRangeDownloader.download_ranges", + "google.storage.v2.Storage/BidiReadObject", + ), + worker( + "storage.AsyncAppendableObjectWriter.append", + "google.storage.v2.Storage/BidiWriteObject", + ), + ) + + asyncio.run(run_all()) + assert recorder.operations_counter.add.call_count == 2 + + +# --------------------------------------------------------------------------- +# `_extract_server_address` & `_MetricsRecorder` Tests +# --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "endpoint,expected", + [ + (None, "storage.googleapis.com"), + ("", "storage.googleapis.com"), + (" ", "storage.googleapis.com"), + ("https://storage.googleapis.com:443", "storage.googleapis.com"), + ("storage.googleapis.com:443", "storage.googleapis.com"), + ("//storage.googleapis.com:443", "storage.googleapis.com"), + ("https://[::1]:8080", "::1"), + ("http://localhost:9023/storage/v1", "localhost"), + ("https://", "storage.googleapis.com"), + ], +) +def test_extract_server_address(endpoint, expected): + assert _opentelemetry_metrics._extract_server_address(endpoint) == expected + + +def test_metrics_recorder_recording_and_shutdown(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + mock_metrics = mock.Mock() + mock_meter = mock.Mock() + mock_meter.create_histogram.side_effect = lambda *a, **kw: mock.Mock() + mock_meter.create_counter.side_effect = lambda *a, **kw: mock.Mock() + mock_metrics.get_meter.return_value = mock_meter + monkeypatch.setattr(_opentelemetry_metrics, "metrics", mock_metrics, raising=False) + + mock_provider = mock.Mock() + recorder = _opentelemetry_metrics._MetricsRecorder( + enable_metrics=True, + meter_provider=mock_provider, + api_endpoint="https://custom.storage.googleapis.com:443", + rpc_system="http", + ) + assert recorder.standard_enabled is True + assert recorder.enabled is True + assert recorder.server_address == "custom.storage.googleapis.com" + + base_attrs = recorder.get_base_attributes() + assert base_attrs["server.address"] == "custom.storage.googleapis.com" + assert base_attrs["rpc.system.name"] == "http" + + # Verify exact instrument names, units, and descriptions + histogram_specs = { + call.args[0]: (call.kwargs["unit"], call.kwargs["description"]) + for call in mock_meter.create_histogram.call_args_list + } + assert histogram_specs == { + "rpc.client.call.duration": ( + "s", + "Duration of an individual gRPC attempt in seconds (excluding retries).", + ), + "http.client.request.duration": ( + "s", + "Duration of an individual HTTP request attempt in seconds (excluding retries).", + ), + "gcp.client.request.duration": ( + "s", + "End-to-end latency of a logical user-facing Veneer operation in seconds.", + ), + "gcp.storage.client.request.body.size": ( + "By", + "Application payload bytes submitted by the caller for an upload or write operation.", + ), + "gcp.storage.client.response.body.size": ( + "By", + "Application payload bytes delivered to the caller.", + ), + "gcp.storage.client.operation.ttfb": ( + "s", + "Time elapsed from attempt initiation to receipt of the first response byte.", + ), + } + counter_specs = { + call.args[0]: (call.kwargs["unit"], call.kwargs["description"]) + for call in mock_meter.create_counter.call_args_list + } + assert counter_specs == { + "gcp.storage.client.operations": ( + "{operation}", + "Number of logical GCS client operations.", + ), + "gcp.storage.client.attempts": ( + "{attempt}", + "Number of physical network transport attempts executed.", + ), + "gcp.storage.client.errors": ( + "{error}", + "Number of physical network transport attempts terminating with a non-OK status.", + ), + } + + # Record within a logical operation context + with _opentelemetry_metrics.start_operation( + recorder, "storage.Blob.upload_from_string", "storage.objects.insert" + ): + recorder.record_http_attempt( + duration=0.05, + http_method="post", + url_template="/upload/storage/v1/b/{bucket}/o", + status_code=503, + error_type="UNAVAILABLE", + network_path="cloudpath", + ) + recorder.record_http_attempt( + duration=0.04, + http_method="POST", + url_template="/upload/storage/v1/b/{bucket}/o", + status_code=200, + error_type="OK", + ) + recorder.record_request_body_size(1024) + recorder.record_response_body_size(512) + recorder.record_ttfb(0.01) + + assert recorder.http_request_duration.record.call_count == 2 + _, http_dur_kwargs = recorder.http_request_duration.record.call_args_list[0] + assert http_dur_kwargs["attributes"]["http.request.method"] == "POST" + assert ( + http_dur_kwargs["attributes"]["url.template"] + == "/upload/storage/v1/b/{bucket}/o" + ) + assert http_dur_kwargs["attributes"]["http.response.status_code"] == 503 + assert http_dur_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert http_dur_kwargs["attributes"]["network.path"] == "cloudpath" + assert "gcp.storage.operation" not in http_dur_kwargs["attributes"] + assert "gcp.client.method" not in http_dur_kwargs["attributes"] + + assert recorder.attempts_counter.add.call_count == 2 + _, attempt_kwargs = recorder.attempts_counter.add.call_args_list[0] + assert ( + attempt_kwargs["attributes"]["gcp.storage.operation"] + == "storage.objects.insert" + ) + assert attempt_kwargs["attributes"]["http.response.status_code"] == 503 + assert attempt_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert attempt_kwargs["attributes"]["network.path"] == "cloudpath" + assert "url.template" not in attempt_kwargs["attributes"] + assert "http.request.method" not in attempt_kwargs["attributes"] + + # Only the 503 attempt records an error + recorder.errors_counter.add.assert_called_once() + _, err_kwargs = recorder.errors_counter.add.call_args + assert err_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert err_kwargs["attributes"]["gcp.storage.operation"] == "storage.objects.insert" + assert ( + err_kwargs["attributes"]["gcp.client.method"] + == "storage.Blob.upload_from_string" + ) + assert "url.template" not in err_kwargs["attributes"] + assert "http.response.status_code" not in err_kwargs["attributes"] + + recorder.request_body_size.record.assert_called_once() + recorder.response_body_size.record.assert_called_once() + recorder.operation_ttfb.record.assert_called_once() + + # gRPC attempt recording (OK and non-OK) + recorder.errors_counter.add.reset_mock() + recorder.attempts_counter.add.reset_mock() + recorder.record_rpc_attempt( + duration=0.02, + operation="google.storage.v2.Storage/BidiReadObject", + status_code="OK", + error_type="OK", + client_method="storage.AsyncMultiRangeDownloader.download_ranges", + network_path="directpath", + ) + recorder.record_rpc_attempt( + duration=0.03, + operation="google.storage.v2.Storage/BidiReadObject", + status_code="UNAVAILABLE", + error_type="UNAVAILABLE", + client_method="storage.AsyncMultiRangeDownloader.download_ranges", + ) + assert recorder.rpc_call_duration.record.call_count == 2 + _, rpc_dur_kwargs = recorder.rpc_call_duration.record.call_args_list[0] + assert ( + rpc_dur_kwargs["attributes"]["gcp.storage.operation"] + == "google.storage.v2.Storage/BidiReadObject" + ) + assert ( + rpc_dur_kwargs["attributes"]["gcp.client.method"] + == "storage.AsyncMultiRangeDownloader.download_ranges" + ) + assert rpc_dur_kwargs["attributes"]["rpc.response.status_code"] == "OK" + assert rpc_dur_kwargs["attributes"]["network.path"] == "directpath" + + assert recorder.attempts_counter.add.call_count == 2 + _, rpc_attempt_kwargs = recorder.attempts_counter.add.call_args_list[0] + assert ( + rpc_attempt_kwargs["attributes"]["gcp.storage.operation"] + == "google.storage.v2.Storage/BidiReadObject" + ) + assert rpc_attempt_kwargs["attributes"]["rpc.response.status_code"] == "OK" + assert rpc_attempt_kwargs["attributes"]["error.type"] == "OK" + assert rpc_attempt_kwargs["attributes"]["network.path"] == "directpath" + assert "gcp.client.method" not in rpc_attempt_kwargs["attributes"] + + recorder.errors_counter.add.assert_called_once() + _, rpc_err_kwargs = recorder.errors_counter.add.call_args + assert rpc_err_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert "rpc.response.status_code" not in rpc_err_kwargs["attributes"] + + # Shutdown flushes custom meter_provider and swallows force_flush errors gracefully + recorder.shutdown() + mock_provider.force_flush.assert_called_once() + + mock_provider.force_flush.side_effect = RuntimeError("flush failed") + recorder.shutdown() # Must not raise + + +def test_metrics_recorder_disabled_no_ops(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", False) + recorder = _opentelemetry_metrics._MetricsRecorder(enable_metrics=True) + assert recorder.standard_enabled is False + assert recorder.enabled is False + + # All recording methods are safe no-ops when disabled + recorder.record_logical_operation(0.1, "m", "op") + recorder.record_http_attempt(0.1, "GET", "/storage/v1/b") + recorder.record_rpc_attempt(0.1, "google.storage.v2.Storage/ReadObject") + recorder.record_request_body_size(100) + recorder.record_response_body_size(100) + recorder.record_ttfb(0.01) + recorder.shutdown() From 799ac3a81cd9c49b633cbb60516395c1158b83ed Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Mon, 19 Oct 2026 19:02:17 +0000 Subject: [PATCH 2/7] fix(storage): fix ruff lint, py310 lower-bound metrics fallback, and branch coverage [Generated-by: AI] --- .../cloud/storage/_opentelemetry_metrics.py | 47 ++++++++---- .../tests/unit/test__opentelemetry_metrics.py | 74 +++++++++++++++++-- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py index c357a968407f..f7394c9b2740 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py +++ b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py @@ -15,11 +15,8 @@ """Manages OpenTelemetry metrics instruments and gating for GCS client.""" import asyncio -from collections import deque import concurrent.futures import contextlib -from contextvars import ContextVar -from dataclasses import dataclass import http.client import logging import os @@ -27,6 +24,9 @@ import socket import ssl import time +from collections import deque +from contextvars import ContextVar +from dataclasses import dataclass from typing import Any, Deque, Dict, Iterator, List, Optional, Set, Tuple from urllib.parse import urlsplit @@ -85,6 +85,7 @@ HAS_OPENTELEMETRY_METRICS = True except ImportError: + metrics = None # type: ignore[assignment] HAS_OPENTELEMETRY_METRICS = False logger.debug( "OpenTelemetry metrics package (opentelemetry-api >= 1.12.0) is not " @@ -207,9 +208,12 @@ def get_meter( Optional[Any]: The OpenTelemetry Meter instance if standard or debug metrics are enabled, or None otherwise. """ - if not ( - is_metrics_enabled(enable_metrics) - or is_debug_metrics_enabled(enable_debug_metrics) + if ( + not ( + is_metrics_enabled(enable_metrics) + or is_debug_metrics_enabled(enable_debug_metrics) + ) + or metrics is None ): return None return metrics.get_meter( @@ -238,13 +242,13 @@ def _build_latency_buckets() -> Tuple[float, ...]: boundary += increment increment = 0.010 - for i in range(150): - if boundary >= 300.0: - break + i = 0 + while boundary < 300.0: if i != 0 and i % 10 == 0: increment *= 2 boundaries.append(round(boundary, 6)) boundary += increment + i += 1 return tuple(boundaries) @@ -452,11 +456,10 @@ def classify_error_type( if callable(code_attr): try: rpc_status = code_attr() - if rpc_status is not None: - name = getattr(rpc_status, "name", None) - if name: - return str(name) - except Exception: # pragma: no cover + name = getattr(rpc_status, "name", None) + if name: + return str(name) + except Exception: # pragma: NO COVER pass if _is_valid_status_code(code_attr): @@ -777,7 +780,9 @@ def start_operation( yield existing return - recorder_active = recorder is not None and recorder.standard_enabled + recorder_active = ( + recorder is not None and getattr(recorder, "standard_enabled", False) is True + ) if not recorder_active and not suppress: yield None return @@ -807,6 +812,14 @@ def start_operation( ) +def get_active_recorder(obj: Any) -> Optional["_MetricsRecorder"]: + """Returns ``obj._metrics_recorder`` if it is an active ``_MetricsRecorder``.""" + recorder = getattr(obj, "_metrics_recorder", None) + if recorder is not None and getattr(recorder, "standard_enabled", False) is True: + return recorder + return None + + # --------------------------------------------------------------------------- # 10. Server Address Parsing & `_MetricsRecorder` # --------------------------------------------------------------------------- @@ -817,7 +830,9 @@ def _extract_server_address(api_endpoint: Optional[str]) -> str: endpoint = api_endpoint.strip() if not endpoint: return DEFAULT_SERVER_ADDRESS - if "://" not in endpoint: + if ":///" in endpoint: + endpoint = "https://" + endpoint.split(":///", 1)[1].lstrip("/") + elif "://" not in endpoint: endpoint = "https://" + endpoint.lstrip("/") parsed = urlsplit(endpoint) return parsed.hostname or DEFAULT_SERVER_ADDRESS diff --git a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py index cb392b2f9870..275baa8f7c5c 100644 --- a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py +++ b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py @@ -22,11 +22,11 @@ import types from unittest import mock -from google.api_core import exceptions as api_exceptions -from google.auth import exceptions as auth_exceptions import pytest import requests.exceptions import urllib3.exceptions +from google.api_core import exceptions as api_exceptions +from google.auth import exceptions as auth_exceptions from google.cloud.storage import _opentelemetry_metrics from google.cloud.storage import exceptions as storage_exceptions @@ -57,6 +57,8 @@ def test_opentelemetry_import_error_on_load(monkeypatch): is False ) assert _opentelemetry_metrics.get_meter(enable_metrics=True) is None + m.setattr(_opentelemetry_metrics, "HAS_OPENTELEMETRY_METRICS", True) + assert _opentelemetry_metrics.get_meter(enable_metrics=True) is None finally: importlib.reload(_opentelemetry_metrics) @@ -1025,11 +1027,11 @@ async def worker(method_name: str, rpc_name: str): async def run_all(): await asyncio.gather( worker( - "storage.AsyncMultiRangeDownloader.download_ranges", + "storage.asyncio.AsyncMultiRangeDownloader.download_ranges", "google.storage.v2.Storage/BidiReadObject", ), worker( - "storage.AsyncAppendableObjectWriter.append", + "storage.asyncio.AsyncAppendableObjectWriter.append", "google.storage.v2.Storage/BidiWriteObject", ), ) @@ -1205,7 +1207,7 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): operation="google.storage.v2.Storage/BidiReadObject", status_code="OK", error_type="OK", - client_method="storage.AsyncMultiRangeDownloader.download_ranges", + client_method="storage.asyncio.AsyncMultiRangeDownloader.download_ranges", network_path="directpath", ) recorder.record_rpc_attempt( @@ -1213,7 +1215,7 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): operation="google.storage.v2.Storage/BidiReadObject", status_code="UNAVAILABLE", error_type="UNAVAILABLE", - client_method="storage.AsyncMultiRangeDownloader.download_ranges", + client_method="storage.asyncio.AsyncMultiRangeDownloader.download_ranges", ) assert recorder.rpc_call_duration.record.call_count == 2 _, rpc_dur_kwargs = recorder.rpc_call_duration.record.call_args_list[0] @@ -1223,7 +1225,7 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): ) assert ( rpc_dur_kwargs["attributes"]["gcp.client.method"] - == "storage.AsyncMultiRangeDownloader.download_ranges" + == "storage.asyncio.AsyncMultiRangeDownloader.download_ranges" ) assert rpc_dur_kwargs["attributes"]["rpc.response.status_code"] == "OK" assert rpc_dur_kwargs["attributes"]["network.path"] == "directpath" @@ -1244,6 +1246,34 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): assert rpc_err_kwargs["attributes"]["error.type"] == "UNAVAILABLE" assert "rpc.response.status_code" not in rpc_err_kwargs["attributes"] + # Record outside of start_operation with status_code=None and callable .code() returning None + recorder.record_http_attempt( + duration=0.01, + http_method="GET", + url_template="/storage/v1/b", + status_code=None, + error_type="UNKNOWN", + ) + recorder.record_rpc_attempt( + duration=0.01, + operation="google.storage.v2.Storage/GetObject", + status_code=None, + error_type="UNKNOWN", + ) + recorder.record_request_body_size(64) + + class _NoneCodeErr(Exception): + def code(self): + return None + + assert _opentelemetry_metrics.classify_error_type(_NoneCodeErr()) == "UNKNOWN" + assert ( + _opentelemetry_metrics.classify_error_type( + urllib3.exceptions.NewConnectionError(None, "conn err") + ) + == "CONNECTIVITY" + ) + # Shutdown flushes custom meter_provider and swallows force_flush errors gracefully recorder.shutdown() mock_provider.force_flush.assert_called_once() @@ -1252,6 +1282,36 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): recorder.shutdown() # Must not raise +def test_get_active_recorder_and_grpc_uri_schemes(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "HAS_OPENTELEMETRY_METRICS", True) + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + + assert ( + _opentelemetry_metrics._extract_server_address("dns:///custom.endpoint.com:443") + == "custom.endpoint.com" + ) + assert ( + _opentelemetry_metrics._extract_server_address( + "google-c2p-experimental:///storage.googleapis.com" + ) + == "storage.googleapis.com" + ) + + # Unconfigured MagicMock should not be treated as an active recorder + unconfigured_mock = mock.MagicMock() + assert _opentelemetry_metrics.get_active_recorder(unconfigured_mock) is None + assert _opentelemetry_metrics.get_active_recorder(None) is None + + active_rec = _opentelemetry_metrics._MetricsRecorder(enable_metrics=True) + holder = mock.MagicMock() + holder._metrics_recorder = active_rec + assert _opentelemetry_metrics.get_active_recorder(holder) is active_rec + + disabled_rec = _opentelemetry_metrics._MetricsRecorder(enable_metrics=False) + holder._metrics_recorder = disabled_rec + assert _opentelemetry_metrics.get_active_recorder(holder) is None + + def test_metrics_recorder_disabled_no_ops(monkeypatch): monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", False) recorder = _opentelemetry_metrics._MetricsRecorder(enable_metrics=True) From 6544cdcf9bfe9190ad814eb1c639f3ce68eab2ed Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Mon, 19 Oct 2026 19:36:20 +0000 Subject: [PATCH 3/7] fix(storage): harden error classification against mocks and future cancellations [Generated-by: AI] --- .../cloud/storage/_opentelemetry_metrics.py | 32 +++++++++++-------- .../tests/unit/test__opentelemetry_metrics.py | 14 ++++++++ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py index f7394c9b2740..27675bc298a0 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py +++ b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py @@ -362,21 +362,25 @@ def _iter_exception_chain(exc: BaseException) -> Iterator[BaseException]: visited.add(id(nested)) queue.append(nested) - for arg in getattr(current, "args", ()): - if isinstance(arg, BaseException) and id(arg) not in visited: - visited.add(id(arg)) - queue.append(arg) + args = getattr(current, "args", ()) + if isinstance(args, (tuple, list)): + for arg in args: + if isinstance(arg, BaseException) and id(arg) not in visited: + visited.add(id(arg)) + queue.append(arg) def _is_subclass_by_module( - cls: type, module_prefix: str, class_names: Optional[Set[str]] = None + cls: type, module_prefix: str, class_names: Set[str] ) -> bool: """Checks whether ``cls`` or any of its base classes match module/name criteria.""" - for base in getattr(cls, "__mro__", ()): - mod = getattr(base, "__module__", "") or "" - if mod.startswith(module_prefix): - if class_names is None or base.__name__ in class_names: - return True + mro = getattr(cls, "__mro__", None) + if isinstance(mro, tuple): + for base in mro: + mod = getattr(base, "__module__", "") or "" + if mod == module_prefix or mod.startswith(module_prefix + "."): + if getattr(base, "__name__", None) in class_names: + return True return False @@ -426,7 +430,9 @@ def classify_error_type( # 3. Cancellation for item in chain: - if isinstance(item, asyncio.CancelledError): + if isinstance( + item, (asyncio.CancelledError, concurrent.futures.CancelledError) + ): return "CANCELLED" # 4. HTTP response status code on exception (checked before grpc_status_code so @@ -457,8 +463,8 @@ def classify_error_type( try: rpc_status = code_attr() name = getattr(rpc_status, "name", None) - if name: - return str(name) + if isinstance(name, str) and name: + return name except Exception: # pragma: NO COVER pass diff --git a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py index 275baa8f7c5c..79a2e76eaee5 100644 --- a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py +++ b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py @@ -1266,7 +1266,21 @@ class _NoneCodeErr(Exception): def code(self): return None + class _NonTupleArgsErr(Exception): + args = 123 # type: ignore[assignment] + assert _opentelemetry_metrics.classify_error_type(_NoneCodeErr()) == "UNKNOWN" + assert _opentelemetry_metrics.classify_error_type(_NonTupleArgsErr()) == "UNKNOWN" + assert ( + _opentelemetry_metrics.classify_error_type(concurrent.futures.CancelledError()) + == "CANCELLED" + ) + assert ( + _opentelemetry_metrics._is_subclass_by_module( + mock.MagicMock(__mro__=123), "google.cloud.storage", {"DataCorruption"} + ) + is False + ) assert ( _opentelemetry_metrics.classify_error_type( urllib3.exceptions.NewConnectionError(None, "conn err") From 49daee8669a9e78d1e5f6162c47f90c00f75f317 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Sun, 25 Oct 2026 18:13:38 +0000 Subject: [PATCH 4/7] fix(storage): align core OTel metrics module with Monarch schema - Add gcp.storage.client.retries counter and rename TTFB metric to gcp.storage.client.ttfb - Remove gcp.client.repo from _COMMON_ATTRIBUTES - Update _build_size_buckets to generate 48 buckets matching Go and Java SDKs - Align per-metric attributes across logical operation, RPC/HTTP attempt, error, retry, and TTFB recorders [Generated-by: AI] --- .../cloud/storage/_opentelemetry_metrics.py | 352 +++++++++++++++--- .../tests/unit/test__opentelemetry_metrics.py | 341 ++++++++++++++--- 2 files changed, 589 insertions(+), 104 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py index 27675bc298a0..f9583d5baba4 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py +++ b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py @@ -26,8 +26,18 @@ import time from collections import deque from contextvars import ContextVar -from dataclasses import dataclass -from typing import Any, Deque, Dict, Iterator, List, Optional, Set, Tuple +from dataclasses import dataclass, field +from typing import ( + Any, + Deque, + Dict, + FrozenSet, + Iterator, + List, + Optional, + Set, + Tuple, +) from urllib.parse import urlsplit from google.cloud.storage.version import __version__ @@ -60,16 +70,17 @@ METRIC_GCP_CLIENT_REQUEST_DURATION = "gcp.client.request.duration" METRIC_STORAGE_CLIENT_OPERATIONS = "gcp.storage.client.operations" METRIC_STORAGE_CLIENT_ATTEMPTS = "gcp.storage.client.attempts" +METRIC_STORAGE_CLIENT_RETRIES = "gcp.storage.client.retries" METRIC_STORAGE_REQUEST_BODY_SIZE = "gcp.storage.client.request.body.size" METRIC_STORAGE_RESPONSE_BODY_SIZE = "gcp.storage.client.response.body.size" -METRIC_STORAGE_OPERATION_TTFB = "gcp.storage.client.operation.ttfb" +METRIC_STORAGE_CLIENT_TTFB = "gcp.storage.client.ttfb" METRIC_STORAGE_CLIENT_ERRORS = "gcp.storage.client.errors" _LATENCY_METRIC_NAMES = ( METRIC_RPC_CLIENT_CALL_DURATION, METRIC_HTTP_CLIENT_REQUEST_DURATION, METRIC_GCP_CLIENT_REQUEST_DURATION, - METRIC_STORAGE_OPERATION_TTFB, + METRIC_STORAGE_CLIENT_TTFB, ) _SIZE_METRIC_NAMES = ( @@ -179,7 +190,6 @@ def is_debug_metrics_enabled( _COMMON_ATTRIBUTES: Dict[str, Any] = { "gcp.client.service": "storage", "gcp.client.version": __version__, - "gcp.client.repo": "googleapis/google-cloud-python", "gcp.client.artifact": "google-cloud-storage", } @@ -256,8 +266,9 @@ def _build_latency_buckets() -> Tuple[float, ...]: def _build_size_buckets() -> Tuple[int, ...]: """Builds the custom byte-size histogram bucket boundaries in bytes. - Uses 128 KiB resolution from 0 up to 4 MiB (33 buckets), then doubles every - bucket up to 16 GiB (12 buckets, 45 total buckets). + Uses 128 KiB increments up to 4 MiB, then doubles the increment on each + subsequent bucket while ``boundary <= 16 GiB`` (48 total buckets, matching + Go and Java SDKs). """ kb = 1024 mb = 1024 * kb @@ -269,10 +280,9 @@ def _build_size_buckets() -> Tuple[int, ...]: while boundary <= 16 * gb: boundaries.append(boundary) - if boundary < 4 * mb: - boundary += increment - else: - boundary *= 2 + boundary += increment + if boundary >= 4 * mb: + increment *= 2 return tuple(boundaries) @@ -362,12 +372,13 @@ def _iter_exception_chain(exc: BaseException) -> Iterator[BaseException]: visited.add(id(nested)) queue.append(nested) - args = getattr(current, "args", ()) - if isinstance(args, (tuple, list)): - for arg in args: - if isinstance(arg, BaseException) and id(arg) not in visited: - visited.add(id(arg)) - queue.append(arg) + for seq_attr in ("args", "errors"): + seq = getattr(current, seq_attr, ()) + if isinstance(seq, (tuple, list)): + for item in seq: + if isinstance(item, BaseException) and id(item) not in visited: + visited.add(id(item)) + queue.append(item) def _is_subclass_by_module( @@ -725,15 +736,96 @@ def normalize_url_template(url_or_path: Optional[str]) -> str: # --------------------------------------------------------------------------- -# 9. Logical Operation Context & Suppression (`ContextVar`) +# 9. Logical Operation Normalization, Context & Suppression (`ContextVar`) # --------------------------------------------------------------------------- -@dataclass(frozen=True) +_GRPC_LOGICAL_OPERATION_OVERRIDES: Dict[str, str] = { + "BidiReadObject": "ReadObject", + "BidiWriteObject": "WriteObject", +} + +_RPC_V2_METHODS: FrozenSet[str] = frozenset( + { + "BidiReadObject", + "BidiWriteObject", + "CancelResumableWrite", + "ComposeObject", + "CreateBucket", + "DeleteBucket", + "DeleteObject", + "GetBucket", + "GetIamPolicy", + "GetObject", + "ListBuckets", + "ListObjects", + "LockBucketRetentionPolicy", + "MoveObject", + "QueryWriteStatus", + "ReadObject", + "RestoreObject", + "RewriteObject", + "SetIamPolicy", + "StartResumableWrite", + "TestIamPermissions", + "UpdateBucket", + "UpdateObject", + "WriteObject", + } +) + + +def normalize_logical_operation(operation: Optional[str]) -> str: + """Normalizes an RPC path or method name into a short logical ``gcp.storage.operation``.""" + if not operation or not isinstance(operation, str): + return "UNKNOWN" + cleaned = operation.strip().strip("/") + if not cleaned: + return "UNKNOWN" + short_name = cleaned.rsplit("/", 1)[-1] if "/" in cleaned else cleaned + return _GRPC_LOGICAL_OPERATION_OVERRIDES.get(short_name, short_name) + + +def resolve_rpc_method( + operation: Optional[str], rpc_method: Optional[str] = None +) -> str: + """Resolves the full RPC path (e.g. ``google.storage.v2.Storage/ReadObject``) for ``rpc.method``.""" + raw = rpc_method if (rpc_method and isinstance(rpc_method, str)) else operation + if not raw or not isinstance(raw, str): + return "UNKNOWN" + cleaned = raw.strip().strip("/") + if not cleaned or cleaned == "UNKNOWN": + return "UNKNOWN" + if "/" in cleaned: + return cleaned + if cleaned in _RPC_V2_METHODS: + return f"google.storage.v2.Storage/{cleaned}" + if "." in cleaned: + return cleaned + return f"google.storage.v1.Storage/{cleaned}" + + +@dataclass class _OperationState: """Represents the active logical GCS Veneer operation in the current context.""" client_method: str + operation: str rpc_method: str suppressed: bool = False + start_time: float = field(default_factory=time.perf_counter) + finished: bool = False + last_failure_error_type: Optional[str] = None + recorded_attempts: int = 0 + + def mark_attempt_failed(self, error_type: str) -> None: + """Records the error type of a failed attempt for a subsequent retry.""" + if error_type and error_type != "OK": + self.last_failure_error_type = error_type + + def consume_retry_error(self) -> Optional[str]: + """Consumes and returns the prior failed attempt's error type, if any.""" + prior = self.last_failure_error_type + self.last_failure_error_type = None + return prior _OP_STATE: ContextVar[Optional[_OperationState]] = ContextVar( @@ -743,15 +835,19 @@ class _OperationState: def get_current_operation() -> Optional[_OperationState]: """Returns the current logical `_OperationState` in this execution context, if any.""" - return _OP_STATE.get() + op = _OP_STATE.get() + if op is not None and op.finished: + return None + return op @contextlib.contextmanager def start_operation( recorder: Optional["_MetricsRecorder"], client_method: str, - rpc_method: str, + operation: str, suppress: bool = False, + rpc_method: Optional[str] = None, ) -> Iterator[Optional[_OperationState]]: """Context manager tracking a logical user-facing GCS operation. @@ -763,24 +859,30 @@ def start_operation( recorder: Optional `_MetricsRecorder` configured on the client. client_method: Fully qualified Python method name (e.g. ``"storage.Blob.download_to_filename"``). - rpc_method: Canonical GCS RPC operation name - (e.g. ``"storage.objects.get"``). + operation: Short logical GCS operation name + (e.g. ``"ReadObject"``, ``"GetObject"``). suppress: If True, suppresses logical operation and payload size emission for this context and any nested calls (used by ``transfer_manager`` workers and ``BlobReader`` prefetches). + rpc_method: Optional full RPC path + (e.g. ``"google.storage.v2.Storage/BidiReadObject"``). If omitted, + inferred from ``operation``. """ - existing = _OP_STATE.get() + existing = get_current_operation() if existing is not None: if suppress and not existing.suppressed: suppressed_child = _OperationState( client_method=existing.client_method, + operation=existing.operation, rpc_method=existing.rpc_method, suppressed=True, + start_time=existing.start_time, ) child_token = _OP_STATE.set(suppressed_child) try: yield suppressed_child finally: + suppressed_child.finished = True _OP_STATE.reset(child_token) else: yield existing @@ -793,13 +895,19 @@ def start_operation( yield None return + resolved_operation = normalize_logical_operation(operation) + resolved_rpc_method = resolve_rpc_method( + resolved_operation, rpc_method=rpc_method or operation + ) + start_time = time.perf_counter() state = _OperationState( client_method=client_method, - rpc_method=rpc_method, + operation=resolved_operation, + rpc_method=resolved_rpc_method, suppressed=suppress, + start_time=start_time, ) token = _OP_STATE.set(state) - start_time = time.perf_counter() error_type = "OK" try: yield state @@ -807,17 +915,66 @@ def start_operation( error_type = classify_error_type(exc) raise finally: + state.finished = True duration = max(0.0, time.perf_counter() - start_time) _OP_STATE.reset(token) if recorder_active and not suppress and recorder is not None: recorder.record_logical_operation( duration=duration, client_method=client_method, - rpc_method=rpc_method, + operation=resolved_operation, error_type=error_type, + rpc_method=resolved_rpc_method, ) +def record_pending_retry( + recorder: Optional["_MetricsRecorder"], + operation: Optional[str] = None, + network_path: Optional[str] = None, +) -> None: + """Emits a pending ``gcp.storage.client.retries`` increment if a prior attempt failed.""" + if recorder is None or not getattr(recorder, "standard_enabled", False): + return + current_op = get_current_operation() + if current_op is None: + return + prior_error = current_op.consume_retry_error() + if prior_error is not None: + recorder.record_retry( + operation=operation or current_op.operation, + error_type=prior_error, + network_path=network_path, + ) + + +def handle_unrecorded_retryable_failure( + recorder: Optional["_MetricsRecorder"], + exc: BaseException, + attempts_before: int, + will_retry: bool, + network_path: Optional[str] = None, +) -> None: + """Tracks retry state when a failed attempt bypassed transport-level attempt recording.""" + if recorder is None or not getattr(recorder, "standard_enabled", False): + return + current_op = get_current_operation() + if current_op is None: + return + if current_op.recorded_attempts > attempts_before: + return + current_op.recorded_attempts += 1 + prior_error = current_op.consume_retry_error() + if prior_error is not None: + recorder.record_retry( + operation=current_op.operation, + error_type=prior_error, + network_path=network_path, + ) + if will_retry: + current_op.mark_attempt_failed(classify_error_type(exc)) + + def get_active_recorder(obj: Any) -> Optional["_MetricsRecorder"]: """Returns ``obj._metrics_recorder`` if it is an active ``_MetricsRecorder``.""" recorder = getattr(obj, "_metrics_recorder", None) @@ -899,6 +1056,11 @@ def __init__( unit="{attempt}", description="Number of physical network transport attempts executed.", ) + self.retries_counter = self.meter.create_counter( + METRIC_STORAGE_CLIENT_RETRIES, + unit="{retry}", + description="Number of transport retries executed after a failed attempt.", + ) self.request_body_size = self.meter.create_histogram( METRIC_STORAGE_REQUEST_BODY_SIZE, unit="By", @@ -909,11 +1071,12 @@ def __init__( unit="By", description="Application payload bytes delivered to the caller.", ) - self.operation_ttfb = self.meter.create_histogram( - METRIC_STORAGE_OPERATION_TTFB, + self.ttfb = self.meter.create_histogram( + METRIC_STORAGE_CLIENT_TTFB, unit="s", description="Time elapsed from attempt initiation to receipt of the first response byte.", ) + self.operation_ttfb = self.ttfb self.errors_counter = self.meter.create_counter( METRIC_STORAGE_CLIENT_ERRORS, unit="{error}", @@ -925,8 +1088,10 @@ def __init__( self.client_request_duration = None self.operations_counter = None self.attempts_counter = None + self.retries_counter = None self.request_body_size = None self.response_body_size = None + self.ttfb = None self.operation_ttfb = None self.errors_counter = None @@ -943,13 +1108,13 @@ def _build_operation_attributes( """Builds base attributes enriched with current or explicit operation metadata.""" attrs = self._base_attributes.copy() if current_op is None: - current_op = _OP_STATE.get() - resolved_op = operation or (current_op.rpc_method if current_op else None) + current_op = get_current_operation() + raw_op = operation or (current_op.operation if current_op else None) resolved_method = client_method or ( current_op.client_method if current_op else None ) - if resolved_op is not None: - attrs["gcp.storage.operation"] = resolved_op + if raw_op is not None: + attrs["gcp.storage.operation"] = normalize_logical_operation(raw_op) if resolved_method is not None: attrs["gcp.client.method"] = resolved_method return attrs @@ -958,20 +1123,52 @@ def record_logical_operation( self, duration: float, client_method: str, - rpc_method: str, + operation: str, error_type: str = "OK", + rpc_method: Optional[str] = None, ) -> None: """Records end-to-end logical operation duration and count.""" if not self.standard_enabled or self.meter is None: return - attrs = { + logical_op = normalize_logical_operation(operation) + full_rpc_method = resolve_rpc_method( + logical_op, rpc_method=rpc_method or operation + ) + op_attrs = { **self._base_attributes, - "gcp.storage.operation": rpc_method, + "gcp.storage.operation": logical_op, "gcp.client.method": client_method, "error.type": error_type, } - self.client_request_duration.record(max(0.0, duration), attributes=attrs) - self.operations_counter.add(1, attributes=attrs) + duration_attrs = { + **op_attrs, + "rpc.method": full_rpc_method, + } + self.client_request_duration.record( + max(0.0, duration), attributes=duration_attrs + ) + self.operations_counter.add(1, attributes=op_attrs) + + def record_retry( + self, + operation: Optional[str] = None, + error_type: str = "UNKNOWN", + network_path: Optional[str] = None, + ) -> None: + """Records a transport retry attempt with the prior failed attempt's error type.""" + if not self.standard_enabled or self.meter is None: + return + current_op = get_current_operation() + raw_op = operation or (current_op.operation if current_op else None) + retry_attrs: Dict[str, Any] = { + **self._base_attributes, + "error.type": error_type, + } + if raw_op is not None: + retry_attrs["gcp.storage.operation"] = normalize_logical_operation(raw_op) + if network_path is not None: + retry_attrs["network.path"] = network_path + self.retries_counter.add(1, attributes=retry_attrs) def record_http_attempt( self, @@ -987,11 +1184,24 @@ def record_http_attempt( """Records a single physical HTTP request attempt.""" if not self.standard_enabled or self.meter is None: return - current_op = _OP_STATE.get() - resolved_op = operation or (current_op.rpc_method if current_op else None) - resolved_method = client_method or ( - current_op.client_method if current_op else None + del client_method + current_op = get_current_operation() + raw_op = operation or (current_op.operation if current_op else None) + resolved_op = ( + normalize_logical_operation(raw_op) if raw_op is not None else None ) + if current_op is not None: + current_op.recorded_attempts += 1 + prior_error = current_op.consume_retry_error() + if prior_error is not None: + self.record_retry( + operation=resolved_op, + error_type=prior_error, + network_path=network_path, + ) + if error_type != "OK": + current_op.mark_attempt_failed(error_type) + valid_code = ( isinstance(status_code, int) and not isinstance(status_code, bool) @@ -1033,29 +1243,47 @@ def record_http_attempt( } if resolved_op is not None: error_attrs["gcp.storage.operation"] = resolved_op - if resolved_method is not None: - error_attrs["gcp.client.method"] = resolved_method + if network_path is not None: + error_attrs["network.path"] = network_path self.errors_counter.add(1, attributes=error_attrs) def record_rpc_attempt( self, duration: float, - operation: str, + operation: Optional[str] = None, status_code: Optional[str] = None, error_type: str = "OK", - client_method: Optional[str] = None, + rpc_method: Optional[str] = None, network_path: Optional[str] = None, + op_state: Optional[_OperationState] = None, ) -> None: """Records a single physical gRPC call attempt.""" if not self.standard_enabled or self.meter is None: return - op_attrs = self._build_operation_attributes( - operation=operation, client_method=client_method + active_op = op_state if op_state is not None else get_current_operation() + raw_rpc = ( + rpc_method or operation or (active_op.rpc_method if active_op else None) ) + resolved_rpc_method = resolve_rpc_method(operation, rpc_method=raw_rpc) + raw_op = operation or rpc_method or (active_op.operation if active_op else None) + resolved_op = normalize_logical_operation(raw_op) + + if active_op is not None: + active_op.recorded_attempts += 1 + prior_error = active_op.consume_retry_error() + if prior_error is not None: + self.record_retry( + operation=resolved_op, + error_type=prior_error, + network_path=network_path, + ) + if error_type != "OK": + active_op.mark_attempt_failed(error_type) # 1. rpc.client.call.duration attributes rpc_attrs: Dict[str, Any] = { - **op_attrs, + **self._base_attributes, + "rpc.method": resolved_rpc_method, "error.type": error_type, } if status_code is not None: @@ -1066,7 +1294,7 @@ def record_rpc_attempt( # 2. gcp.storage.client.attempts attributes attempt_attrs: Dict[str, Any] = { **self._base_attributes, - "gcp.storage.operation": operation, + "gcp.storage.operation": resolved_op, "error.type": error_type, } if status_code is not None: @@ -1080,9 +1308,12 @@ def record_rpc_attempt( # 3. gcp.storage.client.errors attributes if error_type != "OK": error_attrs: Dict[str, Any] = { - **op_attrs, + **self._base_attributes, + "gcp.storage.operation": resolved_op, "error.type": error_type, } + if network_path is not None: + error_attrs["network.path"] = network_path self.errors_counter.add(1, attributes=error_attrs) def record_request_body_size( @@ -1096,7 +1327,7 @@ def record_request_body_size( return if not isinstance(size, int) or isinstance(size, bool): return - current_op = _OP_STATE.get() + current_op = get_current_operation() if current_op is not None and current_op.suppressed: return attrs = self._build_operation_attributes( @@ -1115,7 +1346,7 @@ def record_response_body_size( return if not isinstance(size, int) or isinstance(size, bool): return - current_op = _OP_STATE.get() + current_op = get_current_operation() if current_op is not None and current_op.suppressed: return attrs = self._build_operation_attributes( @@ -1127,18 +1358,21 @@ def record_ttfb( self, duration: float, operation: Optional[str] = None, - client_method: Optional[str] = None, + network_path: Optional[str] = None, ) -> None: """Records time to first response byte (TTFB) in seconds.""" if not self.standard_enabled or self.meter is None: return - current_op = _OP_STATE.get() + current_op = get_current_operation() if current_op is not None and current_op.suppressed: return - attrs = self._build_operation_attributes( - operation=operation, client_method=client_method, current_op=current_op - ) - self.operation_ttfb.record(max(0.0, duration), attributes=attrs) + raw_op = operation or (current_op.operation if current_op else None) + attrs: Dict[str, Any] = self._base_attributes.copy() + if raw_op is not None: + attrs["gcp.storage.operation"] = normalize_logical_operation(raw_op) + if network_path is not None: + attrs["network.path"] = network_path + self.ttfb.record(max(0.0, duration), attributes=attrs) def shutdown(self) -> None: """Flushes pending metrics on a custom MeterProvider if configured.""" diff --git a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py index 79a2e76eaee5..f96cb39a7ca5 100644 --- a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py +++ b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py @@ -230,9 +230,14 @@ def test_otel_missing_disables_metrics(monkeypatch): def test_get_common_attributes(): """Verify common attributes conform to GCS OTel specification.""" attrs = _opentelemetry_metrics.get_common_attributes() + assert set(attrs.keys()) == { + "gcp.client.service", + "gcp.client.version", + "gcp.client.artifact", + } + assert "gcp.client.repo" not in attrs assert attrs["gcp.client.service"] == "storage" assert attrs["gcp.client.version"] == __version__ - assert attrs["gcp.client.repo"] == "googleapis/google-cloud-python" assert attrs["gcp.client.artifact"] == "google-cloud-storage" attrs["gcp.client.service"] = "mutated" @@ -316,12 +321,13 @@ def test_histogram_bucket_boundaries(): assert list(latency) == sorted(latency) sizes = _opentelemetry_metrics.SIZE_HISTOGRAM_BOUNDARIES - assert len(sizes) == 45 + assert len(sizes) == 48 assert sizes[0] == 0 assert sizes[1] == 128 * 1024 assert sizes[32] == 4 * 1024 * 1024 - assert sizes[33] == 8 * 1024 * 1024 - assert sizes[-1] == 16 * 1024 * 1024 * 1024 + assert sizes[33] == int(4.25 * 1024 * 1024) + assert sizes[34] == int(4.75 * 1024 * 1024) + assert sizes[-1] == 8593866752 assert list(sizes) == sorted(sizes) @@ -1111,7 +1117,7 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): "By", "Application payload bytes delivered to the caller.", ), - "gcp.storage.client.operation.ttfb": ( + "gcp.storage.client.ttfb": ( "s", "Time elapsed from attempt initiation to receipt of the first response byte.", ), @@ -1129,6 +1135,10 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): "{attempt}", "Number of physical network transport attempts executed.", ), + "gcp.storage.client.retries": ( + "{retry}", + "Number of transport retries executed after a failed attempt.", + ), "gcp.storage.client.errors": ( "{error}", "Number of physical network transport attempts terminating with a non-OK status.", @@ -1137,7 +1147,10 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): # Record within a logical operation context with _opentelemetry_metrics.start_operation( - recorder, "storage.Blob.upload_from_string", "storage.objects.insert" + recorder, + "storage.Blob.upload_from_string", + "WriteObject", + rpc_method="google.storage.v2.Storage/WriteObject", ): recorder.record_http_attempt( duration=0.05, @@ -1153,10 +1166,30 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): url_template="/upload/storage/v1/b/{bucket}/o", status_code=200, error_type="OK", + network_path="cloudpath", ) recorder.record_request_body_size(1024) recorder.record_response_body_size(512) - recorder.record_ttfb(0.01) + recorder.record_ttfb(0.01, network_path="cloudpath") + + # Logical operation metrics: duration has rpc.method; operations counter does not + recorder.client_request_duration.record.assert_called_once() + _, req_dur_kwargs = recorder.client_request_duration.record.call_args + assert req_dur_kwargs["attributes"]["gcp.storage.operation"] == "WriteObject" + assert ( + req_dur_kwargs["attributes"]["rpc.method"] + == "google.storage.v2.Storage/WriteObject" + ) + assert ( + req_dur_kwargs["attributes"]["gcp.client.method"] + == "storage.Blob.upload_from_string" + ) + assert req_dur_kwargs["attributes"]["error.type"] == "OK" + + recorder.operations_counter.add.assert_called_once() + _, op_cnt_kwargs = recorder.operations_counter.add.call_args + assert op_cnt_kwargs["attributes"]["gcp.storage.operation"] == "WriteObject" + assert "rpc.method" not in op_cnt_kwargs["attributes"] assert recorder.http_request_duration.record.call_count == 2 _, http_dur_kwargs = recorder.http_request_duration.record.call_args_list[0] @@ -1173,79 +1206,101 @@ def test_metrics_recorder_recording_and_shutdown(monkeypatch): assert recorder.attempts_counter.add.call_count == 2 _, attempt_kwargs = recorder.attempts_counter.add.call_args_list[0] - assert ( - attempt_kwargs["attributes"]["gcp.storage.operation"] - == "storage.objects.insert" - ) + assert attempt_kwargs["attributes"]["gcp.storage.operation"] == "WriteObject" assert attempt_kwargs["attributes"]["http.response.status_code"] == 503 assert attempt_kwargs["attributes"]["error.type"] == "UNAVAILABLE" assert attempt_kwargs["attributes"]["network.path"] == "cloudpath" assert "url.template" not in attempt_kwargs["attributes"] assert "http.request.method" not in attempt_kwargs["attributes"] + assert "gcp.client.method" not in attempt_kwargs["attributes"] - # Only the 503 attempt records an error + # Retry counter incremented once for the second attempt after 503 UNAVAILABLE + recorder.retries_counter.add.assert_called_once() + _, http_retry_kwargs = recorder.retries_counter.add.call_args + assert http_retry_kwargs["attributes"]["gcp.storage.operation"] == "WriteObject" + assert http_retry_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert http_retry_kwargs["attributes"]["network.path"] == "cloudpath" + assert "gcp.client.method" not in http_retry_kwargs["attributes"] + + # Only the 503 attempt records an error (no gcp.client.method on errors) recorder.errors_counter.add.assert_called_once() _, err_kwargs = recorder.errors_counter.add.call_args assert err_kwargs["attributes"]["error.type"] == "UNAVAILABLE" - assert err_kwargs["attributes"]["gcp.storage.operation"] == "storage.objects.insert" - assert ( - err_kwargs["attributes"]["gcp.client.method"] - == "storage.Blob.upload_from_string" - ) + assert err_kwargs["attributes"]["gcp.storage.operation"] == "WriteObject" + assert err_kwargs["attributes"]["network.path"] == "cloudpath" + assert "gcp.client.method" not in err_kwargs["attributes"] assert "url.template" not in err_kwargs["attributes"] assert "http.response.status_code" not in err_kwargs["attributes"] recorder.request_body_size.record.assert_called_once() recorder.response_body_size.record.assert_called_once() - recorder.operation_ttfb.record.assert_called_once() + recorder.ttfb.record.assert_called_once() + _, ttfb_kwargs = recorder.ttfb.record.call_args + assert ttfb_kwargs["attributes"]["gcp.storage.operation"] == "WriteObject" + assert ttfb_kwargs["attributes"]["network.path"] == "cloudpath" + assert "gcp.client.method" not in ttfb_kwargs["attributes"] - # gRPC attempt recording (OK and non-OK) + # gRPC attempt recording (UNAVAILABLE -> OK retry sequence) recorder.errors_counter.add.reset_mock() recorder.attempts_counter.add.reset_mock() - recorder.record_rpc_attempt( - duration=0.02, - operation="google.storage.v2.Storage/BidiReadObject", - status_code="OK", - error_type="OK", - client_method="storage.asyncio.AsyncMultiRangeDownloader.download_ranges", - network_path="directpath", - ) - recorder.record_rpc_attempt( - duration=0.03, - operation="google.storage.v2.Storage/BidiReadObject", - status_code="UNAVAILABLE", - error_type="UNAVAILABLE", - client_method="storage.asyncio.AsyncMultiRangeDownloader.download_ranges", - ) + recorder.retries_counter.add.reset_mock() + with _opentelemetry_metrics.start_operation( + recorder, + "storage.asyncio.AsyncMultiRangeDownloader.download_ranges", + "ReadObject", + rpc_method="google.storage.v2.Storage/BidiReadObject", + ): + recorder.record_rpc_attempt( + duration=0.03, + operation="ReadObject", + status_code="UNAVAILABLE", + error_type="UNAVAILABLE", + rpc_method="google.storage.v2.Storage/BidiReadObject", + network_path="directpath", + ) + recorder.record_rpc_attempt( + duration=0.02, + operation="ReadObject", + status_code="OK", + error_type="OK", + rpc_method="google.storage.v2.Storage/BidiReadObject", + network_path="directpath", + ) assert recorder.rpc_call_duration.record.call_count == 2 - _, rpc_dur_kwargs = recorder.rpc_call_duration.record.call_args_list[0] + _, rpc_dur_kwargs = recorder.rpc_call_duration.record.call_args_list[1] assert ( - rpc_dur_kwargs["attributes"]["gcp.storage.operation"] + rpc_dur_kwargs["attributes"]["rpc.method"] == "google.storage.v2.Storage/BidiReadObject" ) - assert ( - rpc_dur_kwargs["attributes"]["gcp.client.method"] - == "storage.asyncio.AsyncMultiRangeDownloader.download_ranges" - ) assert rpc_dur_kwargs["attributes"]["rpc.response.status_code"] == "OK" + assert rpc_dur_kwargs["attributes"]["error.type"] == "OK" assert rpc_dur_kwargs["attributes"]["network.path"] == "directpath" + assert "gcp.storage.operation" not in rpc_dur_kwargs["attributes"] + assert "gcp.client.method" not in rpc_dur_kwargs["attributes"] assert recorder.attempts_counter.add.call_count == 2 - _, rpc_attempt_kwargs = recorder.attempts_counter.add.call_args_list[0] - assert ( - rpc_attempt_kwargs["attributes"]["gcp.storage.operation"] - == "google.storage.v2.Storage/BidiReadObject" - ) + _, rpc_attempt_kwargs = recorder.attempts_counter.add.call_args_list[1] + assert rpc_attempt_kwargs["attributes"]["gcp.storage.operation"] == "ReadObject" assert rpc_attempt_kwargs["attributes"]["rpc.response.status_code"] == "OK" assert rpc_attempt_kwargs["attributes"]["error.type"] == "OK" assert rpc_attempt_kwargs["attributes"]["network.path"] == "directpath" assert "gcp.client.method" not in rpc_attempt_kwargs["attributes"] + assert "rpc.method" not in rpc_attempt_kwargs["attributes"] recorder.errors_counter.add.assert_called_once() _, rpc_err_kwargs = recorder.errors_counter.add.call_args + assert rpc_err_kwargs["attributes"]["gcp.storage.operation"] == "ReadObject" assert rpc_err_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert rpc_err_kwargs["attributes"]["network.path"] == "directpath" + assert "gcp.client.method" not in rpc_err_kwargs["attributes"] assert "rpc.response.status_code" not in rpc_err_kwargs["attributes"] + recorder.retries_counter.add.assert_called_once() + _, rpc_retry_kwargs = recorder.retries_counter.add.call_args + assert rpc_retry_kwargs["attributes"]["gcp.storage.operation"] == "ReadObject" + assert rpc_retry_kwargs["attributes"]["error.type"] == "UNAVAILABLE" + assert rpc_retry_kwargs["attributes"]["network.path"] == "directpath" + # Record outside of start_operation with status_code=None and callable .code() returning None recorder.record_http_attempt( duration=0.01, @@ -1334,9 +1389,205 @@ def test_metrics_recorder_disabled_no_ops(monkeypatch): # All recording methods are safe no-ops when disabled recorder.record_logical_operation(0.1, "m", "op") + recorder.record_retry("ReadObject", "UNAVAILABLE", "cloudpath") recorder.record_http_attempt(0.1, "GET", "/storage/v1/b") recorder.record_rpc_attempt(0.1, "google.storage.v2.Storage/ReadObject") recorder.record_request_body_size(100) recorder.record_response_body_size(100) recorder.record_ttfb(0.01) recorder.shutdown() + + +def test_normalize_logical_operation_and_resolve_rpc_method(): + assert _opentelemetry_metrics.normalize_logical_operation(None) == "UNKNOWN" + assert _opentelemetry_metrics.normalize_logical_operation("") == "UNKNOWN" + assert _opentelemetry_metrics.normalize_logical_operation(123) == "UNKNOWN" # type: ignore[arg-type] + assert _opentelemetry_metrics.normalize_logical_operation("///") == "UNKNOWN" + assert ( + _opentelemetry_metrics.normalize_logical_operation( + "google.storage.v2.Storage/BidiReadObject" + ) + == "ReadObject" + ) + assert ( + _opentelemetry_metrics.normalize_logical_operation( + "/google.storage.v2.Storage/BidiWriteObject" + ) + == "WriteObject" + ) + assert ( + _opentelemetry_metrics.normalize_logical_operation( + "google.storage.v2.Storage/GetObject" + ) + == "GetObject" + ) + assert ( + _opentelemetry_metrics.normalize_logical_operation("GetNotificationConfig") + == "GetNotificationConfig" + ) + + assert _opentelemetry_metrics.resolve_rpc_method(None, None) == "UNKNOWN" + assert _opentelemetry_metrics.resolve_rpc_method("", "") == "UNKNOWN" + assert _opentelemetry_metrics.resolve_rpc_method("///", None) == "UNKNOWN" + assert _opentelemetry_metrics.resolve_rpc_method("UNKNOWN", None) == "UNKNOWN" + assert ( + _opentelemetry_metrics.resolve_rpc_method( + "ReadObject", "google.storage.v2.Storage/BidiReadObject" + ) + == "google.storage.v2.Storage/BidiReadObject" + ) + assert ( + _opentelemetry_metrics.resolve_rpc_method("ReadObject") + == "google.storage.v2.Storage/ReadObject" + ) + assert ( + _opentelemetry_metrics.resolve_rpc_method("GetNotificationConfig") + == "google.storage.v1.Storage/GetNotificationConfig" + ) + assert ( + _opentelemetry_metrics.resolve_rpc_method("storage.objects.get") + == "storage.objects.get" + ) + + +def test_iter_exception_chain_traverses_errors_list(): + wrapped = api_exceptions.Aborted( + "aborted", + errors=[socket.timeout("nested timeout in errors")], + ) + chain = list(_opentelemetry_metrics._iter_exception_chain(wrapped)) + assert any(isinstance(item, socket.timeout) for item in chain) + + class _NonSeqErrorsErr(Exception): + errors = 123 + + assert _opentelemetry_metrics.classify_error_type(_NonSeqErrorsErr()) == "UNKNOWN" + + +def test_operation_state_and_retry_helpers(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + mock_metrics = mock.Mock() + mock_meter = mock.Mock() + mock_meter.create_histogram.side_effect = lambda *a, **kw: mock.Mock() + mock_meter.create_counter.side_effect = lambda *a, **kw: mock.Mock() + mock_metrics.get_meter.return_value = mock_meter + monkeypatch.setattr(_opentelemetry_metrics, "metrics", mock_metrics, raising=False) + + recorder = _opentelemetry_metrics._MetricsRecorder( + enable_metrics=True, rpc_system="grpc" + ) + disabled_recorder = _opentelemetry_metrics._MetricsRecorder(enable_metrics=False) + + # No-ops when recorder is None, disabled, or no active operation + _opentelemetry_metrics.record_pending_retry(None) + _opentelemetry_metrics.record_pending_retry(disabled_recorder) + _opentelemetry_metrics.record_pending_retry(recorder) + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + None, RuntimeError("e"), attempts_before=0, will_retry=True + ) + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + disabled_recorder, RuntimeError("e"), attempts_before=0, will_retry=True + ) + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + recorder, RuntimeError("e"), attempts_before=0, will_retry=True + ) + + # Standalone record_retry and record_ttfb outside start_operation without optional attrs + recorder.record_retry(operation=None, error_type="UNAVAILABLE", network_path=None) + _, retry_no_op_kw = recorder.retries_counter.add.call_args + assert "gcp.storage.operation" not in retry_no_op_kw["attributes"] + assert "network.path" not in retry_no_op_kw["attributes"] + + recorder.record_ttfb(0.02, operation=None, network_path=None) + _, ttfb_no_op_kw = recorder.ttfb.record.call_args + assert "gcp.storage.operation" not in ttfb_no_op_kw["attributes"] + assert "network.path" not in ttfb_no_op_kw["attributes"] + + recorder.retries_counter.add.reset_mock() + with _opentelemetry_metrics.start_operation( + recorder, + "storage.asyncio.AsyncMultiRangeDownloader.open", + "ReadObject", + rpc_method="google.storage.v2.Storage/BidiReadObject", + ) as op_state: + assert op_state is not None + op_state.mark_attempt_failed("OK") + op_state.mark_attempt_failed("") + assert op_state.last_failure_error_type is None + + # Pending retry when no failure occurred is a no-op + _opentelemetry_metrics.record_pending_retry(recorder) + recorder.retries_counter.add.assert_not_called() + + # When transport already recorded the attempt, handle_unrecorded_retryable_failure skips + op_state.recorded_attempts = 1 + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + recorder, + api_exceptions.ServiceUnavailable("503"), + attempts_before=0, + will_retry=True, + network_path="directpath", + ) + assert op_state.recorded_attempts == 1 + assert op_state.last_failure_error_type is None + + # Unrecorded failure 1 (will_retry=True) -> marks failure, no retry yet + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + recorder, + api_exceptions.ServiceUnavailable("503"), + attempts_before=1, + will_retry=True, + network_path="unknown", + ) + assert op_state.recorded_attempts == 2 + assert op_state.last_failure_error_type == "UNAVAILABLE" + recorder.retries_counter.add.assert_not_called() + + # Consecutive unrecorded failure 2 (will_retry=True) -> emits retry for failure 1, marks failure 2 + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + recorder, + api_exceptions.DeadlineExceeded("timeout"), + attempts_before=2, + will_retry=True, + network_path="cloudpath", + ) + assert op_state.recorded_attempts == 3 + assert op_state.last_failure_error_type == "DEADLINE_EXCEEDED" + recorder.retries_counter.add.assert_called_once() + assert ( + recorder.retries_counter.add.call_args.kwargs["attributes"]["error.type"] + == "UNAVAILABLE" + ) + + # Unrecorded terminal failure 3 (will_retry=False) -> emits retry for failure 2, leaves error cleared + _opentelemetry_metrics.handle_unrecorded_retryable_failure( + recorder, + api_exceptions.PermissionDenied("403"), + attempts_before=3, + will_retry=False, + network_path="cloudpath", + ) + assert op_state.recorded_attempts == 4 + assert op_state.last_failure_error_type is None + assert recorder.retries_counter.add.call_count == 2 + assert ( + recorder.retries_counter.add.call_args.kwargs["attributes"]["error.type"] + == "DEADLINE_EXCEEDED" + ) + + # Mark failure and consume via record_pending_retry + op_state.mark_attempt_failed("ABORTED") + _opentelemetry_metrics.record_pending_retry(recorder, network_path="directpath") + assert recorder.retries_counter.add.call_count == 3 + _, pending_kw = recorder.retries_counter.add.call_args + assert pending_kw["attributes"]["gcp.storage.operation"] == "ReadObject" + assert pending_kw["attributes"]["error.type"] == "ABORTED" + assert pending_kw["attributes"]["network.path"] == "directpath" + + import contextvars + + copied_ctx = contextvars.copy_context() + + # Once start_operation exits, state.finished is True even in copied contexts + assert op_state.finished is True + assert copied_ctx.run(_opentelemetry_metrics.get_current_operation) is None From cdea802ab7b5d128ef7227f5bd81598c5cabc3a0 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Mon, 26 Oct 2026 09:57:02 +0000 Subject: [PATCH 5/7] fix(storage): scope OTel histogram views, harden URI parsing, and unify CONNECTION_ERROR - Scope histogram View registrations to meter_name="google.cloud.storage" - Guard normalize_url_template and _extract_server_address against malformed URIs and gRPC authority schemes - Consolidate CONNECTION_REFUSED, CONNECTION_RESET, and CONNECTIVITY error labels into CONNECTION_ERROR [Generated-by: AI] --- .../cloud/storage/_opentelemetry_metrics.py | 49 ++++++++++++++----- .../tests/unit/test__opentelemetry_metrics.py | 30 ++++++++---- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py index f9583d5baba4..f511eff47eea 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py +++ b/packages/google-cloud-storage/google/cloud/storage/_opentelemetry_metrics.py @@ -187,6 +187,8 @@ def is_debug_metrics_enabled( # --------------------------------------------------------------------------- # 5. Standard Common Attributes & Meter Provider # --------------------------------------------------------------------------- +_METER_NAME = "google.cloud.storage" + _COMMON_ATTRIBUTES: Dict[str, Any] = { "gcp.client.service": "storage", "gcp.client.version": __version__, @@ -227,7 +229,7 @@ def get_meter( ): return None return metrics.get_meter( - "google.cloud.storage", + _METER_NAME, __version__, meter_provider=meter_provider, ) @@ -309,6 +311,7 @@ def get_histogram_views() -> List[Any]: views.append( View( instrument_name=name, + meter_name=_METER_NAME, aggregation=ExplicitBucketHistogramAggregation( boundaries=LATENCY_HISTOGRAM_BOUNDARIES ), @@ -318,6 +321,7 @@ def get_histogram_views() -> List[Any]: views.append( View( instrument_name=name, + meter_name=_METER_NAME, aggregation=ExplicitBucketHistogramAggregation( boundaries=SIZE_HISTOGRAM_BOUNDARIES ), @@ -507,10 +511,8 @@ def classify_error_type( {"NameResolutionError"}, ): return "DNS_FAILURE" - if isinstance(item, ConnectionRefusedError): - return "CONNECTION_REFUSED" - if isinstance(item, ConnectionResetError): - return "CONNECTION_RESET" + if isinstance(item, (ConnectionRefusedError, ConnectionResetError)): + return "CONNECTION_ERROR" # 8. Timeouts (Note: urllib3.exceptions.NewConnectionError subclasses ConnectTimeoutError # even for non-timeout connection failures, so exclude NewConnectionError here.) @@ -537,7 +539,7 @@ def classify_error_type( ): return "TIMEOUT" - # 9. Generic Connectivity Failure + # 9. Generic Connection / Network Transport Failure for item in chain: if ( _is_subclass_by_module( @@ -552,7 +554,7 @@ def classify_error_type( ) or isinstance(item, (OSError, http.client.HTTPException)) ): - return "CONNECTIVITY" + return "CONNECTION_ERROR" # 10. Explicit HTTP status_code fallback if valid_status and status_code is not None: @@ -708,7 +710,10 @@ def normalize_url_template(url_or_path: Optional[str]) -> str: if not url_or_path or not isinstance(url_or_path, str): return "UNKNOWN" - parsed = urlsplit(url_or_path) + try: + parsed = urlsplit(url_or_path) + except ValueError: + return "UNKNOWN" path = parsed.path or "" query = parsed.query or "" @@ -993,12 +998,30 @@ def _extract_server_address(api_endpoint: Optional[str]) -> str: endpoint = api_endpoint.strip() if not endpoint: return DEFAULT_SERVER_ADDRESS - if ":///" in endpoint: - endpoint = "https://" + endpoint.split(":///", 1)[1].lstrip("/") - elif "://" not in endpoint: + for grpc_prefix in ( + "dns:", + "google-c2p:", + "google-c2p-experimental:", + "xds:", + "ipv4:", + "ipv6:", + ): + if endpoint.startswith(grpc_prefix): + remainder = endpoint[len(grpc_prefix) :] + if remainder.startswith("//"): + remainder = remainder[2:] + remainder = ( + remainder.split("/", 1)[1] if "/" in remainder else remainder + ) + endpoint = remainder.lstrip("/") + break + if "://" not in endpoint: endpoint = "https://" + endpoint.lstrip("/") - parsed = urlsplit(endpoint) - return parsed.hostname or DEFAULT_SERVER_ADDRESS + try: + parsed = urlsplit(endpoint) + return parsed.hostname or DEFAULT_SERVER_ADDRESS + except ValueError: + return DEFAULT_SERVER_ADDRESS class _MetricsRecorder: diff --git a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py index f96cb39a7ca5..2e10d433cbe5 100644 --- a/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py +++ b/packages/google-cloud-storage/tests/unit/test__opentelemetry_metrics.py @@ -312,6 +312,7 @@ def test_get_meter(monkeypatch): def test_histogram_bucket_boundaries(): """Verify custom latency and byte-size bucket boundaries match specification.""" latency = _opentelemetry_metrics.LATENCY_HISTOGRAM_BOUNDARIES + assert len(latency) == 165 assert latency[0] == 0.0 assert latency[1] == 0.002 assert latency[49] == 0.098 @@ -340,9 +341,10 @@ def __init__(self, boundaries): self.boundaries = boundaries class FakeView: - def __init__(self, instrument_name, aggregation): + def __init__(self, instrument_name, aggregation, meter_name=None): self.instrument_name = instrument_name self.aggregation = aggregation + self.meter_name = meter_name fake_view_mod.ExplicitBucketHistogramAggregation = FakeAggregation fake_view_mod.View = FakeView @@ -361,6 +363,7 @@ def __init__(self, instrument_name, aggregation): views = _opentelemetry_metrics.get_histogram_views() assert len(views) == 6 + assert all(v.meter_name == "google.cloud.storage" for v in views) latency_views = [ v for v in views @@ -611,37 +614,37 @@ def code(self): wrapped_refused = requests.exceptions.ConnectionError(refused_err) assert ( _opentelemetry_metrics.classify_error_type(wrapped_refused) - == "CONNECTION_REFUSED" + == "CONNECTION_ERROR" ) reset_err = ConnectionResetError(104, "Connection reset by peer") wrapped_reset = requests.exceptions.ConnectionError(reset_err) assert ( - _opentelemetry_metrics.classify_error_type(wrapped_reset) == "CONNECTION_RESET" + _opentelemetry_metrics.classify_error_type(wrapped_reset) == "CONNECTION_ERROR" ) - # 10. Generic Connectivity + # 10. Generic Connection Errors assert ( _opentelemetry_metrics.classify_error_type( requests.exceptions.ConnectionError("connection aborted") ) - == "CONNECTIVITY" + == "CONNECTION_ERROR" ) assert ( _opentelemetry_metrics.classify_error_type( requests.exceptions.ChunkedEncodingError("incomplete read") ) - == "CONNECTIVITY" + == "CONNECTION_ERROR" ) assert ( _opentelemetry_metrics.classify_error_type(OSError("network unreachable")) - == "CONNECTIVITY" + == "CONNECTION_ERROR" ) assert ( _opentelemetry_metrics.classify_error_type( http.client.IncompleteRead(b"partial", 10) ) - == "CONNECTIVITY" + == "CONNECTION_ERROR" ) # 11. Explicit status_code fallback when exception is generic @@ -875,6 +878,7 @@ def code(self): ("https://storage.googleapis.com/storage/v2", "UNKNOWN"), ("https://storage.googleapis.com/storage/v2/other?uploadId=xyz", "UNKNOWN"), ("https://storage.googleapis.com/custom/unrecognized/path/here", "UNKNOWN"), + ("https://[::1/storage/v1/b", "UNKNOWN"), ], ) def test_normalize_url_template(raw_url, expected): @@ -1061,6 +1065,14 @@ async def run_all(): ("https://[::1]:8080", "::1"), ("http://localhost:9023/storage/v1", "localhost"), ("https://", "storage.googleapis.com"), + ("https://[::1", "storage.googleapis.com"), + ("[::1", "storage.googleapis.com"), + ("dns://8.8.8.8/custom.endpoint.com:443", "custom.endpoint.com"), + ("dns:custom.endpoint.com:443", "custom.endpoint.com"), + ("dns://custom.endpoint.com:443", "custom.endpoint.com"), + ("dns://8.8.8.8/", "storage.googleapis.com"), + ("ipv4:127.0.0.1:443", "127.0.0.1"), + ("ipv6:[::1]:443", "::1"), ], ) def test_extract_server_address(endpoint, expected): @@ -1340,7 +1352,7 @@ class _NonTupleArgsErr(Exception): _opentelemetry_metrics.classify_error_type( urllib3.exceptions.NewConnectionError(None, "conn err") ) - == "CONNECTIVITY" + == "CONNECTION_ERROR" ) # Shutdown flushes custom meter_provider and swallows force_flush errors gracefully From 04b2dbd2f090c580f81d0669686809fcb90bbad6 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Sun, 1 Nov 2026 18:31:06 +0000 Subject: [PATCH 6/7] feat(storage): add OpenTelemetry standard metrics for HTTP JSON transport - Wire _MetricsRecorder lifecycle, enable_metrics, and meter_provider into Client - Instrument Connection.api_request for per-attempt HTTP duration, TTFB, retries, and errors - Add _start_storage_operation, _PropertyMixin, and lazy HTTPIterator page metrics helpers - Skip per-attempt HTTP metrics while queuing sub-requests into Batch - Suppress internal background BucketMetadataCache lookups from emitting user-facing metrics --- .../cloud/storage/_bucket_metadata_cache.py | 11 +- .../google/cloud/storage/_helpers.py | 290 +++++++--- .../google/cloud/storage/_http.py | 171 +++++- .../google/cloud/storage/batch.py | 1 + .../google/cloud/storage/client.py | 67 ++- .../unit/test_otel_http_transport_metrics.py | 535 ++++++++++++++++++ 6 files changed, 984 insertions(+), 91 deletions(-) create mode 100644 packages/google-cloud-storage/tests/unit/test_otel_http_transport_metrics.py diff --git a/packages/google-cloud-storage/google/cloud/storage/_bucket_metadata_cache.py b/packages/google-cloud-storage/google/cloud/storage/_bucket_metadata_cache.py index e431d00144fb..1c3f83e372a7 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_bucket_metadata_cache.py +++ b/packages/google-cloud-storage/google/cloud/storage/_bucket_metadata_cache.py @@ -20,6 +20,7 @@ from google.api_core import exceptions as api_exceptions from google.cloud.exceptions import NotFound +from google.cloud.storage import _helpers from google.cloud.storage._lru_cache import LRUCache logger = logging.getLogger(__name__) @@ -81,9 +82,10 @@ def check_and_evict(self, bucket_name): def _verify_existence_background(self, bucket_name): try: - bucket = self._client.bucket(bucket_name) - if not bucket.exists(): - self.evict(bucket_name) + with _helpers._suppress_internal_metrics(): + bucket = self._client.bucket(bucket_name) + if not bucket.exists(): + self.evict(bucket_name) except Exception as e: logger.debug( f"Background verification for bucket existence failed for {bucket_name}: {e}" @@ -95,7 +97,8 @@ def _verify_existence_background(self, bucket_name): def _fetch_background(self, bucket_name): """Asynchronously fetch bucket metadata and update the cache.""" try: - bucket = self._client.get_bucket(bucket_name, timeout=10.0) + with _helpers._suppress_internal_metrics(): + bucket = self._client.get_bucket(bucket_name, timeout=10.0) self.update_from_bucket(bucket) except (NotFound, api_exceptions.NotFound): self.evict(bucket_name) diff --git a/packages/google-cloud-storage/google/cloud/storage/_helpers.py b/packages/google-cloud-storage/google/cloud/storage/_helpers.py index 9bea6c40845e..1c089c7e5a7a 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_helpers.py +++ b/packages/google-cloud-storage/google/cloud/storage/_helpers.py @@ -23,7 +23,8 @@ import os import secrets import sys -from contextlib import contextmanager +from contextlib import contextmanager, nullcontext +from contextvars import ContextVar from hashlib import md5 from urllib.parse import urlsplit, urlunsplit from uuid import uuid4 @@ -32,6 +33,7 @@ from google.auth import environment_vars from google.cloud.exceptions import NotFound +from google.cloud.storage import _opentelemetry_metrics from google.cloud.storage._opentelemetry_tracing import ( _is_bucket_metadata_disabled, ) @@ -46,6 +48,107 @@ _logger = logging.getLogger(__name__) +_SUPPRESS_INTERNAL_METRICS = ContextVar("_gcs_suppress_internal_metrics", default=False) +_SUPPRESS_BODY_SIZE = ContextVar("_gcs_suppress_body_size", default=False) +_LIST_METRICS_CONTEXT = ContextVar("_gcs_list_metrics_context", default=None) +_CLIENT_METHOD_OVERRIDE = ContextVar("_gcs_client_method_override", default=None) +_ACTIVE_METRICS_RECORDER = ContextVar("_gcs_active_metrics_recorder", default=None) + + +def _resolve_operation_client(obj, client=None): + """Best-effort resolution of a client instance without raising if unbound.""" + if client is not None: + return client + try: + return obj.client + except Exception: + return getattr(obj, "_client", None) + + +@contextmanager +def _suppress_internal_metrics(): + """Suppresses all GCS standard metrics within internal background/cleanup calls.""" + token = _SUPPRESS_INTERNAL_METRICS.set(True) + try: + yield + finally: + _SUPPRESS_INTERNAL_METRICS.reset(token) + + +@contextmanager +def _suppress_body_size(): + """Suppresses per-chunk body-size emission while keeping TTFB/attempts active.""" + token = _SUPPRESS_BODY_SIZE.set(True) + try: + yield + finally: + _SUPPRESS_BODY_SIZE.reset(token) + + +@contextmanager +def _list_metrics_context(client_method, operation): + """Propagates outermost list Veneer attribution into ``Client._list_resource``.""" + if _LIST_METRICS_CONTEXT.get() is not None: + yield + return + token = _LIST_METRICS_CONTEXT.set((client_method, operation)) + try: + yield + finally: + _LIST_METRICS_CONTEXT.reset(token) + + +@contextmanager +def _client_method_override(client_method): + """Overrides ``gcp.client.method`` for outermost composite helpers (e.g. ``transfer_manager``).""" + if _CLIENT_METHOD_OVERRIDE.get() is not None: + yield + return + token = _CLIENT_METHOD_OVERRIDE.set(client_method) + try: + yield + finally: + _CLIENT_METHOD_OVERRIDE.reset(token) + + +_NON_BATCHABLE_OPERATIONS = frozenset( + {"Batch", "ReadObject", "WriteObject", "StartResumableWrite"} +) + + +@contextmanager +def _start_storage_operation(client, client_method, operation): + """Starts a logical GCS standard metrics operation if enabled on ``client``.""" + if _SUPPRESS_INTERNAL_METRICS.get(): + yield None + return + + recorder = _opentelemetry_metrics.get_active_recorder(client) + if recorder is None: + yield None + return + + current_batch = getattr(client, "current_batch", None) + if ( + operation not in _NON_BATCHABLE_OPERATIONS + and getattr(current_batch, "_IS_BATCH", False) is True + ): + yield None + return + + effective_method = _CLIENT_METHOD_OVERRIDE.get() or client_method + rec_token = _ACTIVE_METRICS_RECORDER.set(recorder) + try: + with _opentelemetry_metrics.start_operation( + recorder=recorder, + client_method=effective_method, + operation=operation, + ) as state: + yield state + finally: + _ACTIVE_METRICS_RECORDER.reset(rec_token) + + STORAGE_EMULATOR_ENV_VAR = "STORAGE_EMULATOR_HOST" # Despite name, includes scheme. """Environment variable defining host for Storage emulator.""" @@ -219,6 +322,10 @@ class _PropertyMixin(object): number or letter. """ + _METRICS_CLASS_NAME = None + _METRICS_GET_OP = None + _METRICS_UPDATE_OP = None + def __init__(self, name=None): self.name = name self._properties = {} @@ -377,35 +484,47 @@ def reload( See: https://cloud.google.com/storage/docs/soft-delete """ client = self._require_client(client) - query_params = self._query_params - # Pass only '?projection=noAcl' here because 'acl' and related - # are handled via custom endpoints. - query_params["projection"] = projection - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - if soft_deleted is not None: - query_params["softDeleted"] = soft_deleted - # Soft delete reload requires a generation, even for targets - # that don't include them in default query params (buckets). - query_params["generation"] = self.generation - headers = self._encryption_headers() - _add_etag_match_headers( - headers, if_etag_match=if_etag_match, if_etag_not_match=if_etag_not_match - ) - api_response = client._get_resource( - self.path, - query_params=query_params, - headers=headers, - timeout=timeout, - retry=retry, - _target_object=self, - ) - self._set_properties(api_response) + op_name = self._METRICS_GET_OP + if op_name is not None: + cls_name = self._METRICS_CLASS_NAME or type(self).__name__ + op_ctx = _start_storage_operation( + client, f"storage.{cls_name}.reload", op_name + ) + else: + op_ctx = nullcontext() + + with op_ctx: + query_params = self._query_params + # Pass only '?projection=noAcl' here because 'acl' and related + # are handled via custom endpoints. + query_params["projection"] = projection + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + if soft_deleted is not None: + query_params["softDeleted"] = soft_deleted + # Soft delete reload requires a generation, even for targets + # that don't include them in default query params (buckets). + query_params["generation"] = self.generation + headers = self._encryption_headers() + _add_etag_match_headers( + headers, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, + ) + api_response = client._get_resource( + self.path, + query_params=query_params, + headers=headers, + timeout=timeout, + retry=retry, + _target_object=self, + ) + self._set_properties(api_response) def _patch_property(self, name, value): """Update field of this object's properties. @@ -490,31 +609,41 @@ def patch( https://cloud.google.com/storage/docs/json_api/v1/objects/patch """ client = self._require_client(client) - query_params = self._query_params - # Pass '?projection=full' here because 'PATCH' documented not - # to work properly w/ 'noAcl'. - query_params["projection"] = "full" - if override_unlocked_retention: - query_params["overrideUnlockedRetention"] = override_unlocked_retention - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - update_properties = {key: self._properties[key] for key in self._changes} - - # Make the API call. - api_response = client._patch_resource( - self.path, - update_properties, - query_params=query_params, - _target_object=self, - timeout=timeout, - retry=retry, - ) - self._set_properties(api_response) + op_name = self._METRICS_UPDATE_OP + if op_name is not None: + cls_name = self._METRICS_CLASS_NAME or type(self).__name__ + op_ctx = _start_storage_operation( + client, f"storage.{cls_name}.patch", op_name + ) + else: + op_ctx = nullcontext() + + with op_ctx: + query_params = self._query_params + # Pass '?projection=full' here because 'PATCH' documented not + # to work properly w/ 'noAcl'. + query_params["projection"] = "full" + if override_unlocked_retention: + query_params["overrideUnlockedRetention"] = override_unlocked_retention + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + update_properties = {key: self._properties[key] for key in self._changes} + + # Make the API call. + api_response = client._patch_resource( + self.path, + update_properties, + query_params=query_params, + _target_object=self, + timeout=timeout, + retry=retry, + ) + self._set_properties(api_response) def update( self, @@ -571,28 +700,37 @@ def update( https://cloud.google.com/storage/docs/json_api/v1/objects/patch """ client = self._require_client(client) + op_name = self._METRICS_UPDATE_OP + if op_name is not None: + cls_name = self._METRICS_CLASS_NAME or type(self).__name__ + op_ctx = _start_storage_operation( + client, f"storage.{cls_name}.update", op_name + ) + else: + op_ctx = nullcontext() + + with op_ctx: + query_params = self._query_params + query_params["projection"] = "full" + if override_unlocked_retention: + query_params["overrideUnlockedRetention"] = override_unlocked_retention + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) - query_params = self._query_params - query_params["projection"] = "full" - if override_unlocked_retention: - query_params["overrideUnlockedRetention"] = override_unlocked_retention - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - - api_response = client._put_resource( - self.path, - self._properties, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=self, - ) - self._set_properties(api_response) + api_response = client._put_resource( + self.path, + self._properties, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=self, + ) + self._set_properties(api_response) def _scalar_property(fieldname): diff --git a/packages/google-cloud-storage/google/cloud/storage/_http.py b/packages/google-cloud-storage/google/cloud/storage/_http.py index 72734f511b41..5b0eaa432cdd 100644 --- a/packages/google-cloud-storage/google/cloud/storage/_http.py +++ b/packages/google-cloud-storage/google/cloud/storage/_http.py @@ -17,12 +17,14 @@ import functools import logging import re +import time +from contextvars import ContextVar from google.api_core import exceptions as api_exceptions from google.cloud.exceptions import NotFound from google.cloud import _http -from google.cloud.storage import __version__, _helpers +from google.cloud.storage import __version__, _helpers, _opentelemetry_metrics from google.cloud.storage._opentelemetry_tracing import ( HAS_OPENTELEMETRY, _is_bucket_metadata_disabled, @@ -32,6 +34,9 @@ logger = logging.getLogger(__name__) +_IN_API_REQUEST = ContextVar("_gcs_in_api_request", default=False) +_LAST_ATTEMPT_RESPONSE = ContextVar("_gcs_last_attempt_response", default=None) + class Connection(_http.JSONConnection): """A connection to Google Cloud Storage via the JSON REST API. @@ -55,6 +60,7 @@ class Connection(_http.JSONConnection): DEFAULT_API_ENDPOINT = _helpers._get_default_storage_base_url() DEFAULT_API_MTLS_ENDPOINT = "https://storage.mtls.googleapis.com" + _IS_BATCH = False def __init__(self, client, client_info=None, api_endpoint=None): super(Connection, self).__init__(client, client_info) @@ -76,6 +82,64 @@ def __init__(self, client, client_info=None, api_endpoint=None): API_URL_TEMPLATE = "{api_base_url}/storage/{api_version}{path}" """A template for the URL of a particular API call.""" + def _make_request(self, method, url, *args, **kwargs): + if ( + getattr(self, "_IS_BATCH", False) is True + or _helpers._SUPPRESS_INTERNAL_METRICS.get() + ): + return super(Connection, self)._make_request(method, url, *args, **kwargs) + + recorder = _opentelemetry_metrics.get_active_recorder(self._client) + if recorder is None: + return super(Connection, self)._make_request(method, url, *args, **kwargs) + + if _IN_API_REQUEST.get(): + req_start = time.perf_counter() + response = super(Connection, self)._make_request( + method, url, *args, **kwargs + ) + _LAST_ATTEMPT_RESPONSE.set((response, time.perf_counter() - req_start, url)) + return response + + url_template = _opentelemetry_metrics.normalize_url_template(url) + attempt_start = time.perf_counter() + try: + response = super(Connection, self)._make_request( + method, url, *args, **kwargs + ) + except BaseException as exc: + recorder.record_http_attempt( + duration=time.perf_counter() - attempt_start, + http_method=method, + url_template=url_template, + status_code=None, + error_type=_opentelemetry_metrics.classify_error_type(exc), + network_path="cloudpath", + ) + raise + + duration = time.perf_counter() - attempt_start + raw_sc = getattr(response, "status_code", None) + status_code = ( + raw_sc + if (isinstance(raw_sc, int) and not isinstance(raw_sc, bool)) + else None + ) + if status_code is not None: + recorder.record_ttfb(duration, network_path="cloudpath") + error_type = _opentelemetry_metrics.classify_error_type( + None, status_code=status_code + ) + recorder.record_http_attempt( + duration=duration, + http_method=method, + url_template=url_template, + status_code=status_code, + error_type=error_type, + network_path="cloudpath", + ) + return response + def api_request(self, *args, **kwargs): retry = kwargs.pop("retry", None) invocation_id = _helpers._get_invocation_id() @@ -103,7 +167,102 @@ def api_request(self, *args, **kwargs): except Exception as e: logger.debug(f"Failed cache.get_or_queue_fetch in api_request: {e}") - call = functools.partial(super(Connection, self).api_request, *args, **kwargs) + base_call = functools.partial( + super(Connection, self).api_request, *args, **kwargs + ) + recorder = ( + None + if ( + getattr(self, "_IS_BATCH", False) is True + or _helpers._SUPPRESS_INTERNAL_METRICS.get() + ) + else _opentelemetry_metrics.get_active_recorder(client) + ) + + if recorder is not None: + http_method = args[0] if len(args) >= 1 else kwargs.get("method", "") + raw_path = args[1] if len(args) >= 2 else kwargs.get("path", "") + fallback_template = _opentelemetry_metrics.normalize_url_template(raw_path) + + def _timed_attempt(): + _LAST_ATTEMPT_RESPONSE.set(None) + attempt_start = time.perf_counter() + try: + result = base_call() + except BaseException as exc: + duration = time.perf_counter() - attempt_start + stashed = _LAST_ATTEMPT_RESPONSE.get() + _LAST_ATTEMPT_RESPONSE.set(None) + if stashed is not None: + resp, ttfb_dur, req_url = stashed + raw_sc = getattr(resp, "status_code", None) + status_code = ( + raw_sc + if ( + isinstance(raw_sc, int) and not isinstance(raw_sc, bool) + ) + else None + ) + url_template = ( + _opentelemetry_metrics.normalize_url_template(req_url) + if req_url + else fallback_template + ) + if status_code is not None: + recorder.record_ttfb(ttfb_dur, network_path="cloudpath") + err_type = _opentelemetry_metrics.classify_error_type( + exc, status_code=status_code + ) + else: + status_code = None + url_template = fallback_template + err_type = _opentelemetry_metrics.classify_error_type(exc) + recorder.record_http_attempt( + duration=duration, + http_method=http_method, + url_template=url_template, + status_code=status_code, + error_type=err_type, + network_path="cloudpath", + ) + raise + + duration = time.perf_counter() - attempt_start + stashed = _LAST_ATTEMPT_RESPONSE.get() + _LAST_ATTEMPT_RESPONSE.set(None) + status_code = 200 + url_template = fallback_template + if stashed is not None: + resp, ttfb_dur, req_url = stashed + raw_sc = getattr(resp, "status_code", None) + status_code = ( + raw_sc + if (isinstance(raw_sc, int) and not isinstance(raw_sc, bool)) + else None + ) + if req_url: + url_template = _opentelemetry_metrics.normalize_url_template( + req_url + ) + if status_code is not None: + recorder.record_ttfb(ttfb_dur, network_path="cloudpath") + else: + recorder.record_ttfb(duration, network_path="cloudpath") + + recorder.record_http_attempt( + duration=duration, + http_method=http_method, + url_template=url_template, + status_code=status_code, + error_type="OK", + network_path="cloudpath", + ) + return result + + call = _timed_attempt + else: + call = base_call + with create_trace_span( name="Storage.Connection.api_request", attributes=span_attributes, @@ -119,6 +278,7 @@ def api_request(self, *args, **kwargs): pass if retry: call = retry(call) + in_api_token = _IN_API_REQUEST.set(True) if recorder is not None else None try: return call() except (NotFound, api_exceptions.NotFound): @@ -140,3 +300,10 @@ def api_request(self, *args, **kwargs): f"Failed cache.check_and_evict on 404 in api_request: {e}" ) raise + finally: + if in_api_token is not None: + _IN_API_REQUEST.reset(in_api_token) + _LAST_ATTEMPT_RESPONSE.set(None) + current_op = _opentelemetry_metrics.get_current_operation() + if current_op is not None: + current_op.last_failure_error_type = None diff --git a/packages/google-cloud-storage/google/cloud/storage/batch.py b/packages/google-cloud-storage/google/cloud/storage/batch.py index aed3703eb03d..d680a103bb91 100644 --- a/packages/google-cloud-storage/google/cloud/storage/batch.py +++ b/packages/google-cloud-storage/google/cloud/storage/batch.py @@ -163,6 +163,7 @@ class Batch(Connection): """ _MAX_BATCH_SIZE = 1000 + _IS_BATCH = True def __init__(self, client, raise_exception=True): api_endpoint = client._connection.API_BASE_URL diff --git a/packages/google-cloud-storage/google/cloud/storage/client.py b/packages/google-cloud-storage/google/cloud/storage/client.py index f9d354a4d269..dc4b393979a7 100644 --- a/packages/google-cloud-storage/google/cloud/storage/client.py +++ b/packages/google-cloud-storage/google/cloud/storage/client.py @@ -37,6 +37,7 @@ from google.cloud.storage._helpers import ( _DEFAULT_SCHEME, _DEFAULT_UNIVERSE_DOMAIN, + _LIST_METRICS_CONTEXT, _NOW, _STORAGE_HOST_TEMPLATE, _UTC, @@ -45,6 +46,7 @@ _get_api_endpoint_override, _get_environ_project, _get_storage_emulator_override, + _start_storage_operation, _virtual_hosted_style_base_url, create_trace_span_helper, ) @@ -140,6 +142,14 @@ class Client(ClientWithProject): metrics. If ``None``, falls back to the ``GCP_STORAGE_PYTHON_ENABLE_OTEL_DEBUG_METRICS`` environment variable, or ``False`` if unset. + + :type meter_provider: :class:`opentelemetry.metrics.MeterProvider` or None + :param meter_provider: + (Optional, Experimental) Custom OpenTelemetry ``MeterProvider`` for + recording client metrics. If ``None``, falls back to the global + OpenTelemetry ``MeterProvider``. Note that custom ``MeterProvider`` + instances are not serialized across ``ProcessPoolExecutor`` workers in + :mod:`~google.cloud.storage.transfer_manager`. """ SCOPE = ( @@ -162,6 +172,7 @@ def __init__( api_key=None, enable_metrics=None, enable_debug_metrics=None, + meter_provider=None, ): self._base_connection = None @@ -181,12 +192,7 @@ 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 - ) + self._meter_provider = meter_provider connection_kw_args = {"client_info": client_info} @@ -317,19 +323,50 @@ def __init__( self._connection = connection self._batch_stack = _LocalStack() self._bucket_metadata_cache = BucketMetadataCache(self) + self._metrics_recorder = _opentelemetry_metrics._MetricsRecorder( + enable_metrics=enable_metrics, + enable_debug_metrics=enable_debug_metrics, + meter_provider=meter_provider, + api_endpoint=self.api_endpoint, + rpc_system="http", + ) + if getattr(self, "_http_internal", None) is not None: + try: + self._http_internal._gcs_metrics_recorder = self._metrics_recorder + except Exception: + pass + + @property + def _http(self): + http = super(Client, self)._http + recorder = getattr(self, "_metrics_recorder", None) + if ( + http is not None + and recorder is not None + and getattr(http, "_gcs_metrics_recorder", None) is not recorder + ): + try: + http._gcs_metrics_recorder = recorder + except Exception: + pass + return http @property def metrics_enabled(self) -> bool: """(Experimental) Returns ``True`` if OpenTelemetry metrics recording is active for this client.""" - return self._metrics_enabled + recorder = getattr(self, "_metrics_recorder", None) + return bool(recorder is not None and recorder.standard_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 + recorder = getattr(self, "_metrics_recorder", None) + return bool(recorder is not None and recorder.debug_enabled) def close(self): """Close the client and clear any cached metadata or active connections.""" + if hasattr(self, "_metrics_recorder") and self._metrics_recorder is not None: + self._metrics_recorder.shutdown() if hasattr(self, "_bucket_metadata_cache") and self._bucket_metadata_cache: self._bucket_metadata_cache.clear() if hasattr(self._http, "close"): @@ -603,9 +640,21 @@ def _list_resource( api_request=kwargs, retry=retry, ): - api_request = functools.partial( + raw_api_request = functools.partial( self._connection.api_request, timeout=timeout, retry=retry ) + list_ctx = _LIST_METRICS_CONTEXT.get() + if ( + list_ctx is not None + and _opentelemetry_metrics.get_active_recorder(self) is not None + ): + client_method, operation = list_ctx + + def api_request(*args, **kw): + with _start_storage_operation(self, client_method, operation): + return raw_api_request(*args, **kw) + else: + api_request = raw_api_request return page_iterator.HTTPIterator( client=self, api_request=api_request, diff --git a/packages/google-cloud-storage/tests/unit/test_otel_http_transport_metrics.py b/packages/google-cloud-storage/tests/unit/test_otel_http_transport_metrics.py new file mode 100644 index 000000000000..a93242efdecc --- /dev/null +++ b/packages/google-cloud-storage/tests/unit/test_otel_http_transport_metrics.py @@ -0,0 +1,535 @@ +# Copyright 2026 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. + +"""Unit tests for OpenTelemetry HTTP JSON transport, Client lifecycle, and core helpers.""" + +import functools +import http.client +import json +from unittest import mock + +import pytest +import requests +from google.api_core import retry +from google.auth import credentials as auth_credentials +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import InMemoryMetricReader +from opentelemetry.sdk.trace import TracerProvider + +from google.cloud.storage import _helpers, _opentelemetry_metrics +from google.cloud.storage._bucket_metadata_cache import BucketMetadataCache +from google.cloud.storage.client import Client + + +def _make_credentials(): + return auth_credentials.AnonymousCredentials() + + +def _json_response(payload, status_code=http.client.OK): + response = requests.Response() + response.status_code = status_code + response.headers.update({"Content-Type": "application/json"}) + response._content = json.dumps(payload).encode("utf-8") + response.request = requests.Request( + "GET", "https://storage.googleapis.com/storage/v1/b" + ).prepare() + return response + + +def _collect_metrics(reader): + data = reader.get_metrics_data() + collected = {} + if data is None: # pragma: NO COVER + return collected + for resource_metric in data.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + collected.setdefault(metric.name, []).extend(metric.data.data_points) + return collected + + +@pytest.fixture(autouse=True) +def _enable_dev_gate(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + monkeypatch.setattr(_opentelemetry_metrics, "HAS_OPENTELEMETRY_METRICS", True) + + +@pytest.fixture +def metrics_env(): + reader = InMemoryMetricReader() + provider = MeterProvider(metric_readers=[reader]) + session = mock.MagicMock() + client = Client( + project="test-project", + credentials=_make_credentials(), + _http=session, + enable_metrics=True, + meter_provider=provider, + ) + # Disable async daemon cache fetches from consuming session.request.side_effect + client._bucket_metadata_cache = None + yield client, session, reader + client.close() + + +def test_client_metrics_disabled_by_default_and_fast_paths(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", False) + session = mock.MagicMock() + session.request.return_value = _json_response({"items": []}) + client = Client( + project="test-project", + credentials=_make_credentials(), + _http=session, + enable_metrics=True, + ) + assert client._metrics_recorder.enabled is False + with _helpers._list_metrics_context("storage.Client.list_buckets", "ListBuckets"): + iterator = client._list_resource("/b", lambda it, item: item) + assert isinstance(iterator.api_request, functools.partial) + assert list(iterator) == [] + resp = client._connection._make_request("GET", "/b") + assert resp.status_code == http.client.OK + client.close() + + +def test_client_custom_endpoint_and_owned_shutdown(): + session = mock.MagicMock() + session.request.return_value = _json_response({"email_address": "sa@example.com"}) + reader = InMemoryMetricReader() + provider = MeterProvider(metric_readers=[reader]) + client = Client( + project="test-project", + credentials=_make_credentials(), + _http=session, + client_options={"api_endpoint": "https://custom.storage.example.com:8443"}, + enable_metrics=True, + meter_provider=provider, + ) + with _helpers._start_storage_operation( + client, + "storage.Client.get_service_account_email", + "GetServiceAccount", + ): + resp = client._get_resource("/projects/test-project/serviceAccount") + assert resp["email_address"] == "sa@example.com" + + pts = _collect_metrics(reader) + op_pt = pts["gcp.storage.client.operations"][0] + dur_pt = pts["gcp.client.request.duration"][0] + assert op_pt.attributes["server.address"] == "custom.storage.example.com" + assert dur_pt.attributes["server.address"] == "custom.storage.example.com" + assert op_pt.attributes["rpc.system.name"] == "http" + + att_pt = pts["gcp.storage.client.attempts"][0] + http_dur_pt = pts["http.client.request.duration"][0] + assert att_pt.attributes["server.address"] == "custom.storage.example.com" + assert http_dur_pt.attributes["server.address"] == "custom.storage.example.com" + assert att_pt.attributes["network.path"] == "cloudpath" + + with client: + pass + + with mock.patch.object(client._metrics_recorder, "shutdown") as mock_shutdown: + client.close() + mock_shutdown.assert_called_once() + + # Immutable _http and missing _metrics_recorder on close() are safe no-ops + ro_client = Client( + project="test-project", + credentials=_make_credentials(), + _http=object(), + enable_metrics=True, + ) + assert ro_client._http is ro_client._http_internal + assert ro_client.metrics_enabled is True + assert ro_client.debug_metrics_enabled is False + ro_client._metrics_recorder.shutdown() + ro_client._metrics_recorder = None + assert ro_client.metrics_enabled is False + assert ro_client.debug_metrics_enabled is False + assert ro_client._http is ro_client._http_internal + ro_client.close() + + # Lazy _http session creation attaches _gcs_metrics_recorder on first access + lazy_client = Client( + project="test-project", + credentials=_make_credentials(), + _http=None, + enable_metrics=True, + meter_provider=provider, + ) + lazy_client._bucket_metadata_cache = None + lazy_session = mock.MagicMock() + with mock.patch( + "google.cloud.client.ClientWithProject._http", + new_callable=mock.PropertyMock, + return_value=lazy_session, + ): + assert lazy_client._http is lazy_session + assert lazy_session._gcs_metrics_recorder is lazy_client._metrics_recorder + lazy_client.close() + + +def test_list_resource_metrics_context_and_unannotated_passthrough(metrics_env): + client, session, reader = metrics_env + + # Unannotated _list_resource leaves raw functools.partial untouched + raw_iter = client._list_resource("/b", lambda it, item: item) + assert isinstance(raw_iter.api_request, functools.partial) + + # Annotated _list_resource wraps api_request and emits one operation per page + with _helpers._list_metrics_context("storage.Client.list_buckets", "ListBuckets"): + paged_iter = client._list_resource("/b", lambda it, item: item["name"]) + assert not isinstance(paged_iter.api_request, functools.partial) + assert _collect_metrics(reader) == {} + + session.request.side_effect = [ + _json_response({"items": [{"name": "b1"}], "nextPageToken": "p2"}), + _json_response({"items": [{"name": "b2"}]}), + ] + assert list(paged_iter) == ["b1", "b2"] + pts = _collect_metrics(reader) + ops = pts["gcp.storage.client.operations"] + assert sum(p.value for p in ops) == 2 + assert ops[0].attributes["gcp.client.method"] == "storage.Client.list_buckets" + assert ops[0].attributes["gcp.storage.operation"] == "ListBuckets" + + +def test_helpers_context_managers_batch_deferral_and_property_mixin(metrics_env): + client, session, reader = metrics_env + + class _BrokenClientObj: + _client = "fallback-client" + + @property + def client(self): + raise RuntimeError("unbound") + + broken = _BrokenClientObj() + assert _helpers._resolve_operation_client(broken, client="explicit") == "explicit" + assert _helpers._resolve_operation_client(broken) == "fallback-client" + + with _helpers._suppress_body_size(): + assert _helpers._SUPPRESS_BODY_SIZE.get() is True + assert _helpers._SUPPRESS_BODY_SIZE.get() is False + + with _helpers._list_metrics_context("outer.method", "ListObjects"): + with _helpers._list_metrics_context("inner.method", "ListBuckets"): + assert _helpers._LIST_METRICS_CONTEXT.get() == ( + "outer.method", + "ListObjects", + ) + assert _helpers._LIST_METRICS_CONTEXT.get() is None + + with _helpers._client_method_override("outer.override"): + with _helpers._client_method_override("inner.override"): + assert _helpers._CLIENT_METHOD_OVERRIDE.get() == "outer.override" + assert _helpers._CLIENT_METHOD_OVERRIDE.get() is None + + # Disabled client yields None in _start_storage_operation + disabled_client = Client( + project="test-project", + credentials=_make_credentials(), + _http=mock.MagicMock(), + enable_metrics=False, + ) + disabled_client._bucket_metadata_cache = None + with _helpers._start_storage_operation( + disabled_client, "storage.Client.get_bucket", "GetBucket" + ) as st_disabled: + assert st_disabled is None + disabled_client.close() + + # Batch deferral in Connection.api_request and _start_storage_operation skips + # queued/batchable ops while keeping non-batchable ops + batch = client.batch() + batch.__enter__() + try: + bkt = client.bucket("b1") + bkt.storage_class = "NEARLINE" + bkt.patch() + assert _collect_metrics(reader) == {} + + with _helpers._start_storage_operation( + client, "storage.Bucket.patch", "UpdateBucket" + ) as st_deferred: + assert st_deferred is None + with _helpers._client_method_override("storage.Blob.download_as_bytes"): + with _helpers._start_storage_operation( + client, "storage.Blob.download_to_file", "ReadObject" + ) as st_non_batch: + assert st_non_batch is not None + finally: + client._pop_batch() + + # Custom _PropertyMixin without _METRICS_GET_OP / _METRICS_UPDATE_OP executes uninstrumented branch + class _CustomResource(_helpers._PropertyMixin): + path = "/custom/1" + user_project = None + + def __init__(self, cl): + super().__init__(name="c1") + self._client = cl + + @property + def client(self): + return self._client + + mock_client = mock.MagicMock() + mock_client._get_resource.return_value = {"k": "v"} + mock_client._patch_resource.return_value = {"k": "v2"} + mock_client._put_resource.return_value = {"k": "v3"} + res = _CustomResource(mock_client) + res.reload() + res.patch() + res.update() + assert res._properties == {"k": "v3"} + + # Instrumented _PropertyMixin records reload, patch, and update with canonical class name + class _InstrumentedResource(_helpers._PropertyMixin): + _METRICS_CLASS_NAME = "Bucket" + _METRICS_GET_OP = "GetBucket" + _METRICS_UPDATE_OP = "UpdateBucket" + path = "/b/my-bucket" + user_project = None + + def __init__(self, cl): + super().__init__(name="my-bucket") + self._client = cl + + @property + def client(self): + return self._client + + session.request.side_effect = [ + _json_response({"name": "my-bucket"}), + _json_response({"name": "my-bucket"}), + _json_response({"name": "my-bucket"}), + ] + inst_res = _InstrumentedResource(client) + inst_res.reload() + inst_res.patch() + inst_res.update() + + pts = _collect_metrics(reader) + methods_seen = { + p.attributes["gcp.client.method"] for p in pts["gcp.storage.client.operations"] + } + assert methods_seen == { + "storage.Blob.download_as_bytes", + "storage.Bucket.reload", + "storage.Bucket.patch", + "storage.Bucket.update", + } + + +def test_http_connection_direct_make_request_and_edge_branches(metrics_env): + client, session, reader = metrics_env + conn = client._connection + + # 1. Suppressed internal metrics bypasses recording + session.request.return_value = _json_response({"ok": True}) + with _helpers._suppress_internal_metrics(): + with _helpers._start_storage_operation( + client, "storage.Client.get_bucket", "GetBucket" + ) as st: + assert st is None + conn._make_request("GET", "https://storage.googleapis.com/storage/v1/b/b1") + conn.api_request(method="GET", path="/b/b1") + assert _collect_metrics(reader) == {} + + # 2. Direct _make_request network exception -> CONNECTION_ERROR, no TTFB, no status_code + session.request.side_effect = requests.ConnectionError("reset by peer") + with _helpers._start_storage_operation(client, "storage.Batch.finish", "Batch"): + with pytest.raises(requests.ConnectionError): + conn._make_request( + "POST", "https://storage.googleapis.com/batch/storage/v1" + ) + pts = _collect_metrics(reader) + assert "gcp.storage.client.ttfb" not in pts + att = pts["gcp.storage.client.attempts"][-1] + assert att.attributes["error.type"] == "CONNECTION_ERROR" + assert "http.response.status_code" not in att.attributes + + # 3. Direct _make_request non-2xx with normal response AND broken response object + session.request.side_effect = None + session.request.return_value = _json_response( + {"error": {"message": "unavailable"}}, status_code=503 + ) + resp_503 = conn._make_request( + "POST", "https://storage.googleapis.com/batch/storage/v1" + ) + assert resp_503.status_code == 503 + + broken_resp = mock.Mock() + broken_resp.status_code = 500 + session.request.return_value = broken_resp + conn._make_request("POST", "https://storage.googleapis.com/batch/storage/v1") + + bool_sc_resp = mock.Mock() + bool_sc_resp.status_code = True + session.request.return_value = bool_sc_resp + conn._make_request("POST", "https://storage.googleapis.com/batch/storage/v1") + + pts2 = _collect_metrics(reader) + statuses = [ + p.attributes.get("http.response.status_code") + for p in pts2["http.client.request.duration"] + ] + assert 503 in statuses + assert 500 in statuses + assert statuses[-1] is None + assert sum(p.count for p in pts2["gcp.storage.client.ttfb"]) == 2 + + # 4. api_request fallback branches when base_call bypasses _make_request or url is empty + with mock.patch("google.cloud._http.JSONConnection.api_request", return_value={}): + conn.api_request("GET", "/b/my-bucket") + weird_resp = _json_response({"ok": True}) + weird_resp.status_code = True # bool is excluded from int status_code + with mock.patch.object( + _helpers._opentelemetry_metrics, + "normalize_url_template", + wraps=_opentelemetry_metrics.normalize_url_template, + ): + session.request.return_value = _json_response({"ok": True}) + with mock.patch( + "google.cloud._http.JSONConnection._make_request", + return_value=weird_resp, + ): + with mock.patch( + "google.cloud._http.JSONConnection.api_request", + side_effect=lambda *a, **kw: (conn._make_request("GET", "") and {}), + ): + conn.api_request(method="GET", path="/b/my-bucket") + + with mock.patch( + "google.cloud._http.JSONConnection._make_request", + return_value=weird_resp, + ): + with mock.patch( + "google.cloud._http.JSONConnection.api_request", + side_effect=lambda *a, **kw: ( + conn._make_request("GET", "") + and (_ for _ in ()).throw(RuntimeError("boom")) + ), + ): + with pytest.raises(RuntimeError): + conn.api_request(method="GET", path="/b/my-bucket") + + +def test_retries_errors_and_ttfb_on_api_request(metrics_env): + client, session, reader = metrics_env + session.request.side_effect = [ + _json_response({"error": {"message": "503-1"}}, status_code=503), + requests.ConnectionError("transient reset"), + _json_response({"id": "my-bucket", "name": "my-bucket"}, status_code=200), + ] + fast_retry = retry.Retry(initial=0.001, maximum=0.002, deadline=5.0) + with _helpers._start_storage_operation( + client, "storage.Client.get_bucket", "GetBucket" + ): + resp = client._connection.api_request( + method="GET", path="/b/my-bucket", retry=fast_retry + ) + assert resp["name"] == "my-bucket" + + pts = _collect_metrics(reader) + assert len(pts["gcp.storage.client.operations"]) == 1 + op = pts["gcp.storage.client.operations"][0] + dur = pts["gcp.client.request.duration"][0] + assert op.attributes["gcp.storage.operation"] == "GetBucket" + assert op.attributes["gcp.client.method"] == "storage.Client.get_bucket" + assert op.attributes["error.type"] == "OK" + assert dur.attributes["rpc.method"] == "google.storage.v2.Storage/GetBucket" + assert dur.attributes["error.type"] == "OK" + + attempts = pts["gcp.storage.client.attempts"] + assert sum(p.value for p in attempts) == 3 + err_types = {p.attributes.get("error.type", "OK") for p in attempts} + assert err_types == {"UNAVAILABLE", "CONNECTION_ERROR", "OK"} + + retries_pts = pts["gcp.storage.client.retries"] + assert sum(p.value for p in retries_pts) == 2 + retry_errs = {p.attributes["error.type"] for p in retries_pts} + assert retry_errs == {"UNAVAILABLE", "CONNECTION_ERROR"} + + errors_pts = pts["gcp.storage.client.errors"] + assert sum(p.value for p in errors_pts) == 2 + + # TTFB recorded only for the 503 response and the 200 response (NOT ConnectionError) + ttfb_pts = pts["gcp.storage.client.ttfb"] + assert sum(p.count for p in ttfb_pts) == 2 + + # Terminal HTTP 400 BadRequest maps to INVALID_ARGUMENT + session.request.side_effect = [ + _json_response( + {"error": {"message": "Invalid bucket configuration"}}, + status_code=400, + ) + ] + with pytest.raises(Exception): + with _helpers._start_storage_operation( + client, "storage.Bucket.patch", "UpdateBucket" + ): + client._connection.api_request( + method="PATCH", path="/b/my-bucket", retry=None + ) + pts_after = _collect_metrics(reader) + patch_ops = [ + p + for p in pts_after["gcp.storage.client.operations"] + if p.attributes["gcp.client.method"] == "storage.Bucket.patch" + ] + assert len(patch_ops) == 1 + assert patch_ops[0].attributes["error.type"] == "INVALID_ARGUMENT" + + +def test_bucket_metadata_cache_background_suppression(metrics_env): + client, session, reader = metrics_env + cache = BucketMetadataCache(client) + + session.request.side_effect = [ + _json_response({"id": "b1", "name": "b1", "location": "US"}), + _json_response({"error": {"message": "404"}}, status_code=404), + ] + cache._fetch_background("b1") + cache._verify_existence_background("b1") + + assert _collect_metrics(reader) == {} + + +def test_trace_exemplar_correlation_on_http_json_metrics(metrics_env): + client, session, reader = metrics_env + session.request.return_value = _json_response({"name": "my-bucket"}) + + tracer_provider = TracerProvider() + tracer = tracer_provider.get_tracer("test-tracer") + with tracer.start_as_current_span("parent-span") as span: + expected_trace_id = span.get_span_context().trace_id + expected_span_id = span.get_span_context().span_id + with _helpers._start_storage_operation( + client, "storage.Client.get_bucket", "GetBucket" + ): + client._connection.api_request(method="GET", path="/b/my-bucket") + + pts = _collect_metrics(reader) + op_exemplars = pts["gcp.client.request.duration"][0].exemplars + http_exemplars = pts["http.client.request.duration"][0].exemplars + assert len(op_exemplars) >= 1 + assert len(http_exemplars) >= 1 + assert op_exemplars[0].trace_id == expected_trace_id + assert op_exemplars[0].span_id != 0 + assert http_exemplars[0].trace_id == expected_trace_id + assert http_exemplars[0].span_id != 0 + assert expected_span_id != 0 From 47160ae48f63a6d5188a2b52d4439f850c2a1600 Mon Sep 17 00:00:00 2001 From: Shradha Katyal Date: Sun, 1 Nov 2026 18:31:07 +0000 Subject: [PATCH 7/7] feat(storage): add OpenTelemetry standard metrics for Client, Bucket, and resource APIs - Instrument logical operations on Client and Bucket metadata, IAM, lifecycle, and list APIs - Record Batch.finish() logical operation metrics while deferring queued sub-requests - Instrument BucketACL, DefaultObjectACL, ObjectACL, HMACKeyMetadata, and BucketNotification --- .../google/cloud/storage/acl.py | 228 ++-- .../google/cloud/storage/batch.py | 32 +- .../google/cloud/storage/bucket.py | 1133 +++++++++-------- .../google/cloud/storage/client.py | 456 ++++--- .../google/cloud/storage/hmac_key.py | 107 +- .../google/cloud/storage/notification.py | 188 +-- .../unit/test_otel_veneer_bucket_metrics.py | 576 +++++++++ 7 files changed, 1765 insertions(+), 955 deletions(-) create mode 100644 packages/google-cloud-storage/tests/unit/test_otel_veneer_bucket_metrics.py diff --git a/packages/google-cloud-storage/google/cloud/storage/acl.py b/packages/google-cloud-storage/google/cloud/storage/acl.py index 038294b8f6fc..b5cab6ad9151 100644 --- a/packages/google-cloud-storage/google/cloud/storage/acl.py +++ b/packages/google-cloud-storage/google/cloud/storage/acl.py @@ -14,7 +14,14 @@ """Manage access to objects and buckets.""" -from google.cloud.storage._helpers import _add_generation_match_parameters +from contextlib import nullcontext + +from google.cloud.storage._helpers import ( + _add_generation_match_parameters, + _client_method_override, + _resolve_operation_client, + _start_storage_operation, +) from google.cloud.storage._opentelemetry_tracing import create_trace_span from google.cloud.storage.constants import _DEFAULT_TIMEOUT from google.cloud.storage.retry import ( @@ -143,6 +150,9 @@ class ACL(object): reload_path = None save_path = None user_project = None + _METRICS_CLASS_NAME = "ACL" + _METRICS_RELOAD_OP = "ListBucketACLs" + _METRICS_SAVE_OP = "UpdateBucketACL" def __init__(self): self.entities = {} @@ -180,8 +190,16 @@ def reset(self): self.entities.clear() self.loaded = False + def _lazy_method_context(self, method_name): + if self.loaded: + return nullcontext() + return _client_method_override( + f"storage.{self._METRICS_CLASS_NAME}.{method_name}" + ) + def __iter__(self): - self._ensure_loaded() + with self._lazy_method_context("__iter__"): + self._ensure_loaded() for entity in self.entities.values(): for role in entity.get_roles(): @@ -201,24 +219,25 @@ def entity_from_dict(self, entity_dict): :rtype: :class:`_ACLEntity` :returns: An Entity constructed from the dictionary. """ - entity = entity_dict["entity"] - role = entity_dict["role"] + with self._lazy_method_context("entity_from_dict"): + entity = entity_dict["entity"] + role = entity_dict["role"] - if entity == "allUsers": - entity = self.all() + if entity == "allUsers": + entity = self.all() - elif entity == "allAuthenticatedUsers": - entity = self.all_authenticated() + elif entity == "allAuthenticatedUsers": + entity = self.all_authenticated() - elif "-" in entity: - entity_type, identifier = entity.split("-", 1) - entity = self.entity(entity_type=entity_type, identifier=identifier) + elif "-" in entity: + entity_type, identifier = entity.split("-", 1) + entity = self.entity(entity_type=entity_type, identifier=identifier) - if not isinstance(entity, _ACLEntity): - raise ValueError(f"Invalid dictionary: {entity_dict}") + if not isinstance(entity, _ACLEntity): + raise ValueError(f"Invalid dictionary: {entity_dict}") - entity.grant(role) - return entity + entity.grant(role) + return entity def has_entity(self, entity): """Returns whether or not this ACL has any entries for an entity. @@ -229,8 +248,9 @@ def has_entity(self, entity): :rtype: bool :returns: True of the entity exists in the ACL. """ - self._ensure_loaded() - return str(entity) in self.entities + with self._lazy_method_context("has_entity"): + self._ensure_loaded() + return str(entity) in self.entities def get_entity(self, entity, default=None): """Gets an entity object from the ACL. @@ -246,8 +266,9 @@ def get_entity(self, entity, default=None): :returns: The corresponding entity or the value provided to ``default``. """ - self._ensure_loaded() - return self.entities.get(str(entity), default) + with self._lazy_method_context("get_entity"): + self._ensure_loaded() + return self.entities.get(str(entity), default) def add_entity(self, entity): """Add an entity to the ACL. @@ -255,8 +276,9 @@ def add_entity(self, entity): :type entity: :class:`_ACLEntity` :param entity: The entity to add to this ACL. """ - self._ensure_loaded() - self.entities[str(entity)] = entity + with self._lazy_method_context("add_entity"): + self._ensure_loaded() + self.entities[str(entity)] = entity def entity(self, entity_type, identifier=None): """Factory method for creating an Entity. @@ -277,12 +299,13 @@ def entity(self, entity_type, identifier=None): :rtype: :class:`_ACLEntity` :returns: A new Entity or a reference to an existing identical entity. """ - entity = _ACLEntity(entity_type=entity_type, identifier=identifier) - if self.has_entity(entity): - entity = self.get_entity(entity) - else: - self.add_entity(entity) - return entity + with self._lazy_method_context("entity"): + entity = _ACLEntity(entity_type=entity_type, identifier=identifier) + if self.has_entity(entity): + entity = self.get_entity(entity) + else: + self.add_entity(entity) + return entity def user(self, identifier): """Factory method for a user Entity. @@ -293,7 +316,8 @@ def user(self, identifier): :rtype: :class:`_ACLEntity` :returns: An Entity corresponding to this user. """ - return self.entity("user", identifier=identifier) + with self._lazy_method_context("user"): + return self.entity("user", identifier=identifier) def group(self, identifier): """Factory method for a group Entity. @@ -304,7 +328,8 @@ def group(self, identifier): :rtype: :class:`_ACLEntity` :returns: An Entity corresponding to this group. """ - return self.entity("group", identifier=identifier) + with self._lazy_method_context("group"): + return self.entity("group", identifier=identifier) def domain(self, domain): """Factory method for a domain Entity. @@ -315,7 +340,8 @@ def domain(self, domain): :rtype: :class:`_ACLEntity` :returns: An entity corresponding to this domain. """ - return self.entity("domain", identifier=domain) + with self._lazy_method_context("domain"): + return self.entity("domain", identifier=domain) def all(self): """Factory method for an Entity representing all users. @@ -323,7 +349,8 @@ def all(self): :rtype: :class:`_ACLEntity` :returns: An entity representing all users. """ - return self.entity("allUsers") + with self._lazy_method_context("all"): + return self.entity("allUsers") def all_authenticated(self): """Factory method for an Entity representing all authenticated users. @@ -331,7 +358,8 @@ def all_authenticated(self): :rtype: :class:`_ACLEntity` :returns: An entity representing all authenticated users. """ - return self.entity("allAuthenticatedUsers") + with self._lazy_method_context("all_authenticated"): + return self.entity("allAuthenticatedUsers") def get_entities(self): """Get a list of all Entity objects. @@ -339,8 +367,9 @@ def get_entities(self): :rtype: list of :class:`_ACLEntity` objects :returns: A list of all Entity objects. """ - self._ensure_loaded() - return list(self.entities.values()) + with self._lazy_method_context("get_entities"): + self._ensure_loaded() + return list(self.entities.values()) @property def client(self): @@ -381,25 +410,30 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): (Optional) How to retry the RPC. See: :ref:`configuring_retries` """ with create_trace_span(name="Storage.ACL.reload"): - path = self.reload_path - client = self._require_client(client) - query_params = {} - - if self.user_project is not None: - query_params["userProject"] = self.user_project - - self.entities.clear() - - found = client._get_resource( - path, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - self.loaded = True + with _start_storage_operation( + _resolve_operation_client(self, client), + f"storage.{self._METRICS_CLASS_NAME}.reload", + self._METRICS_RELOAD_OP, + ): + path = self.reload_path + client = self._require_client(client) + query_params = {} + + if self.user_project is not None: + query_params["userProject"] = self.user_project + + self.entities.clear() + + found = client._get_resource( + path, + query_params=query_params, + timeout=timeout, + retry=retry, + ) + self.loaded = True - for entry in found.get("items", ()): - self.add_entity(self.entity_from_dict(entry)) + for entry in found.get("items", ()): + self.add_entity(self.entity_from_dict(entry)) def _save( self, @@ -545,17 +579,22 @@ def save( save_to_backend = True if save_to_backend: - self._save( - acl, - None, - client, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + _resolve_operation_client(self, client), + f"storage.{self._METRICS_CLASS_NAME}.save", + self._METRICS_SAVE_OP, + ): + self._save( + acl, + None, + client, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + timeout=timeout, + retry=retry, + ) def save_predefined( self, @@ -610,18 +649,23 @@ def save_predefined( (Optional) How to retry the RPC. See: :ref:`configuring_retries` """ with create_trace_span(name="Storage.ACL.savePredefined"): - predefined = self.validate_predefined(predefined) - self._save( - None, - predefined, - client, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + _resolve_operation_client(self, client), + f"storage.{self._METRICS_CLASS_NAME}.save_predefined", + self._METRICS_SAVE_OP, + ): + predefined = self.validate_predefined(predefined) + self._save( + None, + predefined, + client, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + timeout=timeout, + retry=retry, + ) def clear( self, @@ -673,16 +717,21 @@ def clear( (Optional) How to retry the RPC. See: :ref:`configuring_retries` """ with create_trace_span(name="Storage.ACL.clear"): - self.save( - [], - client=client, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + _resolve_operation_client(self, client), + f"storage.{self._METRICS_CLASS_NAME}.clear", + self._METRICS_SAVE_OP, + ): + self.save( + [], + client=client, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + timeout=timeout, + retry=retry, + ) class BucketACL(ACL): @@ -692,6 +741,8 @@ class BucketACL(ACL): :param bucket: The bucket to which this ACL relates. """ + _METRICS_CLASS_NAME = "BucketACL" + def __init__(self, bucket): super(BucketACL, self).__init__() self.bucket = bucket @@ -722,6 +773,9 @@ class DefaultObjectACL(BucketACL): _URL_PATH_ELEM = "defaultObjectAcl" _PREDEFINED_QUERY_PARAM = "predefinedDefaultObjectAcl" + _METRICS_CLASS_NAME = "DefaultObjectACL" + _METRICS_RELOAD_OP = "ListDefaultObjectACLs" + _METRICS_SAVE_OP = "UpdateDefaultObjectACL" class ObjectACL(ACL): @@ -731,6 +785,10 @@ class ObjectACL(ACL): :param blob: The blob that this ACL corresponds to. """ + _METRICS_CLASS_NAME = "ObjectACL" + _METRICS_RELOAD_OP = "ListObjectACLs" + _METRICS_SAVE_OP = "UpdateObjectACL" + def __init__(self, blob): super(ObjectACL, self).__init__() self.blob = blob diff --git a/packages/google-cloud-storage/google/cloud/storage/batch.py b/packages/google-cloud-storage/google/cloud/storage/batch.py index d680a103bb91..304aa0cc6e88 100644 --- a/packages/google-cloud-storage/google/cloud/storage/batch.py +++ b/packages/google-cloud-storage/google/cloud/storage/batch.py @@ -41,6 +41,7 @@ import requests from google.cloud import _helpers, exceptions +from google.cloud.storage._helpers import _start_storage_operation from google.cloud.storage._http import Connection from google.cloud.storage.constants import _DEFAULT_TIMEOUT @@ -300,25 +301,26 @@ def finish(self, raise_exception=True): :rtype: list of tuples :returns: one ``(headers, payload)`` tuple per deferred request. """ - headers, body, timeout = self._prepare_batch_request() + with _start_storage_operation(self._client, "storage.Batch.finish", "Batch"): + headers, body, timeout = self._prepare_batch_request() - url = f"{self.API_BASE_URL}/batch/storage/v1" + url = f"{self.API_BASE_URL}/batch/storage/v1" - # Use the private ``_base_connection`` rather than the property - # ``_connection``, since the property may be this - # current batch. - response = self._client._base_connection._make_request( - "POST", url, data=body, headers=headers, timeout=timeout - ) + # Use the private ``_base_connection`` rather than the property + # ``_connection``, since the property may be this + # current batch. + response = self._client._base_connection._make_request( + "POST", url, data=body, headers=headers, timeout=timeout + ) - # Raise exception if the top-level batch request fails - if not 200 <= response.status_code < 300: - raise exceptions.from_http_response(response) + # Raise exception if the top-level batch request fails + if not 200 <= response.status_code < 300: + raise exceptions.from_http_response(response) - responses = list(_unpack_batch_response(response)) - self._finish_futures(responses, raise_exception=raise_exception) - self._responses = responses - return responses + responses = list(_unpack_batch_response(response)) + self._finish_futures(responses, raise_exception=raise_exception) + self._responses = responses + return responses def current(self): """Return the topmost batch, or None.""" diff --git a/packages/google-cloud-storage/google/cloud/storage/bucket.py b/packages/google-cloud-storage/google/cloud/storage/bucket.py index 7954e70e08f9..c8d52f1aa2ae 100644 --- a/packages/google-cloud-storage/google/cloud/storage/bucket.py +++ b/packages/google-cloud-storage/google/cloud/storage/bucket.py @@ -34,8 +34,11 @@ _add_etag_match_headers, _add_generation_match_parameters, _bucket_bound_hostname_url, + _list_metrics_context, _PropertyMixin, + _resolve_operation_client, _scalar_property, + _start_storage_operation, _validate_name, _virtual_hosted_style_base_url, ) @@ -648,6 +651,10 @@ class Bucket(_PropertyMixin): This is used in Bucket.delete() and Bucket.make_public(). """ + _METRICS_CLASS_NAME = "Bucket" + _METRICS_GET_OP = "GetBucket" + _METRICS_UPDATE_OP = "UpdateBucket" + STORAGE_CLASSES = ( STANDARD_STORAGE_CLASS, NEARLINE_STORAGE_CLASS, @@ -974,43 +981,44 @@ def exists( """ with self._create_trace_span(name="Storage.Bucket.exists"): client = self._require_client(client) - # We only need the status code (200 or not) so we seek to - # minimize the returned payload. - query_params = {"fields": "name"} - - if self.user_project is not None: - query_params["userProject"] = self.user_project + with _start_storage_operation(client, "storage.Bucket.exists", "GetBucket"): + # We only need the status code (200 or not) so we seek to + # minimize the returned payload. + query_params = {"fields": "name"} - _add_generation_match_parameters( - query_params, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) + if self.user_project is not None: + query_params["userProject"] = self.user_project - headers = {} - _add_etag_match_headers( - headers, - if_etag_match=if_etag_match, - if_etag_not_match=if_etag_not_match, - ) + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) - try: - # We intentionally pass `_target_object=None` since fields=name - # would limit the local properties. - client._get_resource( - self.path, - query_params=query_params, - headers=headers, - timeout=timeout, - retry=retry, - _target_object=None, + headers = {} + _add_etag_match_headers( + headers, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, ) - except NotFound: - # NOTE: This will not fail immediately in a batch. However, when - # Batch.finish() is called, the resulting `NotFound` will be - # raised. - return False - return True + + try: + # We intentionally pass `_target_object=None` since fields=name + # would limit the local properties. + client._get_resource( + self.path, + query_params=query_params, + headers=headers, + timeout=timeout, + retry=retry, + _target_object=None, + ) + except NotFound: + # NOTE: This will not fail immediately in a batch. However, when + # Batch.finish() is called, the resulting `NotFound` will be + # raised. + return False + return True def create( self, @@ -1075,17 +1083,20 @@ def create( """ with self._create_trace_span(name="Storage.Bucket.create"): client = self._require_client(client) - client.create_bucket( - bucket_or_name=self, - project=project, - user_project=self.user_project, - location=location, - predefined_acl=predefined_acl, - predefined_default_object_acl=predefined_default_object_acl, - enable_object_retention=enable_object_retention, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + client, "storage.Bucket.create", "CreateBucket" + ): + client.create_bucket( + bucket_or_name=self, + project=project, + user_project=self.user_project, + location=location, + predefined_acl=predefined_acl, + predefined_default_object_acl=predefined_default_object_acl, + enable_object_retention=enable_object_retention, + timeout=timeout, + retry=retry, + ) def update( self, @@ -1124,13 +1135,18 @@ def update( (Optional) How to retry the RPC. See: :ref:`configuring_retries` """ with self._create_trace_span(name="Storage.Bucket.update"): - super(Bucket, self).update( - client=client, - timeout=timeout, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - ) + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.update", + "UpdateBucket", + ): + super(Bucket, self).update( + client=client, + timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, + ) def reload( self, @@ -1191,39 +1207,44 @@ def reload( See: https://cloud.google.com/storage/docs/soft-delete """ with self._create_trace_span(name="Storage.Bucket.reload"): - try: - super(Bucket, self).reload( - client=client, - projection=projection, - timeout=timeout, - if_etag_match=if_etag_match, - if_etag_not_match=if_etag_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - soft_deleted=soft_deleted, - ) - except api_exceptions.Forbidden: + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.reload", + "GetBucket", + ): + try: + super(Bucket, self).reload( + client=client, + projection=projection, + timeout=timeout, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, + soft_deleted=soft_deleted, + ) + except api_exceptions.Forbidden: + active_client = client or self.client + cache = getattr(active_client, "_bucket_metadata_cache", None) + if cache: + try: + cache.update_cache( + self.name, + f"//storage.googleapis.com/projects/_/buckets/{self.name}", + "global", + ) + except Exception: + pass + raise + active_client = client or self.client cache = getattr(active_client, "_bucket_metadata_cache", None) if cache: try: - cache.update_cache( - self.name, - f"//storage.googleapis.com/projects/_/buckets/{self.name}", - "global", - ) + cache.update_from_bucket(self) except Exception: pass - raise - - active_client = client or self.client - cache = getattr(active_client, "_bucket_metadata_cache", None) - if cache: - try: - cache.update_from_bucket(self) - except Exception: - pass def patch( self, @@ -1262,22 +1283,27 @@ def patch( (Optional) How to retry the RPC. See: :ref:`configuring_retries` """ with self._create_trace_span(name="Storage.Bucket.patch"): - # Special case: For buckets, it is possible that labels are being - # removed; this requires special handling. - if self._label_removals: - self._changes.add("labels") - self._properties.setdefault("labels", {}) - for removed_label in self._label_removals: - self._properties["labels"][removed_label] = None - - # Call the superclass method. - super(Bucket, self).patch( - client=client, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.patch", + "UpdateBucket", + ): + # Special case: For buckets, it is possible that labels are being + # removed; this requires special handling. + if self._label_removals: + self._changes.add("labels") + self._properties.setdefault("labels", {}) + for removed_label in self._label_removals: + self._properties["labels"][removed_label] = None + + # Call the superclass method. + super(Bucket, self).patch( + client=client, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + timeout=timeout, + retry=retry, + ) @property def acl(self): @@ -1398,33 +1424,38 @@ def get_blob( :returns: The blob object if it exists, otherwise None. """ with self._create_trace_span(name="Storage.Bucket.getBlob"): - blob = Blob( - bucket=self, - name=blob_name, - encryption_key=encryption_key, - generation=generation, - **kwargs, - ) - try: - # NOTE: This will not fail immediately in a batch. However, when - # Batch.finish() is called, the resulting `NotFound` will be - # raised. - blob.reload( - client=client, - timeout=timeout, - if_etag_match=if_etag_match, - if_etag_not_match=if_etag_not_match, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - soft_deleted=soft_deleted, + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.get_blob", + "GetObject", + ): + blob = Blob( + bucket=self, + name=blob_name, + encryption_key=encryption_key, + generation=generation, + **kwargs, ) - except NotFound: - return None - else: - return blob + try: + # NOTE: This will not fail immediately in a batch. However, when + # Batch.finish() is called, the resulting `NotFound` will be + # raised. + blob.reload( + client=client, + timeout=timeout, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, + soft_deleted=soft_deleted, + ) + except NotFound: + return None + else: + return blob def list_blobs( self, @@ -1555,26 +1586,27 @@ def list_blobs( """ with self._create_trace_span(name="Storage.Bucket.listBlobs"): client = self._require_client(client) - return client.list_blobs( - self, - max_results=max_results, - page_token=page_token, - prefix=prefix, - delimiter=delimiter, - start_offset=start_offset, - end_offset=end_offset, - include_trailing_delimiter=include_trailing_delimiter, - versions=versions, - projection=projection, - fields=fields, - page_size=page_size, - timeout=timeout, - retry=retry, - match_glob=match_glob, - include_folders_as_prefixes=include_folders_as_prefixes, - soft_deleted=soft_deleted, - filter_=filter_, - ) + with _list_metrics_context("storage.Bucket.list_blobs", "ListObjects"): + return client.list_blobs( + self, + max_results=max_results, + page_token=page_token, + prefix=prefix, + delimiter=delimiter, + start_offset=start_offset, + end_offset=end_offset, + include_trailing_delimiter=include_trailing_delimiter, + versions=versions, + projection=projection, + fields=fields, + page_size=page_size, + timeout=timeout, + retry=retry, + match_glob=match_glob, + include_folders_as_prefixes=include_folders_as_prefixes, + soft_deleted=soft_deleted, + filter_=filter_, + ) def list_notifications( self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY @@ -1605,12 +1637,15 @@ def list_notifications( with self._create_trace_span(name="Storage.Bucket.listNotifications"): client = self._require_client(client) path = self.path + "/notificationConfigs" - iterator = client._list_resource( - path, - _item_to_notification, - timeout=timeout, - retry=retry, - ) + with _list_metrics_context( + "storage.Bucket.list_notifications", "ListNotifications" + ): + iterator = client._list_resource( + path, + _item_to_notification, + timeout=timeout, + retry=retry, + ) iterator.bucket = self return iterator @@ -1648,9 +1683,14 @@ def get_notification( :returns: notification instance. """ with self._create_trace_span(name="Storage.Bucket.getNotification"): - notification = self.notification(notification_id=notification_id) - notification.reload(client=client, timeout=timeout, retry=retry) - return notification + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.get_notification", + "GetNotification", + ): + notification = self.notification(notification_id=notification_id) + notification.reload(client=client, timeout=timeout, retry=retry) + return notification def delete( self, @@ -1709,61 +1749,64 @@ def delete( """ with self._create_trace_span(name="Storage.Bucket.delete"): client = self._require_client(client) - query_params = {} + with _start_storage_operation( + client, "storage.Bucket.delete", "DeleteBucket" + ): + query_params = {} - if self.user_project is not None: - query_params["userProject"] = self.user_project + if self.user_project is not None: + query_params["userProject"] = self.user_project - _add_generation_match_parameters( - query_params, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - if force: - blobs = list( - self.list_blobs( - max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + if force: + blobs = list( + self.list_blobs( + max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, + client=client, + timeout=timeout, + retry=retry, + versions=True, + ) + ) + if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: + message = ( + "Refusing to delete bucket with more than " + "%d objects. If you actually want to delete " + "this bucket, please delete the objects " + "yourself before calling Bucket.delete()." + ) % (self._MAX_OBJECTS_FOR_ITERATION,) + raise ValueError(message) + + # Ignore 404 errors on delete. + self.delete_blobs( + blobs, + on_error=lambda blob: None, client=client, timeout=timeout, retry=retry, - versions=True, + preserve_generation=True, ) - ) - if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: - message = ( - "Refusing to delete bucket with more than " - "%d objects. If you actually want to delete " - "this bucket, please delete the objects " - "yourself before calling Bucket.delete()." - ) % (self._MAX_OBJECTS_FOR_ITERATION,) - raise ValueError(message) - - # Ignore 404 errors on delete. - self.delete_blobs( - blobs, - on_error=lambda blob: None, - client=client, + + # We intentionally pass `_target_object=None` since a DELETE + # request has no response value (whether in a standard request or + # in a batch request). + client._delete_resource( + self.path, + query_params=query_params, timeout=timeout, retry=retry, - preserve_generation=True, + _target_object=None, ) - - # We intentionally pass `_target_object=None` since a DELETE - # request has no response value (whether in a standard request or - # in a batch request). - client._delete_resource( - self.path, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=None, - ) - cache = getattr(client, "_bucket_metadata_cache", None) - if cache: - try: - cache.evict(self.name) - except Exception: - pass + cache = getattr(client, "_bucket_metadata_cache", None) + if cache: + try: + cache.evict(self.name) + except Exception: + pass def delete_blob( self, @@ -1838,26 +1881,29 @@ def delete_blob( """ with self._create_trace_span(name="Storage.Bucket.deleteBlob"): client = self._require_client(client) - blob = Blob(blob_name, bucket=self, generation=generation) - - query_params = copy.deepcopy(blob._query_params) - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - # We intentionally pass `_target_object=None` since a DELETE - # request has no response value (whether in a standard request or - # in a batch request). - client._delete_resource( - blob.path, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=None, - ) + with _start_storage_operation( + client, "storage.Bucket.delete_blob", "DeleteObject" + ): + blob = Blob(blob_name, bucket=self, generation=generation) + + query_params = copy.deepcopy(blob._query_params) + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + # We intentionally pass `_target_object=None` since a DELETE + # request has no response value (whether in a standard request or + # in a batch request). + client._delete_resource( + blob.path, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=None, + ) def delete_blobs( self, @@ -1950,44 +1996,51 @@ def delete_blobs( `on_error` is not passed). """ with self._create_trace_span(name="Storage.Bucket.deleteBlobs"): - _raise_if_len_differs( - len(blobs), - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) - if_generation_match = iter(if_generation_match or []) - if_generation_not_match = iter(if_generation_not_match or []) - if_metageneration_match = iter(if_metageneration_match or []) - if_metageneration_not_match = iter(if_metageneration_not_match or []) + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.delete_blobs", + "DeleteObject", + ): + _raise_if_len_differs( + len(blobs), + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + if_generation_match = iter(if_generation_match or []) + if_generation_not_match = iter(if_generation_not_match or []) + if_metageneration_match = iter(if_metageneration_match or []) + if_metageneration_not_match = iter(if_metageneration_not_match or []) - for blob in blobs: - try: - blob_name = blob - generation = None - if not isinstance(blob_name, str): - blob_name = blob.name - generation = blob.generation if preserve_generation else None - - self.delete_blob( - blob_name, - client=client, - generation=generation, - if_generation_match=next(if_generation_match, None), - if_generation_not_match=next(if_generation_not_match, None), - if_metageneration_match=next(if_metageneration_match, None), - if_metageneration_not_match=next( - if_metageneration_not_match, None - ), - timeout=timeout, - retry=retry, - ) - except NotFound: - if on_error is not None: - on_error(blob) - else: - raise + for blob in blobs: + try: + blob_name = blob + generation = None + if not isinstance(blob_name, str): + blob_name = blob.name + generation = ( + blob.generation if preserve_generation else None + ) + + self.delete_blob( + blob_name, + client=client, + generation=generation, + if_generation_match=next(if_generation_match, None), + if_generation_not_match=next(if_generation_not_match, None), + if_metageneration_match=next(if_metageneration_match, None), + if_metageneration_not_match=next( + if_metageneration_not_match, None + ), + timeout=timeout, + retry=retry, + ) + except NotFound: + if on_error is not None: + on_error(blob) + else: + raise def copy_blob( self, @@ -2110,57 +2163,60 @@ def copy_blob( """ with self._create_trace_span(name="Storage.Bucket.copyBlob"): client = self._require_client(client) - query_params = {} - - if self.user_project is not None: - query_params["userProject"] = self.user_project - - if source_generation is not None: - query_params["sourceGeneration"] = source_generation - - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - if_source_generation_match=if_source_generation_match, - if_source_generation_not_match=if_source_generation_not_match, - if_source_metageneration_match=if_source_metageneration_match, - if_source_metageneration_not_match=if_source_metageneration_not_match, - ) + with _start_storage_operation( + client, "storage.Bucket.copy_blob", "RewriteObject" + ): + query_params = {} - if new_name is None: - new_name = blob.name + if self.user_project is not None: + query_params["userProject"] = self.user_project - new_blob = Blob(bucket=destination_bucket, name=new_name) + if source_generation is not None: + query_params["sourceGeneration"] = source_generation - if destination_contexts is not None: - if isinstance(destination_contexts, ObjectContexts): - new_blob.contexts = destination_contexts + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) + + if new_name is None: + new_name = blob.name + + new_blob = Blob(bucket=destination_bucket, name=new_name) + + if destination_contexts is not None: + if isinstance(destination_contexts, ObjectContexts): + new_blob.contexts = destination_contexts + else: + raise ValueError( + "destination_contexts must be an ObjectContexts object" + ) + request_body = new_blob._properties.copy() else: - raise ValueError( - "destination_contexts must be an ObjectContexts object" - ) - request_body = new_blob._properties.copy() - else: - request_body = None - - api_path = blob.path + "/copyTo" + new_blob.path - copy_result = client._post_resource( - api_path, - request_body, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=new_blob, - ) + request_body = None - if not preserve_acl: - new_blob.acl.save(acl={}, client=client, timeout=timeout) + api_path = blob.path + "/copyTo" + new_blob.path + copy_result = client._post_resource( + api_path, + request_body, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=new_blob, + ) - new_blob._set_properties(copy_result) - return new_blob + if not preserve_acl: + new_blob.acl.save(acl={}, client=client, timeout=timeout) + + new_blob._set_properties(copy_result) + return new_blob def rename_blob( self, @@ -2275,36 +2331,41 @@ def rename_blob( :returns: The newly-renamed blob. """ with self._create_trace_span(name="Storage.Bucket.renameBlob"): - same_name = blob.name == new_name - - new_blob = self.copy_blob( - blob, - self, - new_name, - client=client, - timeout=timeout, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - if_source_generation_match=if_source_generation_match, - if_source_generation_not_match=if_source_generation_not_match, - if_source_metageneration_match=if_source_metageneration_match, - if_source_metageneration_not_match=if_source_metageneration_not_match, - retry=retry, - ) - - if not same_name: - blob.delete( + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.rename_blob", + "RewriteObject", + ): + same_name = blob.name == new_name + + new_blob = self.copy_blob( + blob, + self, + new_name, client=client, timeout=timeout, - if_generation_match=if_source_generation_match, - if_generation_not_match=if_source_generation_not_match, - if_metageneration_match=if_source_metageneration_match, - if_metageneration_not_match=if_source_metageneration_not_match, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, retry=retry, ) - return new_blob + + if not same_name: + blob.delete( + client=client, + timeout=timeout, + if_generation_match=if_source_generation_match, + if_generation_not_match=if_source_generation_not_match, + if_metageneration_match=if_source_metageneration_match, + if_metageneration_not_match=if_source_metageneration_not_match, + retry=retry, + ) + return new_blob def move_blob( self, @@ -2396,39 +2457,42 @@ def move_blob( """ with self._create_trace_span(name="Storage.Bucket.moveBlob"): client = self._require_client(client) - query_params = {} - - if self.user_project is not None: - query_params["userProject"] = self.user_project - - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - if_source_generation_match=if_source_generation_match, - if_source_generation_not_match=if_source_generation_not_match, - if_source_metageneration_match=if_source_metageneration_match, - if_source_metageneration_not_match=if_source_metageneration_not_match, - ) + with _start_storage_operation( + client, "storage.Bucket.move_blob", "MoveObject" + ): + query_params = {} - new_blob = Blob(bucket=self, name=new_name) - api_path = "{blob_path}/moveTo/o/{new_name}".format( - blob_path=blob.path, new_name=_quote(new_blob.name) - ) + if self.user_project is not None: + query_params["userProject"] = self.user_project - move_result = client._post_resource( - api_path, - None, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=new_blob, - ) + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + if_source_generation_match=if_source_generation_match, + if_source_generation_not_match=if_source_generation_not_match, + if_source_metageneration_match=if_source_metageneration_match, + if_source_metageneration_not_match=if_source_metageneration_not_match, + ) - new_blob._set_properties(move_result) - return new_blob + new_blob = Blob(bucket=self, name=new_name) + api_path = "{blob_path}/moveTo/o/{new_name}".format( + blob_path=blob.path, new_name=_quote(new_blob.name) + ) + + move_result = client._post_resource( + api_path, + None, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=new_blob, + ) + + new_blob._set_properties(move_result) + return new_blob def restore_blob( self, @@ -2505,35 +2569,38 @@ def restore_blob( """ with self._create_trace_span(name="Storage.Bucket.restore_blob"): client = self._require_client(client) - query_params = {} - - if self.user_project is not None: - query_params["userProject"] = self.user_project - if generation is not None: - query_params["generation"] = generation - if copy_source_acl is not None: - query_params["copySourceAcl"] = copy_source_acl - if projection is not None: - query_params["projection"] = projection - - _add_generation_match_parameters( - query_params, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) + with _start_storage_operation( + client, "storage.Bucket.restore_blob", "RestoreObject" + ): + query_params = {} + + if self.user_project is not None: + query_params["userProject"] = self.user_project + if generation is not None: + query_params["generation"] = generation + if copy_source_acl is not None: + query_params["copySourceAcl"] = copy_source_acl + if projection is not None: + query_params["projection"] = projection + + _add_generation_match_parameters( + query_params, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) - blob = Blob(bucket=self, name=blob_name) - api_response = client._post_resource( - f"{blob.path}/restore", - None, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - blob._set_properties(api_response) - return blob + blob = Blob(bucket=self, name=blob_name) + api_response = client._post_resource( + f"{blob.path}/restore", + None, + query_params=query_params, + timeout=timeout, + retry=retry, + ) + blob._set_properties(api_response) + return blob @property def cors(self): @@ -3400,22 +3467,27 @@ def get_iam_policy( """ with self._create_trace_span(name="Storage.Bucket.getIamPolicy"): client = self._require_client(client) - query_params = {} + with _start_storage_operation( + client, "storage.Bucket.get_iam_policy", "GetIamPolicy" + ): + query_params = {} - if self.user_project is not None: - query_params["userProject"] = self.user_project + if self.user_project is not None: + query_params["userProject"] = self.user_project - if requested_policy_version is not None: - query_params["optionsRequestedPolicyVersion"] = requested_policy_version + if requested_policy_version is not None: + query_params["optionsRequestedPolicyVersion"] = ( + requested_policy_version + ) - info = client._get_resource( - f"{self.path}/iam", - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=None, - ) - return Policy.from_api_repr(info) + info = client._get_resource( + f"{self.path}/iam", + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=None, + ) + return Policy.from_api_repr(info) def set_iam_policy( self, @@ -3454,25 +3526,28 @@ def set_iam_policy( """ with self._create_trace_span(name="Storage.Bucket.setIamPolicy"): client = self._require_client(client) - query_params = {} - - if self.user_project is not None: - query_params["userProject"] = self.user_project - - path = f"{self.path}/iam" - resource = policy.to_api_repr() - resource["resourceId"] = self.path - - info = client._put_resource( - path, - resource, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=None, - ) + with _start_storage_operation( + client, "storage.Bucket.set_iam_policy", "SetIamPolicy" + ): + query_params = {} + + if self.user_project is not None: + query_params["userProject"] = self.user_project - return Policy.from_api_repr(info) + path = f"{self.path}/iam" + resource = policy.to_api_repr() + resource["resourceId"] = self.path + + info = client._put_resource( + path, + resource, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=None, + ) + + return Policy.from_api_repr(info) def test_iam_permissions( self, @@ -3511,20 +3586,25 @@ def test_iam_permissions( """ with self._create_trace_span(name="Storage.Bucket.testIamPermissions"): client = self._require_client(client) - query_params = {"permissions": permissions} - - if self.user_project is not None: - query_params["userProject"] = self.user_project - - path = f"{self.path}/iam/testPermissions" - resp = client._get_resource( - path, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=None, - ) - return resp.get("permissions", []) + with _start_storage_operation( + client, + "storage.Bucket.test_iam_permissions", + "TestIamPermissions", + ): + query_params = {"permissions": permissions} + + if self.user_project is not None: + query_params["userProject"] = self.user_project + + path = f"{self.path}/iam/testPermissions" + resp = client._get_resource( + path, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=None, + ) + return resp.get("permissions", []) def make_public( self, @@ -3576,21 +3656,13 @@ def make_public( for each blob. """ with self._create_trace_span(name="Storage.Bucket.makePublic"): - self.acl.all().grant_read() - self.acl.save( - client=client, - timeout=timeout, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - ) - - if future: - doa = self.default_object_acl - if not doa.loaded: - doa.reload(client=client, timeout=timeout) - doa.all().grant_read() - doa.save( + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.make_public", + "UpdateBucketACL", + ): + self.acl.all().grant_read() + self.acl.save( client=client, timeout=timeout, if_metageneration_match=if_metageneration_match, @@ -3598,31 +3670,44 @@ def make_public( retry=retry, ) - if recursive: - blobs = list( - self.list_blobs( - projection="full", - max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, + if future: + doa = self.default_object_acl + if not doa.loaded: + doa.reload(client=client, timeout=timeout) + doa.all().grant_read() + doa.save( client=client, timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, ) - ) - if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: - message = ( - "Refusing to make public recursively with more than " - "%d objects. If you actually want to make every object " - "in this bucket public, iterate through the blobs " - "returned by 'Bucket.list_blobs()' and call " - "'make_public' on each one." - ) % (self._MAX_OBJECTS_FOR_ITERATION,) - raise ValueError(message) - for blob in blobs: - blob.acl.all().grant_read() - blob.acl.save( - client=client, - timeout=timeout, + if recursive: + blobs = list( + self.list_blobs( + projection="full", + max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, + client=client, + timeout=timeout, + ) ) + if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: + message = ( + "Refusing to make public recursively with more than " + "%d objects. If you actually want to make every object " + "in this bucket public, iterate through the blobs " + "returned by 'Bucket.list_blobs()' and call " + "'make_public' on each one." + ) % (self._MAX_OBJECTS_FOR_ITERATION,) + raise ValueError(message) + + for blob in blobs: + blob.acl.all().grant_read() + blob.acl.save( + client=client, + timeout=timeout, + ) def make_private( self, @@ -3673,21 +3758,13 @@ def make_private( for each blob. """ with self._create_trace_span(name="Storage.Bucket.makePrivate"): - self.acl.all().revoke_read() - self.acl.save( - client=client, - timeout=timeout, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - ) - - if future: - doa = self.default_object_acl - if not doa.loaded: - doa.reload(client=client, timeout=timeout) - doa.all().revoke_read() - doa.save( + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.make_private", + "UpdateBucketACL", + ): + self.acl.all().revoke_read() + self.acl.save( client=client, timeout=timeout, if_metageneration_match=if_metageneration_match, @@ -3695,28 +3772,41 @@ def make_private( retry=retry, ) - if recursive: - blobs = list( - self.list_blobs( - projection="full", - max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, + if future: + doa = self.default_object_acl + if not doa.loaded: + doa.reload(client=client, timeout=timeout) + doa.all().revoke_read() + doa.save( client=client, timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, ) - ) - if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: - message = ( - "Refusing to make private recursively with more than " - "%d objects. If you actually want to make every object " - "in this bucket private, iterate through the blobs " - "returned by 'Bucket.list_blobs()' and call " - "'make_private' on each one." - ) % (self._MAX_OBJECTS_FOR_ITERATION,) - raise ValueError(message) - for blob in blobs: - blob.acl.all().revoke_read() - blob.acl.save(client=client, timeout=timeout) + if recursive: + blobs = list( + self.list_blobs( + projection="full", + max_results=self._MAX_OBJECTS_FOR_ITERATION + 1, + client=client, + timeout=timeout, + ) + ) + if len(blobs) > self._MAX_OBJECTS_FOR_ITERATION: + message = ( + "Refusing to make private recursively with more than " + "%d objects. If you actually want to make every object " + "in this bucket private, iterate through the blobs " + "returned by 'Bucket.list_blobs()' and call " + "'make_private' on each one." + ) % (self._MAX_OBJECTS_FOR_ITERATION,) + raise ValueError(message) + + for blob in blobs: + blob.acl.all().revoke_read() + blob.acl.save(client=client, timeout=timeout) def generate_upload_policy(self, conditions, expiration=None, client=None): """Create a signed upload policy for uploading objects. @@ -3797,38 +3887,43 @@ def lock_retention_policy( if the bucket's retention policy is already locked. """ with self._create_trace_span(name="Storage.Bucket.lockRetentionPolicy"): - if "metageneration" not in self._properties: - raise ValueError( - "Bucket has no retention policy assigned: try 'reload'?" - ) + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.Bucket.lock_retention_policy", + "LockBucketRetentionPolicy", + ): + if "metageneration" not in self._properties: + raise ValueError( + "Bucket has no retention policy assigned: try 'reload'?" + ) - policy = self._properties.get("retentionPolicy") + policy = self._properties.get("retentionPolicy") - if policy is None: - raise ValueError( - "Bucket has no retention policy assigned: try 'reload'?" - ) + if policy is None: + raise ValueError( + "Bucket has no retention policy assigned: try 'reload'?" + ) - if policy.get("isLocked"): - raise ValueError("Bucket's retention policy is already locked.") + if policy.get("isLocked"): + raise ValueError("Bucket's retention policy is already locked.") - client = self._require_client(client) + client = self._require_client(client) - query_params = {"ifMetagenerationMatch": self.metageneration} + query_params = {"ifMetagenerationMatch": self.metageneration} - if self.user_project is not None: - query_params["userProject"] = self.user_project + if self.user_project is not None: + query_params["userProject"] = self.user_project - path = f"/b/{self.name}/lockRetentionPolicy" - api_response = client._post_resource( - path, - None, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=self, - ) - self._set_properties(api_response) + path = f"/b/{self.name}/lockRetentionPolicy" + api_response = client._post_resource( + path, + None, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=self, + ) + self._set_properties(api_response) def generate_signed_url( self, diff --git a/packages/google-cloud-storage/google/cloud/storage/client.py b/packages/google-cloud-storage/google/cloud/storage/client.py index dc4b393979a7..052122ee073a 100644 --- a/packages/google-cloud-storage/google/cloud/storage/client.py +++ b/packages/google-cloud-storage/google/cloud/storage/client.py @@ -46,6 +46,7 @@ _get_api_endpoint_override, _get_environ_project, _get_storage_emulator_override, + _list_metrics_context, _start_storage_operation, _virtual_hosted_style_base_url, create_trace_span_helper, @@ -497,12 +498,17 @@ def get_service_account_email( :returns: service account email address """ with create_trace_span(name="Storage.Client.getServiceAccountEmail"): - if project is None: - project = self.project + with _start_storage_operation( + self, + "storage.Client.get_service_account_email", + "GetServiceAccount", + ): + if project is None: + project = self.project - path = f"/projects/{project}/serviceAccount" - api_response = self._get_resource(path, timeout=timeout, retry=retry) - return api_response["email_address"] + path = f"/projects/{project}/serviceAccount" + api_response = self._get_resource(path, timeout=timeout, retry=retry) + return api_response["email_address"] def bucket(self, bucket_name, user_project=None, generation=None): """Factory constructor for bucket object. @@ -1056,37 +1062,45 @@ def get_bucket( with create_trace_span_helper( self, bucket_name, name="Storage.Client.getBucket" ): - bucket = self._bucket_arg_to_bucket(bucket_or_name, generation=generation) - try: - bucket.reload( - client=self, - timeout=timeout, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - soft_deleted=soft_deleted, + with _start_storage_operation( + self, "storage.Client.get_bucket", "GetBucket" + ): + bucket = self._bucket_arg_to_bucket( + bucket_or_name, generation=generation ) - except api_exceptions.Forbidden: + try: + bucket.reload( + client=self, + timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, + soft_deleted=soft_deleted, + ) + except api_exceptions.Forbidden: + if ( + hasattr(self, "_bucket_metadata_cache") + and self._bucket_metadata_cache + ): + try: + self._bucket_metadata_cache.update_cache( + bucket_name, + f"//storage.googleapis.com/projects/_/buckets/{bucket_name}", + "global", + ) + except Exception: + pass + raise + if ( hasattr(self, "_bucket_metadata_cache") and self._bucket_metadata_cache ): try: - self._bucket_metadata_cache.update_cache( - bucket_name, - f"//storage.googleapis.com/projects/_/buckets/{bucket_name}", - "global", - ) + self._bucket_metadata_cache.update_from_bucket(bucket) except Exception: pass - raise - - if hasattr(self, "_bucket_metadata_cache") and self._bucket_metadata_cache: - try: - self._bucket_metadata_cache.update_from_bucket(bucket) - except Exception: - pass - return bucket + return bucket def lookup_bucket( self, @@ -1127,39 +1141,42 @@ def lookup_bucket( with create_trace_span_helper( self, bucket_name, name="Storage.Client.lookupBucket" ): - try: - bucket = self.get_bucket( - bucket_name, - timeout=timeout, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - retry=retry, - ) - if ( - hasattr(self, "_bucket_metadata_cache") - and self._bucket_metadata_cache - ): - try: - self._bucket_metadata_cache.update_from_bucket(bucket) - except Exception: - pass - return bucket - except api_exceptions.Forbidden: - if ( - hasattr(self, "_bucket_metadata_cache") - and self._bucket_metadata_cache - ): - try: - self._bucket_metadata_cache.update_cache( - bucket_name, - f"//storage.googleapis.com/projects/_/buckets/{bucket_name}", - "global", - ) - except Exception: - pass - raise - except NotFound: - return None + with _start_storage_operation( + self, "storage.Client.lookup_bucket", "GetBucket" + ): + try: + bucket = self.get_bucket( + bucket_name, + timeout=timeout, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + retry=retry, + ) + if ( + hasattr(self, "_bucket_metadata_cache") + and self._bucket_metadata_cache + ): + try: + self._bucket_metadata_cache.update_from_bucket(bucket) + except Exception: + pass + return bucket + except api_exceptions.Forbidden: + if ( + hasattr(self, "_bucket_metadata_cache") + and self._bucket_metadata_cache + ): + try: + self._bucket_metadata_cache.update_cache( + bucket_name, + f"//storage.googleapis.com/projects/_/buckets/{bucket_name}", + "global", + ) + except Exception: + pass + raise + except NotFound: + return None def create_bucket( self, @@ -1242,75 +1259,85 @@ def create_bucket( If the bucket already exists. """ with create_trace_span(name="Storage.Client.createBucket"): - bucket = self._bucket_arg_to_bucket(bucket_or_name) - query_params = {} - - if project is None: - project = self.project + with _start_storage_operation( + self, "storage.Client.create_bucket", "CreateBucket" + ): + bucket = self._bucket_arg_to_bucket(bucket_or_name) + query_params = {} - # Use no project if STORAGE_EMULATOR_HOST is set - if self._is_emulator_set: - if project is None: - project = _get_environ_project() if project is None: - project = "" - - # Only include the project parameter if a project is set. - # If a project is not set, falls back to API validation (BadRequest). - if project is not None: - query_params = {"project": project} - - if requester_pays is not None: - warnings.warn( - "requester_pays arg is deprecated. Use Bucket().requester_pays instead.", - PendingDeprecationWarning, - stacklevel=1, - ) - bucket.requester_pays = requester_pays - - if predefined_acl is not None: - predefined_acl = BucketACL.validate_predefined(predefined_acl) - query_params["predefinedAcl"] = predefined_acl - - if predefined_default_object_acl is not None: - predefined_default_object_acl = DefaultObjectACL.validate_predefined( - predefined_default_object_acl - ) - query_params["predefinedDefaultObjectAcl"] = ( - predefined_default_object_acl - ) + project = self.project + + # Use no project if STORAGE_EMULATOR_HOST is set + if self._is_emulator_set: + if project is None: + project = _get_environ_project() + if project is None: + project = "" + + # Only include the project parameter if a project is set. + # If a project is not set, falls back to API validation (BadRequest). + if project is not None: + query_params = {"project": project} + + if requester_pays is not None: + warnings.warn( + "requester_pays arg is deprecated. Use Bucket().requester_pays instead.", + PendingDeprecationWarning, + stacklevel=1, + ) + bucket.requester_pays = requester_pays + + if predefined_acl is not None: + predefined_acl = BucketACL.validate_predefined(predefined_acl) + query_params["predefinedAcl"] = predefined_acl + + if predefined_default_object_acl is not None: + predefined_default_object_acl = ( + DefaultObjectACL.validate_predefined( + predefined_default_object_acl + ) + ) + query_params["predefinedDefaultObjectAcl"] = ( + predefined_default_object_acl + ) - if user_project is not None: - query_params["userProject"] = user_project + if user_project is not None: + query_params["userProject"] = user_project - if enable_object_retention: - query_params["enableObjectRetention"] = enable_object_retention + if enable_object_retention: + query_params["enableObjectRetention"] = enable_object_retention - properties = {key: bucket._properties[key] for key in bucket._changes} - properties["name"] = bucket.name + properties = {key: bucket._properties[key] for key in bucket._changes} + properties["name"] = bucket.name - if location is not None: - properties["location"] = location + if location is not None: + properties["location"] = location - if data_locations is not None: - properties["customPlacementConfig"] = {"dataLocations": data_locations} + if data_locations is not None: + properties["customPlacementConfig"] = { + "dataLocations": data_locations + } - api_response = self._post_resource( - "/b", - properties, - query_params=query_params, - timeout=timeout, - retry=retry, - _target_object=bucket, - ) + api_response = self._post_resource( + "/b", + properties, + query_params=query_params, + timeout=timeout, + retry=retry, + _target_object=bucket, + ) - bucket._set_properties(api_response) - if hasattr(self, "_bucket_metadata_cache") and self._bucket_metadata_cache: - try: - self._bucket_metadata_cache.update_from_bucket(bucket) - except Exception: - pass - return bucket + bucket._set_properties(api_response) + if ( + hasattr(self, "_bucket_metadata_cache") + and self._bucket_metadata_cache + ): + try: + self._bucket_metadata_cache.update_from_bucket(bucket) + except Exception: + pass + return bucket def download_blob_to_file( self, @@ -1421,26 +1448,29 @@ def download_blob_to_file( bucket_name, name="Storage.Client.downloadBlobToFile", ): - if not isinstance(blob_or_uri, Blob): - blob_or_uri = Blob.from_uri(blob_or_uri) - - blob_or_uri._prep_and_do_download( - file_obj, - client=self, - start=start, - end=end, - raw_download=raw_download, - if_etag_match=if_etag_match, - if_etag_not_match=if_etag_not_match, - if_generation_match=if_generation_match, - if_generation_not_match=if_generation_not_match, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - timeout=timeout, - checksum=checksum, - retry=retry, - single_shot_download=single_shot_download, - ) + with _start_storage_operation( + self, "storage.Client.download_blob_to_file", "ReadObject" + ): + if not isinstance(blob_or_uri, Blob): + blob_or_uri = Blob.from_uri(blob_or_uri) + + blob_or_uri._prep_and_do_download( + file_obj, + client=self, + start=start, + end=end, + raw_download=raw_download, + if_etag_match=if_etag_match, + if_etag_not_match=if_etag_not_match, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + timeout=timeout, + checksum=checksum, + retry=retry, + single_shot_download=single_shot_download, + ) def list_blobs( self, @@ -1630,17 +1660,18 @@ def list_blobs( extra_params["userProject"] = bucket.user_project path = bucket.path + "/o" - iterator = self._list_resource( - path, - _item_to_blob, - page_token=page_token, - max_results=max_results, - extra_params=extra_params, - page_start=_blobs_page_start, - page_size=page_size, - timeout=timeout, - retry=retry, - ) + with _list_metrics_context("storage.Client.list_blobs", "ListObjects"): + iterator = self._list_resource( + path, + _item_to_blob, + page_token=page_token, + max_results=max_results, + extra_params=extra_params, + page_start=_blobs_page_start, + page_size=page_size, + timeout=timeout, + retry=retry, + ) iterator.bucket = bucket iterator.prefixes = set() return iterator @@ -1762,17 +1793,18 @@ def list_buckets( if return_partial_success is not None: extra_params["returnPartialSuccess"] = return_partial_success - iterator = self._list_resource( - "/b", - _item_to_bucket, - page_token=page_token, - max_results=max_results, - extra_params=extra_params, - page_size=page_size, - timeout=timeout, - retry=retry, - page_start=_buckets_page_start, - ) + with _list_metrics_context("storage.Client.list_buckets", "ListBuckets"): + iterator = self._list_resource( + "/b", + _item_to_bucket, + page_token=page_token, + max_results=max_results, + extra_params=extra_params, + page_size=page_size, + timeout=timeout, + retry=retry, + page_start=_buckets_page_start, + ) return iterator def restore_bucket( @@ -1821,24 +1853,27 @@ def restore_bucket( :rtype: :class:`google.cloud.storage.bucket.Bucket` :returns: The restored Bucket. """ - query_params = {"generation": generation, "projection": projection} + with _start_storage_operation( + self, "storage.Client.restore_bucket", "RestoreBucket" + ): + query_params = {"generation": generation, "projection": projection} - _add_generation_match_parameters( - query_params, - if_metageneration_match=if_metageneration_match, - if_metageneration_not_match=if_metageneration_not_match, - ) + _add_generation_match_parameters( + query_params, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) - bucket = self.bucket(bucket_name) - api_response = self._post_resource( - f"{bucket.path}/restore", - None, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - bucket._set_properties(api_response) - return bucket + bucket = self.bucket(bucket_name) + api_response = self._post_resource( + f"{bucket.path}/restore", + None, + query_params=query_params, + timeout=timeout, + retry=retry, + ) + bucket._set_properties(api_response) + return bucket def create_hmac_key( self, @@ -1884,26 +1919,29 @@ def create_hmac_key( :returns: metadata for the created key, plus the bytes of the key's secret, which is an 40-character base64-encoded string. """ with create_trace_span(name="Storage.Client.createHmacKey"): - if project_id is None: - project_id = self.project + with _start_storage_operation( + self, "storage.Client.create_hmac_key", "CreateHMACKey" + ): + if project_id is None: + project_id = self.project - path = f"/projects/{project_id}/hmacKeys" - qs_params = {"serviceAccountEmail": service_account_email} + path = f"/projects/{project_id}/hmacKeys" + qs_params = {"serviceAccountEmail": service_account_email} - if user_project is not None: - qs_params["userProject"] = user_project + if user_project is not None: + qs_params["userProject"] = user_project - api_response = self._post_resource( - path, - None, - query_params=qs_params, - timeout=timeout, - retry=retry, - ) - metadata = HMACKeyMetadata(self) - metadata._properties = api_response["metadata"] - secret = api_response["secret"] - return metadata, secret + api_response = self._post_resource( + path, + None, + query_params=qs_params, + timeout=timeout, + retry=retry, + ) + metadata = HMACKeyMetadata(self) + metadata._properties = api_response["metadata"] + secret = api_response["secret"] + return metadata, secret def list_hmac_keys( self, @@ -1966,14 +2004,15 @@ def list_hmac_keys( if user_project is not None: extra_params["userProject"] = user_project - return self._list_resource( - path, - _item_to_hmac_key_metadata, - max_results=max_results, - extra_params=extra_params, - timeout=timeout, - retry=retry, - ) + with _list_metrics_context("storage.Client.list_hmac_keys", "ListHMACKeys"): + return self._list_resource( + path, + _item_to_hmac_key_metadata, + max_results=max_results, + extra_params=extra_params, + timeout=timeout, + retry=retry, + ) def get_hmac_key_metadata( self, access_id, project_id=None, user_project=None, timeout=_DEFAULT_TIMEOUT @@ -1996,9 +2035,12 @@ def get_hmac_key_metadata( :param user_project: (Optional) This parameter is currently ignored. """ with create_trace_span(name="Storage.Client.getHmacKeyMetadata"): - metadata = HMACKeyMetadata(self, access_id, project_id, user_project) - metadata.reload(timeout=timeout) # raises NotFound for missing key - return metadata + with _start_storage_operation( + self, "storage.Client.get_hmac_key_metadata", "GetHMACKey" + ): + metadata = HMACKeyMetadata(self, access_id, project_id, user_project) + metadata.reload(timeout=timeout) # raises NotFound for missing key + return metadata def generate_signed_post_policy_v4( self, diff --git a/packages/google-cloud-storage/google/cloud/storage/hmac_key.py b/packages/google-cloud-storage/google/cloud/storage/hmac_key.py index 0976b5e5a8e6..bdabcc19118a 100644 --- a/packages/google-cloud-storage/google/cloud/storage/hmac_key.py +++ b/packages/google-cloud-storage/google/cloud/storage/hmac_key.py @@ -20,6 +20,7 @@ from google.cloud._helpers import _rfc3339_nanos_to_datetime from google.cloud.exceptions import NotFound +from google.cloud.storage._helpers import _start_storage_operation from google.cloud.storage._opentelemetry_tracing import create_trace_span from google.cloud.storage.constants import _DEFAULT_TIMEOUT from google.cloud.storage.retry import DEFAULT_RETRY, DEFAULT_RETRY_IF_ETAG_IN_JSON @@ -203,22 +204,25 @@ def exists(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): :returns: True if the key exists in Cloud Storage. """ with create_trace_span(name="Storage.HmacKey.exists"): - try: - qs_params = {} - - if self.user_project is not None: - qs_params["userProject"] = self.user_project - - self._client._get_resource( - self.path, - query_params=qs_params, - timeout=timeout, - retry=retry, - ) - except NotFound: - return False - else: - return True + with _start_storage_operation( + self._client, "storage.HMACKeyMetadata.exists", "GetHMACKey" + ): + try: + qs_params = {} + + if self.user_project is not None: + qs_params["userProject"] = self.user_project + + self._client._get_resource( + self.path, + query_params=qs_params, + timeout=timeout, + retry=retry, + ) + except NotFound: + return False + else: + return True def reload(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): """Reload properties from Cloud Storage. @@ -236,17 +240,20 @@ def reload(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): if the key does not exist on the back-end. """ with create_trace_span(name="Storage.HmacKey.reload"): - qs_params = {} + with _start_storage_operation( + self._client, "storage.HMACKeyMetadata.reload", "GetHMACKey" + ): + qs_params = {} - if self.user_project is not None: - qs_params["userProject"] = self.user_project + if self.user_project is not None: + qs_params["userProject"] = self.user_project - self._properties = self._client._get_resource( - self.path, - query_params=qs_params, - timeout=timeout, - retry=retry, - ) + self._properties = self._client._get_resource( + self.path, + query_params=qs_params, + timeout=timeout, + retry=retry, + ) def update(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY_IF_ETAG_IN_JSON): """Save writable properties to Cloud Storage. @@ -264,18 +271,21 @@ def update(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY_IF_ETAG_IN_JSON): if the key does not exist on the back-end. """ with create_trace_span(name="Storage.HmacKey.update"): - qs_params = {} - if self.user_project is not None: - qs_params["userProject"] = self.user_project - - payload = {"state": self.state} - self._properties = self._client._put_resource( - self.path, - payload, - query_params=qs_params, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + self._client, "storage.HMACKeyMetadata.update", "UpdateHMACKey" + ): + qs_params = {} + if self.user_project is not None: + qs_params["userProject"] = self.user_project + + payload = {"state": self.state} + self._properties = self._client._put_resource( + self.path, + payload, + query_params=qs_params, + timeout=timeout, + retry=retry, + ) def delete(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): """Delete the key from Cloud Storage. @@ -293,13 +303,16 @@ def delete(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): if the key does not exist on the back-end. """ with create_trace_span(name="Storage.HmacKey.delete"): - qs_params = {} - if self.user_project is not None: - qs_params["userProject"] = self.user_project - - self._client._delete_resource( - self.path, - query_params=qs_params, - timeout=timeout, - retry=retry, - ) + with _start_storage_operation( + self._client, "storage.HMACKeyMetadata.delete", "DeleteHMACKey" + ): + qs_params = {} + if self.user_project is not None: + qs_params["userProject"] = self.user_project + + self._client._delete_resource( + self.path, + query_params=qs_params, + timeout=timeout, + retry=retry, + ) diff --git a/packages/google-cloud-storage/google/cloud/storage/notification.py b/packages/google-cloud-storage/google/cloud/storage/notification.py index 21588f6eda4e..0d4319444a6f 100644 --- a/packages/google-cloud-storage/google/cloud/storage/notification.py +++ b/packages/google-cloud-storage/google/cloud/storage/notification.py @@ -21,6 +21,10 @@ from google.api_core.exceptions import NotFound +from google.cloud.storage._helpers import ( + _resolve_operation_client, + _start_storage_operation, +) from google.cloud.storage._opentelemetry_tracing import create_trace_span from google.cloud.storage.constants import _DEFAULT_TIMEOUT from google.cloud.storage.retry import DEFAULT_RETRY @@ -254,35 +258,40 @@ def create(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=None): :raises ValueError: if the notification already exists. """ with create_trace_span(name="Storage.BucketNotification.create"): - if self.notification_id is not None: - raise ValueError( - f"notification_id already set to {self.notification_id}; must be None to create a Notification." # noqa: E702 - ) - - client = self._require_client(client) - - query_params = {} - if self.bucket.user_project is not None: - query_params["userProject"] = self.bucket.user_project - - path = f"/b/{self.bucket.name}/notificationConfigs" - properties = self._properties.copy() - - if self.topic_name is None: - properties["topic"] = _TOPIC_REF_FMT.format(self.topic_project, "") - else: - properties["topic"] = _TOPIC_REF_FMT.format( - self.topic_project, self.topic_name + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.BucketNotification.create", + "CreateNotification", + ): + if self.notification_id is not None: + raise ValueError( + f"notification_id already set to {self.notification_id}; must be None to create a Notification." # noqa: E702 + ) + + client = self._require_client(client) + + query_params = {} + if self.bucket.user_project is not None: + query_params["userProject"] = self.bucket.user_project + + path = f"/b/{self.bucket.name}/notificationConfigs" + properties = self._properties.copy() + + if self.topic_name is None: + properties["topic"] = _TOPIC_REF_FMT.format(self.topic_project, "") + else: + properties["topic"] = _TOPIC_REF_FMT.format( + self.topic_project, self.topic_name + ) + + self._properties = client._post_resource( + path, + properties, + query_params=query_params, + timeout=timeout, + retry=retry, ) - self._properties = client._post_resource( - path, - properties, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - def exists(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): """Test whether this notification exists. @@ -310,28 +319,33 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): :raises ValueError: if the notification has no ID. """ with create_trace_span(name="Storage.BucketNotification.exists"): - if self.notification_id is None: - raise ValueError( - "Notification ID not set: set an explicit notification_id" - ) - - client = self._require_client(client) - - query_params = {} - if self.bucket.user_project is not None: - query_params["userProject"] = self.bucket.user_project - - try: - client._get_resource( - self.path, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - except NotFound: - return False - else: - return True + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.BucketNotification.exists", + "GetNotification", + ): + if self.notification_id is None: + raise ValueError( + "Notification ID not set: set an explicit notification_id" + ) + + client = self._require_client(client) + + query_params = {} + if self.bucket.user_project is not None: + query_params["userProject"] = self.bucket.user_project + + try: + client._get_resource( + self.path, + query_params=query_params, + timeout=timeout, + retry=retry, + ) + except NotFound: + return False + else: + return True def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): """Update this notification from the server configuration. @@ -359,24 +373,29 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): :raises ValueError: if the notification has no ID. """ with create_trace_span(name="Storage.BucketNotification.reload"): - if self.notification_id is None: - raise ValueError( - "Notification ID not set: set an explicit notification_id" + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.BucketNotification.reload", + "GetNotification", + ): + if self.notification_id is None: + raise ValueError( + "Notification ID not set: set an explicit notification_id" + ) + + client = self._require_client(client) + + query_params = {} + if self.bucket.user_project is not None: + query_params["userProject"] = self.bucket.user_project + + response = client._get_resource( + self.path, + query_params=query_params, + timeout=timeout, + retry=retry, ) - - client = self._require_client(client) - - query_params = {} - if self.bucket.user_project is not None: - query_params["userProject"] = self.bucket.user_project - - response = client._get_resource( - self.path, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - self._set_properties(response) + self._set_properties(response) def delete(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): """Delete this notification. @@ -405,24 +424,29 @@ def delete(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY): :raises ValueError: if the notification has no ID. """ with create_trace_span(name="Storage.BucketNotification.delete"): - if self.notification_id is None: - raise ValueError( - "Notification ID not set: set an explicit notification_id" + with _start_storage_operation( + _resolve_operation_client(self, client), + "storage.BucketNotification.delete", + "DeleteNotification", + ): + if self.notification_id is None: + raise ValueError( + "Notification ID not set: set an explicit notification_id" + ) + + client = self._require_client(client) + + query_params = {} + if self.bucket.user_project is not None: + query_params["userProject"] = self.bucket.user_project + + client._delete_resource( + self.path, + query_params=query_params, + timeout=timeout, + retry=retry, ) - client = self._require_client(client) - - query_params = {} - if self.bucket.user_project is not None: - query_params["userProject"] = self.bucket.user_project - - client._delete_resource( - self.path, - query_params=query_params, - timeout=timeout, - retry=retry, - ) - def _parse_topic_path(topic_path): """Verify that a topic path is in the correct format. diff --git a/packages/google-cloud-storage/tests/unit/test_otel_veneer_bucket_metrics.py b/packages/google-cloud-storage/tests/unit/test_otel_veneer_bucket_metrics.py new file mode 100644 index 000000000000..6de64f2502f9 --- /dev/null +++ b/packages/google-cloud-storage/tests/unit/test_otel_veneer_bucket_metrics.py @@ -0,0 +1,576 @@ +# Copyright 2026 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. + +"""Unit tests for OpenTelemetry Client, Bucket, Batch, ACL, HMAC & Notification Veneer metrics.""" + +import http.client +import io +import json +from unittest import mock + +import pytest +import requests +from google.api_core import retry +from google.auth import credentials as auth_credentials +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import InMemoryMetricReader + +from google.cloud.storage import _opentelemetry_metrics +from google.cloud.storage.acl import ( + ACL, + BucketACL, + DefaultObjectACL, + ObjectACL, + _ACLEntity, +) +from google.cloud.storage.blob import Blob +from google.cloud.storage.bucket import Bucket +from google.cloud.storage.client import Client +from google.cloud.storage.hmac_key import HMACKeyMetadata +from google.cloud.storage.notification import BucketNotification + + +def _make_credentials(): + return auth_credentials.AnonymousCredentials() + + +def _json_response(payload=None, status_code=http.client.OK, headers=None): + response = requests.Response() + response.status_code = status_code + response.headers.update({"Content-Type": "application/json"}) + if headers: + response.headers.update(headers) + if payload is not None: + response._content = ( + payload + if isinstance(payload, bytes) + else json.dumps(payload).encode("utf-8") + ) + else: + response._content = b"" + response.request = requests.Request( + "GET", "https://storage.googleapis.com/storage/v1/b" + ).prepare() + return response + + +def _collect_metrics(reader): + data = reader.get_metrics_data() + collected = {} + if data is None: # pragma: NO COVER + return collected + for resource_metric in data.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + collected.setdefault(metric.name, []).extend(metric.data.data_points) + return collected + + +@pytest.fixture(autouse=True) +def _enable_dev_gate(monkeypatch): + monkeypatch.setattr(_opentelemetry_metrics, "_ENABLE_METRICS_DEV_GATE", True) + monkeypatch.setattr(_opentelemetry_metrics, "HAS_OPENTELEMETRY_METRICS", True) + + +@pytest.fixture +def metrics_env(): + reader = InMemoryMetricReader() + provider = MeterProvider(metric_readers=[reader]) + session = mock.MagicMock() + client = Client( + project="test-project", + credentials=_make_credentials(), + _http=session, + enable_metrics=True, + meter_provider=provider, + ) + client._bucket_metadata_cache = None + yield client, session, reader + client.close() + + +def test_client_get_bucket_retries_and_bucket_patch_error(metrics_env): + client, session, reader = metrics_env + session.request.side_effect = [ + _json_response({"error": {"message": "503-1"}}, status_code=503), + requests.ConnectionError("transient reset"), + _json_response({"id": "my-bucket", "name": "my-bucket"}, status_code=200), + ] + fast_retry = retry.Retry(initial=0.001, maximum=0.002, deadline=5.0) + bucket = client.get_bucket("my-bucket", retry=fast_retry) + assert bucket.name == "my-bucket" + + pts = _collect_metrics(reader) + assert len(pts["gcp.storage.client.operations"]) == 1 + op = pts["gcp.storage.client.operations"][0] + dur = pts["gcp.client.request.duration"][0] + assert op.attributes["gcp.storage.operation"] == "GetBucket" + assert op.attributes["gcp.client.method"] == "storage.Client.get_bucket" + assert op.attributes["error.type"] == "OK" + assert dur.attributes["rpc.method"] == "google.storage.v2.Storage/GetBucket" + assert dur.attributes["error.type"] == "OK" + assert sum(p.value for p in pts["gcp.storage.client.attempts"]) == 3 + assert sum(p.value for p in pts["gcp.storage.client.retries"]) == 2 + assert sum(p.value for p in pts["gcp.storage.client.errors"]) == 2 + + # Terminal HTTP 400 BadRequest on Bucket.patch maps to INVALID_ARGUMENT + session.request.side_effect = [ + _json_response( + {"error": {"message": "Invalid bucket configuration"}}, + status_code=400, + ) + ] + b = client.bucket("my-bucket") + with pytest.raises(Exception): + b.patch(retry=None) + pts_after = _collect_metrics(reader) + patch_ops = [ + p + for p in pts_after["gcp.storage.client.operations"] + if p.attributes["gcp.client.method"] == "storage.Bucket.patch" + ] + assert len(patch_ops) == 1 + assert patch_ops[0].attributes["error.type"] == "INVALID_ARGUMENT" + + +def test_existence_and_lookup_404_semantics(metrics_env): + client, session, reader = metrics_env + session.request.side_effect = [ + _json_response({"error": {"message": "not found"}}, status_code=404), + _json_response({"error": {"message": "not found"}}, status_code=404), + _json_response({"error": {"message": "not found"}}, status_code=404), + _json_response({"error": {"message": "not found"}}, status_code=404), + _json_response({"error": {"message": "not found"}}, status_code=404), + ] + + assert client.lookup_bucket("missing-b") is None + bucket = client.bucket("missing-b") + assert bucket.exists() is False + assert bucket.get_blob("missing-obj") is None + hmac_meta = HMACKeyMetadata(client, access_id="GOOG123") + assert hmac_meta.exists() is False + notif = BucketNotification(bucket, notification_id="n1") + assert notif.exists() is False + + pts = _collect_metrics(reader) + ops = pts["gcp.storage.client.operations"] + assert sum(p.value for p in ops) == 5 + for op in ops: + assert op.attributes["error.type"] == "OK" + for dur in pts["gcp.client.request.duration"]: + assert dur.attributes["error.type"] == "OK" + + attempts = pts["gcp.storage.client.attempts"] + assert sum(p.value for p in attempts) == 5 + for att in attempts: + assert att.attributes["error.type"] == "NOT_FOUND" + for http_dur in pts["http.client.request.duration"]: + assert http_dur.attributes["http.response.status_code"] == 404 + + +def test_lazy_pagination_iterators_and_precedence(metrics_env): + client, session, reader = metrics_env + + # Constructing iterators emits 0 metrics + b_iter = client.bucket("my-bucket").list_blobs() + c_iter = client.list_blobs("my-bucket") + buckets_iter = client.list_buckets() + hmac_iter = client.list_hmac_keys() + notif_iter = client.bucket("my-bucket").list_notifications() + assert _collect_metrics(reader) == {} + + # Iterating 2 pages on Bucket.list_blobs emits 2 ListObjects operations + # attributed to storage.Bucket.list_blobs (outermost wins over Client.list_blobs) + session.request.side_effect = [ + _json_response({"items": [{"name": "o1"}], "nextPageToken": "tok2"}), + _json_response({"items": [{"name": "o2"}]}), + _json_response({"items": [{"name": "o3"}]}), + _json_response({"items": [{"name": "b1"}]}), + _json_response({"items": [{"accessId": "k1"}]}), + _json_response( + { + "items": [ + { + "id": "n1", + "topic": "//pubsub.googleapis.com/projects/my-proj/topics/my-topic", + } + ] + } + ), + ] + assert [b.name for b in b_iter] == ["o1", "o2"] + assert [b.name for b in c_iter] == ["o3"] + assert [b.name for b in buckets_iter] == ["b1"] + assert [k.access_id for k in hmac_iter] == ["k1"] + assert [n.notification_id for n in notif_iter] == ["n1"] + + pts = _collect_metrics(reader) + by_method = {} + for p in pts["gcp.storage.client.operations"]: + key = ( + p.attributes["gcp.client.method"], + p.attributes["gcp.storage.operation"], + ) + by_method[key] = by_method.get(key, 0) + p.value + + assert by_method == { + ("storage.Bucket.list_blobs", "ListObjects"): 2, + ("storage.Client.list_blobs", "ListObjects"): 1, + ("storage.Client.list_buckets", "ListBuckets"): 1, + ("storage.Client.list_hmac_keys", "ListHMACKeys"): 1, + ("storage.Bucket.list_notifications", "ListNotifications"): 1, + } + + +def test_all_client_bucket_acl_hmac_and_notification_methods(metrics_env): + client, session, reader = metrics_env + + # Client methods + session.request.side_effect = [ + _json_response({"email_address": "sa@example.com"}), # get_service_account + _json_response({"name": "created-b"}), # create_bucket + _json_response({"name": "restored-b"}), # restore_bucket + _json_response( + {"secret": "sec", "metadata": {"accessId": "GOOG1"}} + ), # create_hmac_key + _json_response({"accessId": "GOOG1", "state": "ACTIVE"}), # get_hmac_key_meta + ] + assert client.get_service_account_email() == "sa@example.com" + assert client.create_bucket("created-b").name == "created-b" + assert client.restore_bucket("restored-b", generation=123).name == "restored-b" + meta, secret = client.create_hmac_key("sa@test.iam.gserviceaccount.com") + assert secret == "sec" + assert meta.access_id == "GOOG1" + assert client.get_hmac_key_metadata("GOOG1").state == "ACTIVE" + + # Client.download_blob_to_file (mocking _prep_and_do_download to test Veneer wrapper) + bucket = client.bucket("my-bucket") + blob = bucket.blob("my-obj") + with mock.patch.object(Blob, "_prep_and_do_download"): + client.download_blob_to_file(blob, io.BytesIO()) + + # Bucket methods + iam_payload = { + "version": 1, + "etag": "CAE=", + "bindings": [{"role": "roles/storage.objectViewer", "members": ["allUsers"]}], + } + session.request.side_effect = [ + _json_response({"name": "my-bucket"}), # bucket.create + _json_response({"name": "my-bucket"}), # bucket.reload + _json_response({"name": "my-bucket"}), # bucket.update + _json_response({"name": "my-bucket"}), # bucket.patch + _json_response( + { + "id": "n1", + "topic": "//pubsub.googleapis.com/projects/my-proj/topics/my-topic", + } + ), # bucket.get_notification + _json_response(None, status_code=204), # bucket.delete_blob + _json_response({"resource": {"name": "copied"}}), # bucket.copy_blob + _json_response({"name": "moved"}), # bucket.move_blob + _json_response({"name": "restored"}), # bucket.restore_blob + _json_response(iam_payload), # bucket.get_iam_policy + _json_response(iam_payload), # bucket.set_iam_policy + _json_response({"permissions": ["storage.objects.get"]}), # test_iam + _json_response({"name": "my-bucket", "metageneration": "2"}), # lock_retention + _json_response(None, status_code=204), # bucket.delete + ] + bucket.create() + bucket.reload() + bucket.update() + bucket.patch() + assert bucket.get_notification("n1").notification_id == "n1" + bucket.delete_blob("old-obj") + bucket.copy_blob(blob, bucket, new_name="copied") + bucket.move_blob(blob, new_name="moved") + bucket.restore_blob("restored", generation=1) + pol = bucket.get_iam_policy() + bucket.set_iam_policy(pol) + assert bucket.test_iam_permissions(["storage.objects.get"]) == [ + "storage.objects.get" + ] + bucket._properties["metageneration"] = 1 + bucket._properties["retentionPolicy"] = {"retentionPeriod": 100} + bucket.lock_retention_policy() + bucket.delete() + + # Subclass of Bucket preserves canonical class name in gcp.client.method + class _UserBucket(Bucket): + pass + + session.request.side_effect = [_json_response({"name": "sub-b"})] + ub = _UserBucket(client, "sub-b") + ub.reload() + + # Multi-RPC Bucket methods: delete(force=True), delete_blobs(on_error=...), + # rename_blob, make_public(recursive=True, future=True), make_private(recursive=True, future=True) + session.request.side_effect = [ + _json_response({"items": [{"name": "o1"}]}), # delete(force=True) list_blobs + _json_response(None, status_code=204), # delete(force=True) delete o1 + _json_response(None, status_code=204), # delete(force=True) delete bucket + _json_response( + {"error": {"message": "404"}}, status_code=404 + ), # delete_blobs 1 + _json_response(None, status_code=204), # delete_blobs 2 + _json_response({"resource": {"name": "renamed"}}), # rename_blob copy + _json_response(None, status_code=204), # rename_blob delete + _json_response({"items": []}), # make_public reload bucket acl + _json_response({"acl": []}), # make_public patch bucket acl + _json_response({"items": []}), # make_public reload default acl + _json_response({"defaultObjectAcl": []}), # make_public patch default acl + _json_response({"items": [{"name": "o1", "acl": []}]}), # make_public list + _json_response({"acl": []}), # make_public blob o1 patch + _json_response({"acl": []}), # make_private patch bucket acl (already loaded) + _json_response({"defaultObjectAcl": []}), # make_private patch default acl + _json_response({"items": [{"name": "o1", "acl": []}]}), # make_private list + _json_response({"acl": []}), # make_private blob o1 patch + ] + bucket.delete(force=True) + errors = [] + bucket.delete_blobs(["missing", "present"], on_error=errors.append) + assert len(errors) == 1 + bucket.rename_blob(blob, new_name="renamed") + bucket.make_public(recursive=True, future=True) + bucket.make_private(recursive=True, future=True) + + # ACL, DefaultObjectACL, ObjectACL + for acl_obj in (BucketACL(bucket), DefaultObjectACL(bucket), ObjectACL(blob)): + # save() when loaded=False is a local no-op -> 0 metrics + acl_obj.loaded = False + acl_obj.save() + session.request.side_effect = [ + _json_response({"items": [{"entity": "allUsers", "role": "READER"}]}), + _json_response({}), + _json_response({}), + _json_response({}), + ] + acl_obj.reload() + acl_obj.save() + acl_obj.save_predefined("publicRead") + acl_obj.clear() + assert isinstance(acl_obj, ACL) + + # HMACKeyMetadata + hmac_meta = HMACKeyMetadata(client, access_id="GOOG999") + hmac_meta.state = "INACTIVE" + session.request.side_effect = [ + _json_response({"accessId": "GOOG999", "state": "ACTIVE"}), + _json_response({"accessId": "GOOG999", "state": "ACTIVE"}), + _json_response({"accessId": "GOOG999", "state": "INACTIVE"}), + _json_response(None, status_code=204), + ] + assert hmac_meta.exists() is True + hmac_meta.reload() + hmac_meta.state = "INACTIVE" + hmac_meta.update() + hmac_meta.delete() + + # BucketNotification (including client validation error when notification_id is None) + notif_missing_id = BucketNotification(bucket) + for fn in ( + notif_missing_id.exists, + notif_missing_id.reload, + notif_missing_id.delete, + ): + with pytest.raises(ValueError): + fn() + + notif = BucketNotification(bucket, topic_name="my-topic", topic_project="my-proj") + session.request.side_effect = [ + _json_response( + { + "id": "notif-1", + "topic": "//pubsub.googleapis.com/projects/my-proj/topics/my-topic", + } + ), + _json_response({"id": "notif-1"}), + _json_response( + { + "id": "notif-1", + "topic": "//pubsub.googleapis.com/projects/my-proj/topics/my-topic", + } + ), + _json_response(None, status_code=204), + ] + notif.create() + assert notif.exists() is True + notif.reload() + notif.delete() + + pts = _collect_metrics(reader) + methods_seen = { + p.attributes["gcp.client.method"] for p in pts["gcp.storage.client.operations"] + } + expected_subset = { + "storage.Client.get_service_account_email", + "storage.Client.create_bucket", + "storage.Client.restore_bucket", + "storage.Client.create_hmac_key", + "storage.Client.get_hmac_key_metadata", + "storage.Client.download_blob_to_file", + "storage.Bucket.create", + "storage.Bucket.reload", + "storage.Bucket.update", + "storage.Bucket.patch", + "storage.Bucket.get_notification", + "storage.Bucket.delete_blob", + "storage.Bucket.delete_blobs", + "storage.Bucket.copy_blob", + "storage.Bucket.rename_blob", + "storage.Bucket.move_blob", + "storage.Bucket.restore_blob", + "storage.Bucket.get_iam_policy", + "storage.Bucket.set_iam_policy", + "storage.Bucket.test_iam_permissions", + "storage.Bucket.make_public", + "storage.Bucket.make_private", + "storage.Bucket.lock_retention_policy", + "storage.Bucket.delete", + "storage.BucketACL.reload", + "storage.BucketACL.save", + "storage.BucketACL.save_predefined", + "storage.BucketACL.clear", + "storage.DefaultObjectACL.reload", + "storage.DefaultObjectACL.save", + "storage.DefaultObjectACL.save_predefined", + "storage.DefaultObjectACL.clear", + "storage.ObjectACL.reload", + "storage.ObjectACL.save", + "storage.ObjectACL.save_predefined", + "storage.ObjectACL.clear", + "storage.HMACKeyMetadata.exists", + "storage.HMACKeyMetadata.reload", + "storage.HMACKeyMetadata.update", + "storage.HMACKeyMetadata.delete", + "storage.BucketNotification.create", + "storage.BucketNotification.exists", + "storage.BucketNotification.reload", + "storage.BucketNotification.delete", + } + assert expected_subset.issubset(methods_seen) + assert not any("_UserBucket" in m for m in methods_seen) + assert "gcp.storage.client.retries" not in pts + + +def test_batch_deferral_and_subresponse_semantics_on_bucket(metrics_env): + client, session, reader = metrics_env + bucket = client.bucket("my-bucket") + + # Empty batch finish -> CLIENT_VALIDATION_ERROR + empty_batch = client.batch() + with pytest.raises(ValueError): + empty_batch.finish() + pts_empty = _collect_metrics(reader) + assert ( + pts_empty["gcp.storage.client.operations"][0].attributes["error.type"] + == "CLIENT_VALIDATION_ERROR" + ) + + multipart_ok_and_403 = ( + b"--batch_boundary\r\n" + b"Content-Type: application/http\r\n" + b"Content-ID: \r\n\r\n" + b"HTTP/1.1 200 OK\r\n" + b"Content-Type: application/json\r\n\r\n" + b'{"name": "my-bucket"}\r\n' + b"--batch_boundary\r\n" + b"Content-Type: application/http\r\n" + b"Content-ID: \r\n\r\n" + b"HTTP/1.1 403 Forbidden\r\n" + b"Content-Type: application/json\r\n\r\n" + b'{"error": {"message": "denied"}}\r\n' + b"--batch_boundary--\r\n" + ) + + # 1. raise_exception=False: queuing emits 0 metrics; finish emits Batch OK + session.request.return_value = _json_response( + multipart_ok_and_403, + status_code=200, + headers={"Content-Type": "multipart/mixed; boundary=batch_boundary"}, + ) + with client.batch(raise_exception=False): + bucket.patch() + bucket.delete() + pts_during = _collect_metrics(reader) + assert len(pts_during["gcp.storage.client.operations"]) == 1 + + pts_after_no_raise = _collect_metrics(reader) + assert len(pts_after_no_raise["gcp.storage.client.operations"]) == 2 + + # 2. raise_exception=True: subresponse 403 fails Batch with PERMISSION_DENIED + session.request.return_value = _json_response( + multipart_ok_and_403, + status_code=200, + headers={"Content-Type": "multipart/mixed; boundary=batch_boundary"}, + ) + with pytest.raises(Exception): + with client.batch(raise_exception=True): + bucket.patch() + bucket.delete() + + pts_final = _collect_metrics(reader) + batch_ops = pts_final["gcp.storage.client.operations"] + err_map = [p.attributes.get("error.type", "OK") for p in batch_ops] + assert "CLIENT_VALIDATION_ERROR" in err_map + assert "OK" in err_map + assert "PERMISSION_DENIED" in err_map + + http_att = pts_final["http.client.request.duration"][0] + assert http_att.attributes["url.template"] == "/batch/storage/v1" + assert http_att.attributes["http.request.method"] == "POST" + + +def test_lazy_acl_methods_attribution(metrics_env): + client, session, reader = metrics_env + bucket = client.bucket("my-bucket") + + acl_payload = {"items": [{"entity": "user-a@example.com", "role": "READER"}]} + lazy_cases = [ + ("__iter__", lambda acl: list(acl)), + ( + "entity_from_dict", + lambda acl: acl.entity_from_dict( + {"entity": "user-b@example.com", "role": "READER"} + ), + ), + ("has_entity", lambda acl: acl.has_entity("user-a@example.com")), + ("get_entity", lambda acl: acl.get_entity("user-a@example.com")), + ( + "add_entity", + lambda acl: acl.add_entity(_ACLEntity("user", "c@example.com")), + ), + ("entity", lambda acl: acl.entity("user", "a@example.com")), + ("user", lambda acl: acl.user("a@example.com")), + ("group", lambda acl: acl.group("g@example.com")), + ("domain", lambda acl: acl.domain("example.com")), + ("all", lambda acl: acl.all()), + ("all_authenticated", lambda acl: acl.all_authenticated()), + ("get_entities", lambda acl: acl.get_entities()), + ] + for _, fn in lazy_cases: + session.request.return_value = _json_response(acl_payload) + acl = BucketACL(bucket) + fn(acl) + fn(acl) # loaded=True fast path -> 0 additional metrics + + pts_acl = _collect_metrics(reader) + acl_ops = pts_acl["gcp.storage.client.operations"] + assert len(acl_ops) == len(lazy_cases) + assert {p.attributes["gcp.client.method"] for p in acl_ops} == { + f"storage.BucketACL.{name}" for name, _ in lazy_cases + } + assert all( + p.attributes["gcp.storage.operation"] == "ListBucketACLs" for p in acl_ops + )