Skip to content

feat(storage): add client feature tracking header - #14764

Open
cpriti-os wants to merge 4 commits into
googleapis:mainfrom
cpriti-os:feature-tracking-header
Open

cpriti-os wants to merge 4 commits into
googleapis:mainfrom
cpriti-os:feature-tracking-header

Conversation

@cpriti-os

@cpriti-os cpriti-os commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Why

The service cannot currently observe when a client is using certain SDK-side features. In particular, a BlobReadSession transparently opens additional BidiReadObject streams when a new read cannot share the existing stream (e.g. a streaming channel read concurrent with another read). There is no user-facing option for this, so the only place it can be reported is from the SDK itself.

This PR adds a lightweight, opt-less feature tracking header so the service can attribute traffic to SDK features.

What

  • New package-private TrackedFeature enum — single source of truth for feature → bit position:
    • MULTISTREAM_IN_MRD(0): a BlobReadSession is backed by more than one BidiReadObject stream.
  • New package-private FeatureTracking helper:
    • HEADER = "x-goog-storage-java-features"
    • encode(int mask): 4-byte big-endian uint32 → URL-safe base64 (RFC 4648 §5, with padding).
    • FeatureTracking.Mask: thread-safe monotonically growing bitmask.
  • ObjectReadSessionState:
    • A FeatureTracking.Mask is shared between a session's root state and every state produced by forkChild().
    • forkChild() sets MULTISTREAM_IN_MRD. forkChild() is only ever called from ObjectReadSessionImpl#registerReadInState when canHandleNewRead returns false — i.e. exactly when the session is about to open a second (or later) stream.
    • getOpenArguments() merges the header into the GrpcCallContext extra headers alongside x-goog-request-params when the mask is non-zero. The header is omitted when the mask is 0.
    • Consequence: the first stream of a session never carries the header; every additional stream does; and if the original stream is later re-opened (retry/redirect) after a fork it carries the header too, since the mask is shared.
  • UnifiedOpts.Headers.BLOCKLIST now includes x-goog-storage-java-features, so user-supplied extraHeaders cannot spoof it.

No public API surface was added or changed (everything is package-private).

Encoding contract

mask (uint32) -> 4 bytes big-endian -> base64url with padding
1    -> "AAAAAQ=="
4    -> "AAAABA=="
0xF8 -> "AAAA-A=="
0    -> header omitted

Parity with google-cloud-go's x-goog-storage-go-features (googleapis/google-cloud-go#20684); the Java header uses the same encoding with the SDK-specific header name and feature name MULTISTREAM_IN_MRD.

Tests

  • FeatureTrackingTest (11): encoding vectors 0, 1, 2, 3, 4, 0xF8 (-), 0xFC (_), 0x80000000, 0xFFFFFFFF; every single bit encodes URL-safe with padding; Mask yields no header when empty, uses the literal wire name/value, set is idempotent, and a multi-threaded test (8 threads x 1000 sets behind a latch) loses no bits; every bitMask() is a distinct single bit (Integer.bitCount == 1, no duplicates), MULTISTREAM_IN_MRD.bitMask() == 1; the header is rejected (exact and mixed case) by UnifiedOpts.extraHeaders and by the public BlobSourceOption.extraHeaders factory.
  • ObjectReadSessionStateTest (+5): header absent before fork; present on child and on parent OpenArguments after forkChild(); a second fork and a grandchild fork neither duplicate nor alter the value; x-goog-request-params (incl. routing token) and user-supplied extra headers on the base GrpcCallContext coexist with the feature header.
  • ITObjectReadSessionFakeTest (+3, in-process fake gRPC server, headers captured per stream with GrpcRequestAuditing):
    • singleStreamSessionNeverSendsFeatureTrackingHeader: two sequential reads share one stream; no request carries the header.
    • forkedStreamCarriesFeatureTrackingHeader: channel read + concurrent future read forces a 2nd stream; header AAAAAQ== sent on exactly one of two streams (the forked one), and not before the fork.
    • streamReopenedAfterForkCarriesFeatureTrackingHeader: after the fork the server fails the original stream with UNAVAILABLE; the retry re-open of that stream (resuming read id 1 at offset 20) also carries the header - 3 streams, header on exactly 2.
  • UnifiedOptsGrpcTest blocklist expectation updated.

