Skip to content

Commit 903f043

Browse files
committed
Address code review feedback: improve tests, use type-safe OTel detection, and remove redundant code
1 parent 08203b8 commit 903f043

File tree

19 files changed

+790
-250
lines changed

19 files changed

+790
-250
lines changed

aws/codegen-aws-sdk/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ val DECORATORS: List<ClientCodegenDecorator> =
4646
CredentialsProviderDecorator(),
4747
RegionDecorator(),
4848
RequireEndpointRules(),
49+
EndpointOverrideDecorator(),
4950
UserAgentDecorator(),
5051
SigV4AuthDecorator(),
5152
HttpRequestChecksumDecorator(),
@@ -65,7 +66,6 @@ val DECORATORS: List<ClientCodegenDecorator> =
6566
AwsRequestIdDecorator(),
6667
DisabledAuthDecorator(),
6768
RecursionDetectionDecorator(),
68-
EndpointOverrideDecorator(),
6969
ObservabilityDetectionDecorator(),
7070
InvocationIdDecorator(),
7171
RetryInformationHeaderDecorator(),

aws/codegen-aws-sdk/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointOverrideDecoratorTest.kt

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,7 @@ class EndpointOverrideDecoratorTest {
6767
}
6868

6969
@Test
70-
fun `generated code includes endpoint override interceptor registration`() {
71-
awsSdkIntegrationTest(model) { _, _ ->
72-
// The test passes if the code compiles successfully
73-
// This verifies that the decorator generates valid Rust code
74-
}
75-
}
76-
77-
@Test
78-
fun `generated code compiles with endpoint override interceptor`() {
79-
// Create custom test params with endpoint_url config enabled
70+
fun `endpoint override interceptor adds business metric to user agent`() {
8071
val testParams =
8172
awsIntegrationTestParams().copy(
8273
additionalSettings =
@@ -94,22 +85,58 @@ class EndpointOverrideDecoratorTest {
9485
awsSdkIntegrationTest(model, testParams) { context, rustCrate ->
9586
val rc = context.runtimeConfig
9687
val moduleName = context.moduleUseName()
97-
rustCrate.integrationTest("endpoint_override_compiles") {
98-
tokioTest("can_build_client_with_endpoint_url") {
88+
rustCrate.integrationTest("endpoint_override_functional") {
89+
tokioTest("interceptor_adds_metric_when_endpoint_overridden") {
9990
rustTemplate(
10091
"""
10192
use $moduleName::config::Region;
10293
use $moduleName::{Client, Config};
10394
104-
let (http_client, _rcvr) = #{capture_request}(#{None});
95+
let (http_client, rcvr) = #{capture_request}(#{None});
10596
let config = Config::builder()
10697
.region(Region::new("us-east-1"))
10798
.endpoint_url("https://custom.example.com")
10899
.http_client(http_client.clone())
109-
.with_test_defaults()
110100
.build();
111-
let _client = Client::from_conf(config);
112-
// Test passes if code compiles and client can be created
101+
let client = Client::from_conf(config);
102+
103+
// CRITICAL: Actually make a request
104+
let _ = client.some_operation().send().await;
105+
106+
// Capture and verify the request
107+
let request = rcvr.expect_request();
108+
109+
// Verify endpoint was overridden
110+
let uri = request.uri().to_string();
111+
assert!(
112+
uri.starts_with("https://custom.example.com"),
113+
"Expected custom endpoint, got: {}",
114+
uri
115+
);
116+
117+
// Verify x-amz-user-agent contains business metric 'N' (endpoint override)
118+
// The metric appears in the business-metrics section as "m/..." with comma-separated IDs
119+
let x_amz_user_agent = request.headers()
120+
.get("x-amz-user-agent")
121+
.expect("x-amz-user-agent header missing");
122+
123+
// Extract the business metrics section (starts with "m/")
124+
let has_endpoint_override_metric = x_amz_user_agent
125+
.split_whitespace()
126+
.find(|part| part.starts_with("m/"))
127+
.map(|metrics| {
128+
// Check if 'N' appears as a metric ID (either alone or in a comma-separated list)
129+
metrics.strip_prefix("m/")
130+
.map(|ids| ids.split(',').any(|id| id == "N"))
131+
.unwrap_or(false)
132+
})
133+
.unwrap_or(false);
134+
135+
assert!(
136+
has_endpoint_override_metric,
137+
"Expected metric ID 'N' (endpoint override) in x-amz-user-agent business metrics, got: {}",
138+
x_amz_user_agent
139+
);
113140
""",
114141
*preludeScope,
115142
"capture_request" to RuntimeType.captureRequest(rc),

0 commit comments

Comments
 (0)