Skip to content

Commit a24d0b5

Browse files
feat(storage): add OpenTelemetry tracing support for gRPC bidi APIs (#18397)
This PR adds OpenTelemetry tracing support for gRPC bidi APIs (Rapid Storage and Standard buckets) across the asynchronous gRPC client and streaming classes (`AsyncGrpcClient`, `AsyncAppendableObjectWriter`, and `AsyncMultiRangeDownloader`). The implementation aligns with existing regional bucket tracing conventions: - Instruments **T2 (Client API)** spans using the naming format `Storage.<Class>.<methodName>`. - Reuses existing opt-in environment variables (`ENABLE_GCS_PYTHON_CLIENT_OTEL_TRACES` and `DISABLE_GCS_PYTHON_CLIENT_OTEL_BUCKET_METADATA`). - Sets `rpc.system="grpc"` and attaches App-Centric Observability (ACO) attributes (`gcp.resource.destination.id` and `gcp.resource.destination.location`). - Injects W3C `traceparent` headers directly into gRPC request metadata tuples. Fixes: b/489239033 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 931968b commit a24d0b5

12 files changed

Lines changed: 2116 additions & 596 deletions

‎packages/google-cloud-storage/google/cloud/storage/_bucket_metadata_cache.py‎

Lines changed: 123 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
"""In-memory LRU cache for bucket metadata supporting App-centric Observability (ACO)."""
1616

17+
import asyncio
18+
import contextvars
1719
import logging
1820
import threading
1921

@@ -37,6 +39,20 @@ def __init__(self, client, max_size=10000):
3739
self._lock = threading.Lock()
3840
self._inflight_fetches = set()
3941
self._inflight_checks = set()
42+
self._background_tasks = set()
43+
44+
def _spawn_async_task(self, coro, inflight_set, bucket_name):
45+
try:
46+
loop = asyncio.get_running_loop()
47+
except RuntimeError:
48+
coro.close()
49+
with self._lock:
50+
inflight_set.discard(bucket_name)
51+
return
52+
# Detach from the caller's tracing context.
53+
task = contextvars.Context().run(loop.create_task, coro)
54+
self._background_tasks.add(task)
55+
task.add_done_callback(self._background_tasks.discard)
4056

4157
def get(self, bucket_name):
4258
"""Thread-safely retrieve cached metadata without queueing fetch."""
@@ -49,21 +65,29 @@ def get_or_queue_fetch(self, bucket_name):
4965
Returns None immediately on cache miss so caller does not block.
5066
"""
5167
with self._lock:
52-
if bucket_name in self._cache:
53-
return self._cache.get(bucket_name)
54-
elif bucket_name in self._inflight_fetches:
68+
cached = self._cache.get(bucket_name)
69+
if cached is not None:
70+
return cached
71+
if bucket_name in self._inflight_fetches:
5572
# This handles a thundering herd where 'n' threads
5673
# simultaneously experience a cache miss while 1 is already
5774
# fetching metadata. The remaining n - 1 threads should
5875
# bypass starting duplicate fetches.
5976
return None
60-
else:
61-
# fire a background thread and get bucket metadata.
62-
self._inflight_fetches.add(bucket_name)
63-
threading.Thread(
64-
target=self._fetch_background, args=(bucket_name,), daemon=True
65-
).start()
66-
return None
77+
# fire a background fetch and get bucket metadata.
78+
self._inflight_fetches.add(bucket_name)
79+
80+
if getattr(self._client, "_is_async_grpc_client", False) is True:
81+
self._spawn_async_task(
82+
self._fetch_background_async(bucket_name),
83+
self._inflight_fetches,
84+
bucket_name,
85+
)
86+
else:
87+
threading.Thread(
88+
target=self._fetch_background, args=(bucket_name,), daemon=True
89+
).start()
90+
return None
6791

6892
def check_and_evict(self, bucket_name):
6993
"""Asynchronously verify if a bucket exists on 404 and evict if deleted."""
@@ -73,6 +97,14 @@ def check_and_evict(self, bucket_name):
7397
if bucket_name in self._inflight_checks:
7498
return
7599
self._inflight_checks.add(bucket_name)
100+
101+
if getattr(self._client, "_is_async_grpc_client", False) is True:
102+
self._spawn_async_task(
103+
self._verify_existence_background_async(bucket_name),
104+
self._inflight_checks,
105+
bucket_name,
106+
)
107+
else:
76108
threading.Thread(
77109
target=self._verify_existence_background,
78110
args=(bucket_name,),
@@ -92,6 +124,27 @@ def _verify_existence_background(self, bucket_name):
92124
with self._lock:
93125
self._inflight_checks.discard(bucket_name)
94126

127+
async def _verify_existence_background_async(self, bucket_name):
128+
try:
129+
from google.cloud import _storage_v2 as storage_v2
130+
131+
request = storage_v2.GetBucketRequest(
132+
name=f"projects/_/buckets/{bucket_name}"
133+
)
134+
bucket = await self._client.grpc_client.get_bucket(
135+
request=request, timeout=10.0
136+
)
137+
self.update_from_bucket(bucket, bucket_name=bucket_name)
138+
except (NotFound, api_exceptions.NotFound):
139+
self.evict(bucket_name)
140+
except Exception as e:
141+
logger.debug(
142+
f"Async background verification for bucket existence failed for {bucket_name}: {e}"
143+
)
144+
finally:
145+
with self._lock:
146+
self._inflight_checks.discard(bucket_name)
147+
95148
def _fetch_background(self, bucket_name):
96149
"""Asynchronously fetch bucket metadata and update the cache."""
97150
try:
@@ -114,28 +167,76 @@ def _fetch_background(self, bucket_name):
114167
with self._lock:
115168
self._inflight_fetches.discard(bucket_name)
116169

117-
def update_from_bucket(self, bucket):
118-
"""Update cache from a Bucket instance."""
119-
if not bucket or not bucket.name:
170+
async def _fetch_background_async(self, bucket_name):
171+
"""Asynchronously fetch bucket metadata via gRPC and update the cache."""
172+
try:
173+
from google.cloud import _storage_v2 as storage_v2
174+
175+
request = storage_v2.GetBucketRequest(
176+
name=f"projects/_/buckets/{bucket_name}"
177+
)
178+
bucket = await self._client.grpc_client.get_bucket(
179+
request=request, timeout=10.0
180+
)
181+
self.update_from_bucket(bucket, bucket_name=bucket_name)
182+
except (NotFound, api_exceptions.NotFound):
183+
self.evict(bucket_name)
184+
except api_exceptions.Forbidden:
185+
self.update_cache(
186+
bucket_name,
187+
f"//storage.googleapis.com/projects/_/buckets/{bucket_name}",
188+
"global",
189+
)
190+
except Exception as e:
191+
logger.debug(
192+
f"Async background fetch for bucket metadata failed for {bucket_name}: {e}"
193+
)
194+
finally:
195+
with self._lock:
196+
self._inflight_fetches.discard(bucket_name)
197+
198+
def update_from_bucket(self, bucket, bucket_name=None):
199+
"""Update cache from a Bucket instance or storage_v2.Bucket proto."""
200+
if not bucket:
201+
return
202+
name = bucket_name or getattr(bucket, "name", None)
203+
if not name or not isinstance(name, str):
120204
return
205+
if name.startswith("projects/") and "/buckets/" in name:
206+
name = name.split("/buckets/", 1)[1]
121207

122208
project_number = getattr(bucket, "project_number", None)
123-
location = getattr(bucket, "location", None) or "global"
124-
location = location.lower()
125-
location_type = getattr(bucket, "location_type", None) or "region"
126-
location_type = location_type.lower()
209+
if not project_number:
210+
proj_attr = getattr(bucket, "project", None)
211+
if isinstance(proj_attr, (str, int)):
212+
proj_str = str(proj_attr)
213+
if proj_str.startswith("projects/"):
214+
project_number = proj_str.split("projects/", 1)[1]
215+
elif proj_str:
216+
project_number = proj_str
217+
218+
loc_attr = getattr(bucket, "location", None)
219+
location = (
220+
loc_attr.lower() if isinstance(loc_attr, str) and loc_attr else "global"
221+
)
222+
loc_type_attr = getattr(bucket, "location_type", None)
223+
location_type = (
224+
loc_type_attr.lower()
225+
if isinstance(loc_type_attr, str) and loc_type_attr
226+
else "region"
227+
)
127228

128229
if location_type in ("multi-region", "dual-region"):
129230
location = "global"
130231

131-
if project_number:
132-
destination_id = f"//storage.googleapis.com/projects/{project_number}/buckets/{bucket.name}"
133-
else:
232+
if project_number and str(project_number) != "_":
134233
destination_id = (
135-
f"//storage.googleapis.com/projects/_/buckets/{bucket.name}"
234+
f"//storage.googleapis.com/projects/{project_number}/buckets/{name}"
136235
)
236+
else:
237+
destination_id = f"//storage.googleapis.com/projects/_/buckets/{name}"
137238

138-
self.update_cache(bucket.name, destination_id, location)
239+
self.update_cache(name, destination_id, location)
139240

140241
def update_cache(self, bucket_name, destination_id, location):
141242
"""Thread-safely update or insert a cache entry with bounded size."""

‎packages/google-cloud-storage/google/cloud/storage/_helpers.py‎

Lines changed: 117 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,16 @@
3232
from google.auth import environment_vars
3333
from google.cloud.exceptions import NotFound
3434

35-
from google.cloud.storage._opentelemetry_tracing import (
36-
_is_bucket_metadata_disabled,
37-
)
38-
from google.cloud.storage._opentelemetry_tracing import (
39-
create_trace_span as _base_create_trace_span,
40-
)
35+
from google.cloud.storage import _opentelemetry_tracing
4136
from google.cloud.storage.constants import _DEFAULT_TIMEOUT
4237
from google.cloud.storage.retry import (
4338
DEFAULT_RETRY,
4439
DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED,
4540
)
4641

42+
# Kept as a patch point for unit tests.
43+
_base_create_trace_span = None
44+
4745
_logger = logging.getLogger(__name__)
4846

4947
STORAGE_EMULATOR_ENV_VAR = "STORAGE_EMULATOR_HOST" # Despite name, includes scheme.
@@ -149,61 +147,127 @@ def _validate_name(name):
149147
return name
150148

151149

152-
@contextmanager
153-
def create_trace_span_helper(client, bucket_name, name, attributes=None, **kwargs):
154-
span_attrs = dict(attributes) if attributes else {}
150+
_BUCKET_METADATA_FETCH_SPAN_NAMES = frozenset(
151+
(
152+
"Storage.Client.getBucket",
153+
"Storage.Client.lookupBucket",
154+
"Storage.Bucket.reload",
155+
"Storage.Bucket.exists",
156+
)
157+
)
155158

156-
if (
157-
bucket_name
158-
and isinstance(bucket_name, str)
159-
and client
160-
and hasattr(client, "_bucket_metadata_cache")
161-
and client._bucket_metadata_cache
162-
and not _is_bucket_metadata_disabled()
163-
):
164-
try:
165-
if name in (
166-
"Storage.Client.getBucket",
167-
"Storage.Client.lookupBucket",
168-
"Storage.Bucket.reload",
169-
"Storage.Bucket.exists",
170-
):
171-
cached = client._bucket_metadata_cache.get(bucket_name)
172-
else:
173-
cached = client._bucket_metadata_cache.get_or_queue_fetch(bucket_name)
174-
175-
if cached and isinstance(cached, tuple) and len(cached) == 2:
176-
dest_id, loc = cached
177-
span_attrs.update(
178-
{
179-
"gcp.resource.destination.id": dest_id,
180-
"gcp.resource.destination.location": loc,
181-
}
182-
)
183-
except Exception as e:
184-
_logger.debug(f"Failed cache lookup in create_trace_span_helper: {e}")
185159

186-
if "client" not in kwargs and client:
187-
kwargs["client"] = client
160+
class _TraceSpanHelperContext:
161+
"""Context manager supporting both sync and async tracing span creation with bucket metadata."""
188162

189-
with _base_create_trace_span(name, attributes=span_attrs, **kwargs) as span:
190-
try:
191-
yield span
192-
except (NotFound, api_exceptions.NotFound):
193-
if (
194-
bucket_name
195-
and isinstance(bucket_name, str)
196-
and client
197-
and hasattr(client, "_bucket_metadata_cache")
198-
and client._bucket_metadata_cache
199-
):
163+
__slots__ = (
164+
"client",
165+
"bucket_name",
166+
"name",
167+
"attributes",
168+
"kwargs",
169+
"_base_cm",
170+
)
171+
172+
def __init__(self, client, bucket_name, name, attributes=None, **kwargs):
173+
self.client = client
174+
self.bucket_name = bucket_name
175+
self.name = name
176+
self.attributes = attributes
177+
self.kwargs = kwargs
178+
self._base_cm = None
179+
180+
def _prepare_base_cm(self):
181+
span_attrs = dict(self.attributes) if self.attributes else {}
182+
client = self.client
183+
bucket_name = self.bucket_name
184+
name = self.name
185+
186+
if (
187+
_opentelemetry_tracing._is_otel_traces_enabled()
188+
and not _opentelemetry_tracing._is_bucket_metadata_disabled()
189+
and bucket_name
190+
and isinstance(bucket_name, str)
191+
and client is not None
192+
):
193+
cache = getattr(client, "_bucket_metadata_cache", None)
194+
if cache is not None:
200195
try:
201-
client._bucket_metadata_cache.check_and_evict(bucket_name)
196+
if name in _BUCKET_METADATA_FETCH_SPAN_NAMES:
197+
cached = cache.get(bucket_name)
198+
else:
199+
cached = cache.get_or_queue_fetch(bucket_name)
200+
201+
if cached and isinstance(cached, tuple) and len(cached) == 2:
202+
dest_id, loc = cached
203+
span_attrs["gcp.resource.destination.id"] = dest_id
204+
span_attrs["gcp.resource.destination.location"] = loc
205+
except Exception as e:
206+
_logger.debug(
207+
f"Failed cache lookup in create_trace_span_helper: {e}"
208+
)
209+
210+
kwargs = self.kwargs
211+
if "client" not in kwargs and client is not None:
212+
kwargs = dict(kwargs)
213+
kwargs["client"] = client
214+
215+
create_span_fn = (
216+
_base_create_trace_span
217+
if _base_create_trace_span is not None
218+
else _opentelemetry_tracing.create_trace_span
219+
)
220+
self._base_cm = create_span_fn(name, attributes=span_attrs, **kwargs)
221+
return self._base_cm
222+
223+
def _handle_not_found(self):
224+
if (
225+
_opentelemetry_tracing._is_otel_traces_enabled()
226+
and self.bucket_name
227+
and isinstance(self.bucket_name, str)
228+
and self.client is not None
229+
):
230+
cache = getattr(self.client, "_bucket_metadata_cache", None)
231+
if cache is not None:
232+
try:
233+
cache.check_and_evict(self.bucket_name)
202234
except Exception as e:
203235
_logger.debug(
204236
f"Failed cache eviction on 404 in create_trace_span_helper: {e}"
205237
)
206-
raise
238+
239+
def __enter__(self):
240+
self._prepare_base_cm()
241+
return self._base_cm.__enter__()
242+
243+
def __exit__(self, exc_type, exc_val, exc_tb):
244+
suppressed = False
245+
if self._base_cm is not None:
246+
suppressed = bool(self._base_cm.__exit__(exc_type, exc_val, exc_tb))
247+
if (
248+
not suppressed
249+
and exc_val is not None
250+
and isinstance(exc_val, (NotFound, api_exceptions.NotFound))
251+
):
252+
self._handle_not_found()
253+
return suppressed
254+
255+
async def __aenter__(self):
256+
return self.__enter__()
257+
258+
async def __aexit__(self, exc_type, exc_val, exc_tb):
259+
return self.__exit__(exc_type, exc_val, exc_tb)
260+
261+
262+
def create_trace_span_helper(client, bucket_name, name, attributes=None, **kwargs):
263+
if (
264+
_base_create_trace_span is None
265+
and not _opentelemetry_tracing._is_otel_traces_enabled()
266+
):
267+
return _opentelemetry_tracing._NOOP_TRACE_SPAN_CONTEXT
268+
return _TraceSpanHelperContext(
269+
client, bucket_name, name, attributes=attributes, **kwargs
270+
)
207271

208272

209273
class _PropertyMixin(object):

0 commit comments

Comments
 (0)