Skip to content

Commit 3bcbb1c

Browse files
authored
Move ApplyUserAgentStage to later stage in request pipeline (#6513)
* Move ApplyUserAgentStage to later position in request pipeline * Fixing gzip test * Add gzip sync test
1 parent 436a19b commit 3bcbb1c

File tree

5 files changed

+35
-5
lines changed

5 files changed

+35
-5
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "feature",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Move `ApplyUserAgentStage` to later position in request pipeline to ensure all business metrics are properly recorded before finalizing user agent."
6+
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonAsyncHttpClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,12 @@ public <OutputT> CompletableFuture<OutputT> execute(
193193
.first(RequestPipelineBuilder
194194
.first(MakeRequestMutableStage::new)
195195
.then(ApplyTransactionIdStage::new)
196-
.then(ApplyUserAgentStage::new)
197196
.then(MergeCustomHeadersStage::new)
198197
.then(MergeCustomQueryParamsStage::new)
199198
.then(QueryParametersToBodyStage::new)
200199
.then(() -> new CompressRequestStage(httpClientDependencies))
201200
.then(() -> new HttpChecksumStage(ClientType.ASYNC))
201+
.then(ApplyUserAgentStage::new)
202202
.then(MakeRequestImmutableStage::new)
203203
.then(RequestPipelineBuilder
204204
.first(AsyncSigningStage::new)

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/AmazonSyncHttpClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,12 @@ public <OutputT> OutputT execute(HttpResponseHandler<Response<OutputT>> response
181181
.first(RequestPipelineBuilder
182182
.first(MakeRequestMutableStage::new)
183183
.then(ApplyTransactionIdStage::new)
184-
.then(ApplyUserAgentStage::new)
185184
.then(MergeCustomHeadersStage::new)
186185
.then(MergeCustomQueryParamsStage::new)
187186
.then(QueryParametersToBodyStage::new)
188187
.then(() -> new CompressRequestStage(httpClientDependencies))
189188
.then(() -> new HttpChecksumStage(ClientType.SYNC))
189+
.then(ApplyUserAgentStage::new)
190190
.then(MakeRequestImmutableStage::new)
191191
// End of mutating request
192192
.then(RequestPipelineBuilder

core/sdk-core/src/main/java/software/amazon/awssdk/core/useragent/BusinessMetricFeatureId.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public enum BusinessMetricFeatureId {
3434
RETRY_MODE_STANDARD("E"),
3535
RETRY_MODE_ADAPTIVE("F"),
3636
S3_TRANSFER("G"),
37-
GZIP_REQUEST_COMPRESSION("L"), //TODO(metrics): Not working, compression happens after header
37+
GZIP_REQUEST_COMPRESSION("L"),
3838
PROTOCOL_RPC_V2_CBOR("M"),
3939
ENDPOINT_OVERRIDE("N"),
4040
ACCOUNT_ID_MODE_PREFERRED("P"),

test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/BusinessMetricsUserAgentTest.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
import software.amazon.awssdk.regions.Region;
4949
import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClient;
5050
import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClientBuilder;
51+
import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient;
52+
import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClientBuilder;
5153
import software.amazon.awssdk.services.protocolrestjson.internal.ServiceVersionInfo;
5254
import software.amazon.awssdk.services.protocolrestjson.model.PaginatedOperationWithResultKeyResponse;
5355
import software.amazon.awssdk.services.protocolrestjson.paginators.PaginatedOperationWithResultKeyPublisher;
@@ -162,7 +164,7 @@ void when_paginatedOperationIsCalled_correctMetricIsAdded() throws Exception {
162164
}
163165

164166
@Test
165-
void when_compressedOperationIsCalled_metricIsRecordedButNotAddedToUserAgentString() throws Exception {
167+
void when_asyncCompressedOperationIsCalled_metricIsRecordedAndAddedToUserAgentString() throws Exception {
166168
ProtocolRestJsonAsyncClientBuilder clientBuilder = asyncClientBuilderForProtocolRestJson();
167169

168170
assertThatThrownBy(() -> clientBuilder.build().putOperationWithRequestCompression(r -> r.body(SdkBytes.fromUtf8String(
@@ -173,7 +175,22 @@ void when_compressedOperationIsCalled_metricIsRecordedButNotAddedToUserAgentStri
173175
BusinessMetricCollection attribute = interceptor.executionAttributes().getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS);
174176
assertThat(attribute).isNotNull();
175177
assertThat(attribute.recordedMetrics()).contains(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value());
176-
assertThat(userAgent).doesNotMatch(METRIC_SEARCH_PATTERN.apply(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value()));
178+
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value()));
179+
}
180+
181+
@Test
182+
void when_syncCompressedOperationIsCalled_metricIsRecordedAndAddedToUserAgentString() throws Exception {
183+
ProtocolRestJsonClientBuilder clientBuilder = syncClientBuilderForProtocolRestJson();
184+
185+
assertThatThrownBy(() -> clientBuilder.build().putOperationWithRequestCompression(r -> r.body(SdkBytes.fromUtf8String(
186+
"whoo")).overrideConfiguration(o -> o.compressionConfiguration(c -> c.minimumCompressionThresholdInBytes(1)))))
187+
.hasMessageContaining("stop");
188+
189+
String userAgent = assertAndGetUserAgentString();
190+
BusinessMetricCollection attribute = interceptor.executionAttributes().getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS);
191+
assertThat(attribute).isNotNull();
192+
assertThat(attribute.recordedMetrics()).contains(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value());
193+
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(BusinessMetricFeatureId.GZIP_REQUEST_COMPRESSION.value()));
177194
}
178195

179196
private String assertAndGetUserAgentString() {
@@ -196,6 +213,13 @@ private ProtocolRestJsonAsyncClientBuilder asyncClientBuilderForProtocolRestJson
196213
.overrideConfiguration(c -> c.addExecutionInterceptor(interceptor));
197214
}
198215

216+
private ProtocolRestJsonClientBuilder syncClientBuilderForProtocolRestJson() {
217+
return ProtocolRestJsonClient.builder()
218+
.region(Region.US_WEST_2)
219+
.credentialsProvider(CREDENTIALS_PROVIDER)
220+
.overrideConfiguration(c -> c.addExecutionInterceptor(interceptor));
221+
}
222+
199223
public static class CapturingInterceptor implements ExecutionInterceptor {
200224
private Context.BeforeTransmission context;
201225
private ExecutionAttributes executionAttributes;

0 commit comments

Comments
 (0)