Skip to content

feat(storage): add resource span attributes for ACO ( App Centric Observability ) for async client#16151

Open
bajajneha27 wants to merge 20 commits into
googleapis:mainfrom
bajajneha27:509338299-async
Open

feat(storage): add resource span attributes for ACO ( App Centric Observability ) for async client#16151
bajajneha27 wants to merge 20 commits into
googleapis:mainfrom
bajajneha27:509338299-async

Conversation

@bajajneha27

Copy link
Copy Markdown
Contributor

No description provided.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jun 10, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a BucketMetadataCache to cache bucket metadata (ID and location) and uses it to enrich OpenTelemetry spans with destination resource attributes in both TracingConnection and AsyncConnectionTracing. When metadata is not cached, a background fetch is triggered. The review feedback highlights several important issues: a critical data race on bg_tasks_ in TracingConnection::MaybeTriggerBackgroundFetch due to a lack of mutex protection during push_back, potential undefined behavior in BucketMetadataCache::Put if max_size is initialized to 0, unnormalized bucket names on permission denied errors in TracingConnection, and performance concerns regarding the expensive creation of a new StorageConnection on every background fetch in AsyncConnectionTracing.

Comment thread google/cloud/storage/internal/tracing_connection.cc Outdated
Comment thread google/cloud/storage/internal/tracing_connection.cc
Comment thread google/cloud/storage/internal/bucket_metadata_cache.h
Comment thread google/cloud/storage/internal/async/connection_tracing.cc Outdated
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.32653% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.29%. Comparing base (f5f12f3) to head (f18bad4).

Files with missing lines Patch % Lines
...cloud/storage/internal/async/connection_tracing.cc 93.85% 7 Missing ⚠️
...le/cloud/storage/internal/bucket_metadata_cache.cc 83.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #16151    +/-   ##
========================================
  Coverage   92.29%   92.29%            
========================================
  Files        2221     2221            
  Lines      207362   207583   +221     
========================================
+ Hits       191378   191591   +213     
- Misses      15984    15992     +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bajajneha27
bajajneha27 marked this pull request as ready for review June 11, 2026 05:04
@bajajneha27
bajajneha27 requested review from a team as code owners June 11, 2026 05:04
@v-pratap
v-pratap self-requested a review June 11, 2026 05:20
@bajajneha27
bajajneha27 marked this pull request as draft July 16, 2026 14:24
@bajajneha27
bajajneha27 force-pushed the 509338299-async branch 4 times, most recently from 4244a97 to 0f627af Compare July 21, 2026 12:52
@bajajneha27
bajajneha27 marked this pull request as ready for review July 22, 2026 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant