Add request-level MetricPublisher support to the S3 CRT client by jencymaryjoseph · Pull Request #7316 · aws/aws-sdk-java-v2

jencymaryjoseph · 2026-08-25T19:06:30Z

Motivation and Context

Follow-up to the client-level MetricPublisher support (#7299). This adds request-level metric publishers to the S3 CRT-based client: a publisher attached to a single request via overrideConfiguration(o -> o.addMetricPublisher(...)) now receives that request's CRT telemetry, matching the standard S3 client. When a request sets its own publishers, they take precedence over the client-level ones for that request; an empty request-level list falls back to the client-level publishers, same as the standard client.

Modifications

Request-level publishers are moved off the request's override configuration and routed to the CRT
transport, so the inner standard client stays publisher-free (no hollow, duplicate ApiCall metric) and
telemetry is published from onTelemetry, once per underlying CRT request.

DefaultS3CrtAsyncClient
- invokeOperation strips the request-level publishers off the override viastashRequestMetricPublishers and stashes them in a request execution attribute; an interceptor bridges that to the transport's SDK-HTTP attribute.
- copyObject stashes once at the entry point. copyObject's sub-requests bypass invokeOperation, so a single stash there plus the copy helper's existing override propagation carries the publishers to every sub-request, with no per-sub-request wrapper.
- Removed the interceptor guard that rejected request-level metric publishers.
S3CrtAsyncHttpClient resolves request-level vs client-level publishers (request-level takes precedence) and folds the effective set back into the execution attributes for the response adapter.
CopyObjectHelper now propagates the copy request's override configuration to the tagging and annotation sub-requests (GetObjectTagging / PutObjectTagging / annotation reads and writes), which were previously built without it. This was a pre-existing gap: an override set on a copy was silently dropped for those sub-requests, so their CRT telemetry never reached a request-level publisher. All copy sub-requests (head / create / part-copies / complete, and now tagging / annotation) inherit the copy's override.

Testing

Unit (S3CrtMetricPublisherResolutionTest): the resolution matrix (none / client-only / request-only / request-overrides-client) and the stash transform (for a normal operation and for copyObject).
Integration (S3CrtClientMetricPublisherIntegrationTest): client-level single-part / multipart get, put, and failed paths; request-level getObject and copyObject publish to the request-level publisher while the client-level one is bypassed; and a tagging-directive copy verifies itsGetObjectTagging and PutObjectTagging sub-requests publish to the request-level publisher, exercising the CopyObjectHelper propagation fix end to end.

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

I have read the CONTRIBUTING document
Local run of mvn install succeeds
My code follows the code style of this project
My change requires a change to the Javadoc documentation
I have updated the Javadoc documentation accordingly
I have added tests to cover my changes
All new and existing tests passed
I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

I confirm that this pull request can be released under the Apache 2 license

Fred1155 · 2026-08-25T20:48:25Z

+     * Request-level publishers (if any were set on the request override) take precedence over the client-level
+     * publishers; otherwise the client-level publishers are used.
+     */
+    @SdkTestInternalApi


Why is it a test internal api?

ah, good catch. These aren't test only apis. Removing all such references.

Fred1155 · 2026-08-25T21:00:03Z

+        return operation.apply(stashRequestMetricPublishers(request));
+    }
+
+    @SdkTestInternalApi


Same here and below

Fred1155 · 2026-08-25T21:25:37Z

-        long partSizeInBytes = builder.minimalPartSizeInBytes == null ? DEFAULT_PART_SIZE_IN_BYTES :
-                               builder.minimalPartSizeInBytes;
-        long thresholdInBytes = builder.thresholdInBytes == null ? partSizeInBytes : builder.thresholdInBytes;
+        this.copyPartSizeInBytes = builder.minimalPartSizeInBytes == null ? DEFAULT_PART_SIZE_IN_BYTES :


Nit: though these fields are only used by copy helper, they are still general partSize and threshold, suggesting keep the original name

Fred1155 · 2026-08-25T21:35:23Z

+     * publishers. Used only by {@link #copyObject(CopyObjectRequest)} when the copy request specifies request-level
+     * publishers, so each copy sub-request publishes CRT telemetry to them instead of to the client-level publishers.
+     */
+    static final class RequestMetricPublisherInjectingClient extends DelegatingS3AsyncClient {


Hope to confirm: this includes head/create/part-copies/complete and tagging/annotation reads and writes, this is the intended design right?

… of a per-sub-request wrapper

zoewangg · 2026-08-26T17:01:38Z


    @Test
    void failedGetObject_publishesUnsuccessfulApiCallMetrics() throws InterruptedException {
        try {


Can we use wiremock tests instead for error cases since it's more deterministic?

Added a deterministic wiremock test in S3CrtClientWiremockTest

zoewangg · 2026-08-26T17:02:27Z

+        }
+    }
+
+    @Test


Should we add assertThat(clientPublisher.awaitAtLeast(1, Duration.ofSeconds(1))).isEmpty();?

Good catch, added.

zoewangg · 2026-08-26T17:03:49Z

+        // The adapter reads its inputs from the execution attributes. METRIC_PUBLISHERS currently holds the
+        // request-level publishers (if any) the interceptor stashed; resolve them against the client-level publishers
+        // (request-level takes precedence) and attach the effective set here when there are any. toBuilder() preserves
+        // everything already in the bag (including CRT_PROGRESS_LISTENER); skip the copy entirely when no publishers
+        // are configured, to avoid rebuilding the bag on every request.


Are the comments here necessary? Seems pretty straightforward

Agreed, removed

zoewangg · 2026-08-26T17:07:56Z

    }

    private CompletableFuture<List<Tag>> fetchSourceTagging(CopyObjectRequest copyObjectRequest, String sourceVersionId) {
+        AwsRequestOverrideConfiguration override = copyObjectRequest.overrideConfiguration().orElse(null);


Can we add some tests?

Added unit tests in CopyObjectHelperTest verifying the copy's override propagates to every tagging and annotation sub request - getObjectTagging/putObjectTagging and listObjectAnnotations/getObjectAnnotation/putObjectAnnotation.

github-actions · 2026-08-27T00:10:39Z

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

add request-level MetricPublisher support to the S3 CRT client

fad00ae

jencymaryjoseph requested a review from a team as a code owner August 25, 2026 19:06

jencymaryjoseph requested a review from Fred1155 August 25, 2026 19:11

zoewangg self-requested a review August 25, 2026 20:34

Fred1155 reviewed Aug 25, 2026

View reviewed changes

drop @SdkTestInternalApi from CRT request-level metric helpers

cba59c3

Fred1155 reviewed Aug 25, 2026

View reviewed changes

refactor : stash request-level publishers once for copyObject instead…

898bf8a

… of a per-sub-request wrapper

zoewangg reviewed Aug 26, 2026

View reviewed changes

address review comments

6b6c556

jencymaryjoseph requested review from Fred1155 and zoewangg August 26, 2026 19:10

zoewangg approved these changes Aug 26, 2026

View reviewed changes

jencymaryjoseph merged commit ab4f33f into feature/master/S3CRTMetricPublisher Aug 27, 2026
5 checks passed

github-actions Bot locked as resolved and limited conversation to collaborators Aug 27, 2026

jencymaryjoseph deleted the jencyjos/s3crt/request-metrics branch August 27, 2026 04:04

+                      }
+                  }
+                  @Test

Uh oh!

Conversation

jencymaryjoseph commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

Modifications

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jencymaryjoseph commented Aug 25, 2026 •

edited

Loading