Skip to content

Commit f50c8eb

Browse files
authored
remove sdkId transform by default (smithy-lang#4064)
## Motivation and Context fixes awslabs/aws-sdk-rust#1252 ## Description The service ID we [were setting](https://github.com/smithy-lang/smithy-rs/blob/5f7113f506f301296311ef637e40d226e0a6e548/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/ServiceEnvConfigDecorator.kt#L27) for service specific environment configuration was wrong. It was wrong in two ways: 1. The [sdkId](https://github.com/smithy-lang/smithy-rs/blob/7db3e9f08e422e386c56d5916c69ab495d739ee8/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt#L157) extension we defined strips whitespace and lowercases the ID from the model. 2. It also used `toSnakeCase()` which will not have the same effect as internally that uses `splitOnWordBoundaries` which uses heuristics to determine where a word boundary is and isn't restricted to spaces. It looks like we've always applied this transform to sdkId from what I can tell with the original use being to set the operation metadata for the service name and it was carried through when making an extension. This PR makes `sdkId()` return the ID from the model untouched. I think this is the safest default as remembering when you can use the extension vs not is error prone. Any required/desired transformations should be done localized to where it's used. In this case the `ServiceEnvConfigDecorator` doesn't need to transform sdkId as the [runtime already does it](https://github.com/smithy-lang/smithy-rs/blob/d1bbd018618da9ab8f8bdb1f27d9ec75c42d2505/aws/rust-runtime/aws-runtime/src/env_config.rs#L351) ## Testing Tested on the bedrock-agent and bedrock-agent-runtime models described in the issue ## Questions There were only four uses of the `sdkId()` function I could find: 1. Operation metadata 2. Span names 3. Service env config 4. Default retry partition naming I think this is a safe change though it will result in different log output. Unclear if anyone would be predicating off service name if we expose it anywhere (e.g. in an interceptor). ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] For changes to the AWS SDK, generated SDK code, or SDK runtime crates, I have created a changelog entry Markdown file in the `.changelog` directory, specifying "aws-sdk-rust" in the `applies_to` key. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent f10556d commit f50c8eb

File tree

9 files changed

+41
-29
lines changed

9 files changed

+41
-29
lines changed

Diff for: .changelog/1743016749.md

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
applies_to:
3+
- aws-sdk-rust
4+
authors:
5+
- aajtodd
6+
references:
7+
- aws-sdk-rust#1252
8+
breaking: false
9+
new_feature: false
10+
bug_fix: true
11+
---
12+
Fix service specific endpoint url keys

Diff for: aws/rust-runtime/Cargo.lock

+4-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/ServiceEnvConfigDecorator.kt

+1-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
1313
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
1414
import software.amazon.smithy.rust.codegen.core.util.dq
1515
import software.amazon.smithy.rust.codegen.core.util.sdkId
16-
import software.amazon.smithy.rust.codegen.core.util.toSnakeCase
1716

1817
class ServiceEnvConfigDecorator : ClientCodegenDecorator {
1918
override val name: String = "ServiceEnvConfigDecorator"
@@ -24,7 +23,7 @@ class ServiceEnvConfigDecorator : ClientCodegenDecorator {
2423
rustCrate: RustCrate,
2524
) {
2625
val rc = codegenContext.runtimeConfig
27-
val serviceId = codegenContext.serviceShape.sdkId().toSnakeCase().dq()
26+
val serviceId = codegenContext.serviceShape.sdkId().dq()
2827
rustCrate.withModule(ClientRustModule.config) {
2928
Attribute.AllowDeadCode.render(this)
3029
rustTemplate(

Diff for: aws/sdk/Cargo.lock

+6-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: aws/sdk/integration-tests/telemetry/tests/metrics.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ async fn metrics_have_expected_attributes() {
8686
let call_duration_attributes =
8787
extract_metric_attributes(&finished_metrics, "smithy.client.call.duration");
8888
assert!(call_duration_attributes[0].contains(&KeyValue::new("rpc.method", "GetObject")));
89-
assert!(call_duration_attributes[0].contains(&KeyValue::new("rpc.service", "s3")));
89+
assert!(call_duration_attributes[0].contains(&KeyValue::new("rpc.service", "S3")));
9090

9191
let attempt_duration_attributes =
9292
extract_metric_attributes(&finished_metrics, "smithy.client.call.attempt.duration");
9393
assert!(attempt_duration_attributes[0].contains(&KeyValue::new("rpc.method", "GetObject")));
94-
assert!(attempt_duration_attributes[0].contains(&KeyValue::new("rpc.service", "s3")));
94+
assert!(attempt_duration_attributes[0].contains(&KeyValue::new("rpc.service", "S3")));
9595

9696
// The attempt metric contains an attempt counter attribute that correctly increments
9797
assert!(attempt_duration_attributes

Diff for: aws/sdk/integration-tests/telemetry/tests/spans.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ async fn top_level_spans_exist_with_correct_attributes() {
2323
let ddb_top_level: fn() -> Box<dyn Visit + 'static> = || Box::new(DdbTestVisitor);
2424
let subscriber = tracing_subscriber::registry::Registry::default().with(TestLayer {
2525
visitor_factories: HashMap::from([
26-
("s3.GetObject", s3_top_level),
27-
("dynamodb.GetItem", ddb_top_level),
26+
("S3.GetObject", s3_top_level),
27+
("DynamoDB.GetItem", ddb_top_level),
2828
]),
2929
});
3030
let _guard = tracing::subscriber::set_default(subscriber);
@@ -61,7 +61,7 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() {
6161
let subscriber = base_subscriber.with(AssertionsLayer::new(&assertion_registry));
6262
let _guard = tracing::subscriber::set_default(subscriber);
6363

64-
const OPERATION_NAME: &str = "s3.GetObject";
64+
const OPERATION_NAME: &str = "S3.GetObject";
6565
const INVOKE: &str = "invoke";
6666
const TRY_OP: &str = "try_op";
6767
const TRY_ATTEMPT: &str = "try_attempt";
@@ -311,7 +311,7 @@ impl Visit for S3TestVisitor {
311311
if field_name == "rpc.system" {
312312
assert_eq!("aws-api".to_string(), field_value);
313313
} else if field_name == "rpc.service" {
314-
assert_eq!("s3".to_string(), field_value);
314+
assert_eq!("S3".to_string(), field_value);
315315
} else if field_name == "rpc.method" {
316316
assert_eq!("GetObject".to_string(), field_value);
317317
} else if field_name == "sdk_invocation_id" {
@@ -333,7 +333,7 @@ impl Visit for DdbTestVisitor {
333333
if field_name == "rpc.system" {
334334
assert_eq!("aws-api".to_string(), field_value);
335335
} else if field_name == "rpc.service" {
336-
assert_eq!("dynamodb".to_string(), field_value);
336+
assert_eq!("DynamoDB".to_string(), field_value);
337337
} else if field_name == "rpc.method" {
338338
assert_eq!("GetItem".to_string(), field_value);
339339
} else if field_name == "sdk_invocation_id" {

Diff for: codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ private fun baseClientRuntimePluginsFn(
253253
RuntimeType.forInlineFun("base_client_runtime_plugins", ClientRustModule.config) {
254254
val api = RuntimeType.smithyRuntimeApiClient(rc)
255255
val rt = RuntimeType.smithyRuntime(rc)
256+
val serviceId = codegenContext.serviceShape.sdkId().lowercase().replace(" ", "")
256257
val behaviorVersionError =
257258
"Invalid client configuration: A behavior major version must be set when sending a " +
258259
"request or constructing a client. You must set it during client construction or by enabling the " +
@@ -266,7 +267,7 @@ private fun baseClientRuntimePluginsFn(
266267
::std::mem::swap(&mut config.runtime_plugins, &mut configured_plugins);
267268
#{update_bmv}
268269
269-
let default_retry_partition = ${codegenContext.serviceShape.sdkId().dq()};
270+
let default_retry_partition = ${serviceId.dq()};
270271
#{before_plugin_setup}
271272
272273
let scope = ${codegenContext.moduleName.dq()};

Diff for: codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,4 @@ fun String.shapeId() = ShapeId.from(this)
154154
fun ServiceShape.serviceNameOrDefault(default: String) = getTrait<TitleTrait>()?.value ?: default
155155

156156
/** Returns the SDK ID of the given service shape */
157-
fun ServiceShape.sdkId(): String = getTrait<ServiceTrait>()?.sdkId?.lowercase()?.replace(" ", "") ?: id.getName(this)
157+
fun ServiceShape.sdkId(): String = getTrait<ServiceTrait>()?.sdkId ?: id.getName(this)

Diff for: rust-runtime/Cargo.lock

+8-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)