Note

Semantics: bit 0 (MULTISTREAM_IN_MRD) is set only when a BlobReadSession forks a second (or later) BidiReadObject stream - the first stream of a session never carries the header.
MULTISTREAM_IN_MRD in Java means the session actually opened a 2nd stream; in Go it means the user configured >1 connection. HTTP/JSON transport is unaffected: the only producer of feature bits is ObjectReadSessionState (gRPC BidiReadObject only); StorageImpl (HTTP) never constructs it, so no header is ever added on that path. Blocklist enforcement happens in UnifiedOpts.extraHeaders at option construction, before either transport is involved.

Testing

Ran locally from java-storage/ (JDK 26 for build/tests, JDK 21 for fmt-maven-plugin which is incompatible with 26):

Step Result
mvn com.spotify.fmt:fmt-maven-plugin:format -pl google-cloud-storage applied, no diff after
mvn test-compile -pl google-cloud-storage -am -T 1C -P quick-build OK
mvn test -pl google-cloud-storage -Dtest='FeatureTrackingTest,ObjectReadSessionStateTest,ITObjectReadSessionFakeTest,UnifiedOptsGrpcTest*,UnifiedOptsTest*,GrpcStorageImplTest,*ReadSession*' -Dsurefire.failIfNoSpecifiedTests=false FeatureTrackingTest 11/11, ObjectReadSessionStateTest 10/10, ITObjectReadSessionFakeTest 23/23, ObjectReadSessionStreamTest 13/13, ObjectReadSessionStreamReadTest 17/17, ObjectReadSessionTest 3/3, ObjectReadSessionLifeCycleTest 1/1, UnifiedOptsTest 24/24, all UnifiedOptsGrpcTest$* suites green. GrpcStorageImplTest does not exist in this module.
ITObjectReadSessionTest (matched by *ReadSession*) SKIPPED (environmental): 8 errors, all Connection refused to the storage-testbench emulator at 127.0.0.1:9000, which is not running locally; unrelated to this change.
mvn clirr:check -pl google-cloud-storage -Dclirr.skip=false (clirr is skipped by default upstream) BUILD SUCCESS comparing to 2.75.0; javap confirms FeatureTracking, FeatureTracking$Mask, TrackedFeature are all non-public.
Full unit suite, real-GCS ITs not run locally (CI).

Note: to resolve dependencies locally the not-yet-released shared-deps / gax / api-common / iam / common-protos / kms versions were temporarily pinned to their latest Maven Central releases; those pom edits are not part of this PR (verified with git diff --stat before each commit).

Add a package-private feature tracking mechanism so the service can
observe SDK-side features it otherwise cannot see. The first tracked
feature is MULTISTREAM_IN_MRD: a BlobReadSession transparently opening
more than one BidiReadObject stream, which has no user-facing option.

* TrackedFeatures: single source of truth enum, feature -> bit position.
* FeatureTracking: header name `x-goog-storage-java-features`, encoding
  (4-byte big-endian uint32 -> URL-safe base64 with padding), and a
  thread-safe Mask. Header is omitted when the mask is 0.
* ObjectReadSessionState: a Mask shared by a session's root state and
  all forkChild() states; forkChild() sets MULTISTREAM_IN_MRD and
  getOpenArguments() attaches the header on subsequent stream opens.
* UnifiedOpts.Headers.BLOCKLIST: reject user-supplied
  x-goog-storage-java-features.

