diff --git a/.changes/next-release/feature-AWSSDKforJavav2-5eb5d9f.json b/.changes/next-release/feature-AWSSDKforJavav2-5eb5d9f.json new file mode 100644 index 00000000000..372ea366593 --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-5eb5d9f.json @@ -0,0 +1,6 @@ +{ + "type": "feature", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Move `ApplyUserAgentStage` to later position in request pipeline to ensure all business metrics are properly recorded before finalizing user agent." +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonAsyncHttpClient.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonAsyncHttpClient.java index 8dd6db8acf1..59bd964d6fa 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonAsyncHttpClient.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonAsyncHttpClient.java @@ -193,12 +193,12 @@ public CompletableFuture execute( .first(RequestPipelineBuilder .first(MakeRequestMutableStage::new) .then(ApplyTransactionIdStage::new) - .then(ApplyUserAgentStage::new) .then(MergeCustomHeadersStage::new) .then(MergeCustomQueryParamsStage::new) .then(QueryParametersToBodyStage::new) .then(() -> new CompressRequestStage(httpClientDependencies)) .then(() -> new HttpChecksumStage(ClientType.ASYNC)) + .then(ApplyUserAgentStage::new) .then(MakeRequestImmutableStage::new) .then(RequestPipelineBuilder .first(AsyncSigningStage::new) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonSyncHttpClient.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonSyncHttpClient.java index 21fb5cbe130..dccaad1e210 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonSyncHttpClient.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonSyncHttpClient.java @@ -181,12 +181,12 @@ public OutputT execute(HttpResponseHandler> response .first(RequestPipelineBuilder .first(MakeRequestMutableStage::new) .then(ApplyTransactionIdStage::new) - .then(ApplyUserAgentStage::new) .then(MergeCustomHeadersStage::new) .then(MergeCustomQueryParamsStage::new) .then(QueryParametersToBodyStage::new) .then(() -> new CompressRequestStage(httpClientDependencies)) .then(() -> new HttpChecksumStage(ClientType.SYNC)) + .then(ApplyUserAgentStage::new) .then(MakeRequestImmutableStage::new) // End of mutating request .then(RequestPipelineBuilder diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/useragent/BusinessMetricFeatureId.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/useragent/BusinessMetricFeatureId.java index df481f975d5..fe1af17823c 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/useragent/BusinessMetricFeatureId.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/useragent/BusinessMetricFeatureId.java @@ -34,7 +34,7 @@ public enum BusinessMetricFeatureId { RETRY_MODE_STANDARD("E"), RETRY_MODE_ADAPTIVE("F"), S3_TRANSFER("G"), - GZIP_REQUEST_COMPRESSION("L"), //TODO(metrics): Not working, compression happens after header + GZIP_REQUEST_COMPRESSION("L"), PROTOCOL_RPC_V2_CBOR("M"), ENDPOINT_OVERRIDE("N"), ACCOUNT_ID_MODE_PREFERRED("P"), diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/BusinessMetricsUserAgentTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/BusinessMetricsUserAgentTest.java index 204c0e389ed..ab9394e70ac 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/BusinessMetricsUserAgentTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/BusinessMetricsUserAgentTest.java @@ -48,6 +48,8 @@ import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClient; import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClientBuilder; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClientBuilder; import software.amazon.awssdk.services.protocolrestjson.internal.ServiceVersionInfo; import software.amazon.awssdk.services.protocolrestjson.model.PaginatedOperationWithResultKeyResponse; import software.amazon.awssdk.services.protocolrestjson.paginators.PaginatedOperationWithResultKeyPublisher; @@ -162,7 +164,7 @@ void when_paginatedOperationIsCalled_correctMetricIsAdded() throws Exception { } @Test - void when_compressedOperationIsCalled_metricIsRecordedButNotAddedToUserAgentString() throws Exception { + void when_asyncCompressedOperationIsCalled_metricIsRecordedAndAddedToUserAgentString() throws Exception { ProtocolRestJsonAsyncClientBuilder clientBuilder = asyncClientBuilderForProtocolRestJson(); assertThatThrownBy(() -> clientBuilder.build().putOperationWithRequestCompression(r -> r.body(SdkBytes.fromUtf8String( @@ -173,7 +175,22 @@ void when_compressedOperationIsCalled_metricIsRecordedButNotAddedToUserAgentStri BusinessMetricCollection attribute = interceptor.executionAttributes().getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS); assertThat(attribute).isNotNull(); assertThat(attribute.recordedMetrics()).contains(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value()); - assertThat(userAgent).doesNotMatch(METRIC_SEARCH_PATTERN.apply(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value())); + assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value())); + } + + @Test + void when_syncCompressedOperationIsCalled_metricIsRecordedAndAddedToUserAgentString() throws Exception { + ProtocolRestJsonClientBuilder clientBuilder = syncClientBuilderForProtocolRestJson(); + + assertThatThrownBy(() -> clientBuilder.build().putOperationWithRequestCompression(r -> r.body(SdkBytes.fromUtf8String( + "whoo")).overrideConfiguration(o -> o.compressionConfiguration(c -> c.minimumCompressionThresholdInBytes(1))))) + .hasMessageContaining("stop"); + + String userAgent = assertAndGetUserAgentString(); + BusinessMetricCollection attribute = interceptor.executionAttributes().getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS); + assertThat(attribute).isNotNull(); + assertThat(attribute.recordedMetrics()).contains(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value()); + assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value())); } private String assertAndGetUserAgentString() { @@ -196,6 +213,13 @@ private ProtocolRestJsonAsyncClientBuilder asyncClientBuilderForProtocolRestJson .overrideConfiguration(c -> c.addExecutionInterceptor(interceptor)); } + private ProtocolRestJsonClientBuilder syncClientBuilderForProtocolRestJson() { + return ProtocolRestJsonClient.builder() + .region(Region.US_WEST_2) + .credentialsProvider(CREDENTIALS_PROVIDER) + .overrideConfiguration(c -> c.addExecutionInterceptor(interceptor)); + } + public static class CapturingInterceptor implements ExecutionInterceptor { private Context.BeforeTransmission context; private ExecutionAttributes executionAttributes;