[Do not Merge] POC for ACO

[nidhiii-27]

No description provided.

[gemini-code-assist]

Code Review

This pull request implements a background metadata fetching mechanism to enrich OpenTelemetry spans with bucket-specific resource IDs and locations. It introduces a BucketMetadataCache and decorators for Span and SpanBuilder to handle attribute extraction and application across both gRPC and HTTP transports. Feedback highlights several critical areas for improvement: the use of fragile reflection to access internal gRPC channels, the risks associated with an unbounded thread pool for background fetches, and the omission of certain SpanBuilder method overrides which could bypass the metadata logic. Additionally, it is recommended to improve error handling for background tasks and replace brittle sleep statements in tests with robust polling.

[gemini-code-assist]

The use of reflection to access private fields (backgroundResources) within GrpcStorageStub and its internal aggregation classes is highly fragile. These internal implementation details are not part of the public API and are subject to change in any dependency update (e.g., gax or google-cloud-storage updates), which would lead to NoSuchFieldException or IllegalAccessException at runtime. Since this is a POC, it might be acceptable for now, but a production implementation should find a supported way to access the underlying channel or use the generated client's methods directly.

[gemini-code-assist]

Using Executors.newCachedThreadPool() creates an unbounded thread pool. This can lead to resource exhaustion (too many threads) if a large number of unique buckets are accessed simultaneously, especially since each fetch involves a blocking gRPC/HTTP call. Per the general rules, a bounded thread pool should be used instead.

  private final ExecutorService cacheExecutor =
      new ThreadPoolExecutor(
          0,
          20,
          60L,
          TimeUnit.SECONDS,
          new SynchronousQueue<>(),
          r -> {
            Thread t = new Thread(r);
            t.setDaemon(true);
            t.setName("gcs-aco-metadata-cache-pool");
            return t;
          });

References

1. For safety, use a bounded thread pool (e.g., ThreadPoolExecutor with a defined maximum size) instead of an unbounded one (e.g., Executors.newCachedThreadPool()), even if the current logic seems to limit concurrent tasks.

[gemini-code-assist]

Exceptions during the background metadata fetch are swallowed without logging. This makes it difficult to diagnose why bucket attributes might be missing or stuck on default values. At a minimum, these exceptions should be logged.

[gemini-code-assist]

The AcoSpanBuilder wrapper is missing an implementation for setAllAttributes(Attributes attributes). If a user calls setAllAttributes instead of setAttribute, the bucketName extraction logic will be bypassed, and the background metadata fetch will not be triggered. Ensure all relevant SpanBuilder methods that can set attributes are overridden to check for the bucket URI.

[gemini-code-assist]

Using Thread.sleep() in integration tests is brittle and can lead to flaky tests or unnecessary delays in the CI pipeline. It is better to use a polling mechanism like Awaitility to wait for the background metadata fetch to complete and the cache to be updated.