Parity with google-cloud-go `x-goog-storage-go-features`
(googleapis/google-cloud-go#20684).

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces client feature tracking for the Google Cloud Storage Java SDK, specifically tracking when a read session forks into multiple streams (represented by the MULTISTREAM_IN_MRD feature). It adds the FeatureTracking utility to encode a 32-bit feature mask into a URL-safe Base64 header and integrates this tracking into ObjectReadSessionState. The new header is also blocklisted from manual user configuration. Feedback on the changes suggests optimizing the encoding process in FeatureTracking.encode by using bitwise shifts to extract bytes from the integer mask directly, thereby avoiding the allocation of a ByteBuffer object.

Comment on lines +53 to +57
static String encode(int mask) {
ByteBuffer buf = ByteBuffer.allocate(4);
buf.putInt(mask); // ByteBuffer default byte order is BIG_ENDIAN
return ENCODER.encodeToString(buf.array());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

We can optimize the encoding process by avoiding the allocation of a ByteBuffer object. Since we are dealing with a 32-bit integer, we can directly extract the bytes using bitwise shifts.

  static String encode(int mask) {
    byte[] bytes = new byte[] {
      (byte) (mask >>> 24),
      (byte) (mask >>> 16),
      (byte) (mask >>> 8),
      (byte) mask
    };
    return ENCODER.encodeToString(bytes);
  }

* Move the bit-mask derivation onto the enum (TrackedFeatures#bitMask)
  and drop the static FeatureTracking#bit helper.
* Make FeatureTracking#headers(int) private; production code only goes
  through Mask#headers(). Remove the unused Mask#isSet, mark the
  test-only accessors @VisibleForTesting, and drop the redundant
  explicit Mask constructor.
* Encode with Guava Ints.toByteArray (big-endian by contract) instead
  of a ByteBuffer and a byte-order comment.
* Annotate Mask @threadsafe (consistent with ByteRangeSpec) and fix
  Javadoc summary fragments to the declarative form.

No behavior change; all additions remain package-private.
FeatureTrackingTest
* Encoding vectors for 0, 1, 2, 3, 4, 0xF8 ('-'), 0xFC ('_'),
  0x80000000 and 0xFFFFFFFF; every single bit is URL-safe and padded.
* Mask: empty headers when nothing is set, literal wire header name and
  value, idempotent set, and a deterministic multi-threaded test that
  concurrent set() calls lose no bits.
* Bit positions unique and < 32; MULTISTREAM_IN_MRD pinned to bit 0.
* Blocklist: exact and mixed-case header rejected by
  UnifiedOpts.extraHeaders and by the public BlobSourceOption factory
  (transport agnostic, so both gRPC and HTTP/JSON paths are covered).

ObjectReadSessionStateTest
* Header absent before fork; present on child and on parent after fork;
  a second fork / grandchild fork does not duplicate or alter the value;
  x-goog-request-params (with routing token) and user supplied extra
  headers on the base context coexist with the feature header.

ITObjectReadSessionFakeTest
* singleStreamSessionNeverSendsFeatureTrackingHeader: two sequential
  reads sharing one stream never send the header.
* forkedStreamCarriesFeatureTrackingHeader: moved below the test whose
  Javadoc it had been inserted in front of; redundant assertions
  removed.
* streamReopenedAfterForkCarriesFeatureTrackingHeader: after a fork, the
  original stream is failed with UNAVAILABLE; the retry re-open of that
  stream also carries the header (3 streams, header on exactly 2).
* Rename enum TrackedFeatures -> TrackedFeature (singular, matching the
  C++ TrackedFeature / Go trackedFeature naming).
* Remove getBitPosition(); it was only used by tests. bitMask() remains
  the single accessor. Tests now assert MULTISTREAM_IN_MRD.bitMask() == 1
  and that every bitMask() is a distinct single bit.
* Mask#set uses accumulateAndGet(bitMask, (a, b) -> a | b).
* Clarify the enum Javadoc: external tooling parses the NAME(bit)
  constants, the type name is irrelevant to it.
@cpriti-os
cpriti-os marked this pull request as ready for review October 26, 2026 07:09
@cpriti-os
cpriti-os requested review from a team as code owners October 26, 2026 07:09

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant