Avoid emitting repeated business metrics (#4392)

## Motivation and Context
P334648004

## Description
At times, the Rust SDK emits the business metric looking as follows:
```
M/E,b,0,0,0,0,0,0
```
The issue is due to [the retry
loop](https://github.com/smithy-lang/smithy-rs/blob/54f42235c9bce0be0207d5d00a6b35c2501e81d1/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs#L290)
in the orchestrator, where any features added to the config bag within
`try_attempt` persist throughout subsequent retry iterations (we do have
the
[rewind](https://github.com/smithy-lang/smithy-rs/blob/54f42235c9bce0be0207d5d00a6b35c2501e81d1/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs#L367)
method but it currently does not touch the config bag at all).

This PR uniquifies business metric values emitted in
`UserAgentInterceptor`. It also fixes a bug related to the order for
emitting `AwsCredentialFeature` where it previously emitted their metric
values in a reversed order.

## Dismissed alternative
We discussed an alternative approach involving a separate interceptor
that runs `modify_before_attempt_completion` that removes
`AwsCredentialsFeature` from config bag so the next retry loop can start
from the clean slate. This would require extra code, and the approach
employed in this PR is simpler. Plus, should the need for the cleanup
interceptor arise, it can co-exist with the fix of this PR.

## Testing
- CI
- Unit tests in `UserAgentInterceptor`
- Codegen test in `UserAgentDecoratorTest`

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

Author: ysaito1001

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

@@ -145,6 +145,82 @@ class UserAgentDecoratorTest {
         }
     }
 
+    @Test
+    fun `it avoids emitting repeated business metrics on retry`() {
+        awsSdkIntegrationTest(model) { context, rustCrate ->
+            rustCrate.integrationTest("business_metrics") {
+                tokioTest("metrics_should_not_be_repeated") {
+                    val rc = context.runtimeConfig
+                    val moduleName = context.moduleUseName()
+                    rustTemplate(
+                        """
+                        use $moduleName::config::{AppName, Credentials, Region, SharedCredentialsProvider};
+                        use $moduleName::{Config, Client};
+
+                        let http_client = #{StaticReplayClient}::new(vec![
+                            #{ReplayEvent}::new(
+                                #{http}::Request::builder()
+                                    .uri("http://localhost:1234/")
+                                    .body(#{SdkBody}::empty())
+                                    .unwrap(),
+                                #{http}::Response::builder()
+                                    .status(500)
+                                    .body(#{SdkBody}::empty())
+                                    .unwrap(),
+                            ),
+                            #{ReplayEvent}::new(
+                                #{http}::Request::builder()
+                                    .uri("http://localhost:1234/")
+                                    .body(#{SdkBody}::empty())
+                                    .unwrap(),
+                                #{http}::Response::builder()
+                                    .status(200)
+                                    .body(#{SdkBody}::empty())
+                                    .unwrap(),
+                            ),
+                        ]);
+
+                        let mut creds = Credentials::for_tests();
+                        creds.get_property_mut_or_default::<Vec<#{AwsCredentialFeature}>>()
+                            .push(#{AwsCredentialFeature}::CredentialsEnvVars);
+
+                        let config = Config::builder()
+                            .credentials_provider(SharedCredentialsProvider::new(creds))
+                            .retry_config(#{RetryConfig}::standard().with_max_attempts(2))
+                            .region(Region::new("us-east-1"))
+                            .http_client(http_client.clone())
+                            .app_name(AppName::new("test-app-name").expect("valid app name"))
+                            .build();
+                        let client = Client::from_conf(config);
+                        let _ = client.some_operation().send().await;
+
+                        let req = http_client.actual_requests().last().unwrap();
+                        let aws_ua_header = req.headers().get("x-amz-user-agent").unwrap();
+                        let metrics_section = aws_ua_header
+                            .split(" m/")
+                            .nth(1)
+                            .unwrap()
+                            .split_ascii_whitespace()
+                            .nth(0)
+                            .unwrap();
+                        assert_eq!(1, metrics_section.matches("g").count());
+                        """,
+                        "http" to RuntimeType.Http,
+                        "AwsCredentialFeature" to
+                            AwsRuntimeType.awsCredentialTypes(rc)
+                                .resolve("credential_feature::AwsCredentialFeature"),
+                        "RetryConfig" to RuntimeType.smithyTypes(rc).resolve("retry::RetryConfig"),
+                        "ReplayEvent" to RuntimeType.smithyHttpClientTestUtil(rc).resolve("test_util::ReplayEvent"),
+                        "SdkBody" to RuntimeType.sdkBody(rc),
+                        "StaticReplayClient" to
+                            RuntimeType.smithyHttpClientTestUtil(rc)
+                                .resolve("test_util::StaticReplayClient"),
+                    )
+                }
+            }
+        }
+    }
+
     @Test
     fun `it emits business metric for RPC v2 CBOR in user agent`() {
         val model =

aws/rust-runtime/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "aws-runtime"
-version = "1.5.15"
+version = "1.5.16"
 authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>"]
 description = "Runtime support code for the AWS SDK. This crate isn't intended to be used directly."
 edition = "2021"

aws/rust-runtime/aws-runtime/Cargo.toml

@@ -25,6 +25,36 @@ use crate::sdk_feature::AwsSdkFeature;
 use crate::user_agent::metrics::ProvideBusinessMetric;
 use crate::user_agent::{AdditionalMetadata, ApiMetadata, AwsUserAgent, InvalidMetadataValue};
 
+macro_rules! add_metrics_unique {
+    ($features:expr, $ua:expr, $added:expr) => {
+        for feature in $features {
+            if let Some(m) = feature.provide_business_metric() {
+                if !$added.contains(&m) {
+                    $added.insert(m.clone());
+                    $ua.add_business_metric(m);
+                }
+            }
+        }
+    };
+}
+
+macro_rules! add_metrics_unique_reverse {
+    ($features:expr, $ua:expr, $added:expr) => {
+        let mut unique_metrics = Vec::new();
+        for feature in $features {
+            if let Some(m) = feature.provide_business_metric() {
+                if !$added.contains(&m) {
+                    $added.insert(m.clone());
+                    unique_metrics.push(m);
+                }
+            }
+        }
+        for m in unique_metrics.into_iter().rev() {
+            $ua.add_business_metric(m);
+        }
+    };
+}
+
 #[allow(clippy::declare_interior_mutable_const)] // we will never mutate this
 const X_AMZ_USER_AGENT: HeaderName = HeaderName::from_static("x-amz-user-agent");
 
@@ -133,26 +163,17 @@ impl Intercept for UserAgentInterceptor {
             .expect("`AwsUserAgent should have been created in `read_before_execution`")
             .clone();
 
-        let smithy_sdk_features = cfg.load::<SmithySdkFeature>();
-        for smithy_sdk_feature in smithy_sdk_features {
-            smithy_sdk_feature
-                .provide_business_metric()
-                .map(|m| ua.add_business_metric(m));
-        }
+        let mut added_metrics = std::collections::HashSet::new();
 
-        let aws_sdk_features = cfg.load::<AwsSdkFeature>();
-        for aws_sdk_feature in aws_sdk_features {
-            aws_sdk_feature
-                .provide_business_metric()
-                .map(|m| ua.add_business_metric(m));
-        }
-
-        let aws_credential_features = cfg.load::<AwsCredentialFeature>();
-        for aws_credential_feature in aws_credential_features {
-            aws_credential_feature
-                .provide_business_metric()
-                .map(|m| ua.add_business_metric(m));
-        }
+        add_metrics_unique!(cfg.load::<SmithySdkFeature>(), &mut ua, &mut added_metrics);
+        add_metrics_unique!(cfg.load::<AwsSdkFeature>(), &mut ua, &mut added_metrics);
+        // The order we emit credential features matters.
+        // Reverse to preserve emission order since StoreAppend pops backwards.
+        add_metrics_unique_reverse!(
+            cfg.load::<AwsCredentialFeature>(),
+            &mut ua,
+            &mut added_metrics
+        );
 
         let maybe_connector_metadata = runtime_components
             .http_client()
@@ -259,6 +280,80 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_modify_before_signing_no_duplicate_metrics() {
+        let rc = RuntimeComponentsBuilder::for_tests().build().unwrap();
+        let mut context = context();
+
+        let api_metadata = ApiMetadata::new("test-service", "1.0");
+        let mut layer = Layer::new("test");
+        layer.store_put(api_metadata);
+        // Duplicate features
+        layer.store_append(SmithySdkFeature::Waiter);
+        layer.store_append(SmithySdkFeature::Waiter);
+        layer.store_append(AwsSdkFeature::S3Transfer);
+        layer.store_append(AwsSdkFeature::S3Transfer);
+        layer.store_append(AwsCredentialFeature::CredentialsCode);
+        layer.store_append(AwsCredentialFeature::CredentialsCode);
+        let mut config = ConfigBag::of_layers(vec![layer]);
+
+        let interceptor = UserAgentInterceptor::new();
+        let ctx = Into::into(&context);
+        interceptor
+            .read_after_serialization(&ctx, &rc, &mut config)
+            .unwrap();
+        let mut ctx = Into::into(&mut context);
+        interceptor
+            .modify_before_signing(&mut ctx, &rc, &mut config)
+            .unwrap();
+
+        let aws_ua_header = expect_header(&context, "x-amz-user-agent");
+        let metrics_section = aws_ua_header.split(" m/").nth(1).unwrap();
+        let waiter_count = metrics_section.matches("B").count();
+        let s3_transfer_count = metrics_section.matches("G").count();
+        let credentials_code_count = metrics_section.matches("e").count();
+        assert_eq!(
+            1, waiter_count,
+            "Waiter metric should appear only once, but found {waiter_count} occurrences in: {aws_ua_header}",
+        );
+        assert_eq!(1, s3_transfer_count, "S3Transfer metric should appear only once, but found {s3_transfer_count} occurrences in metrics section: {aws_ua_header}");
+        assert_eq!(1, credentials_code_count, "CredentialsCode metric should appear only once, but found {credentials_code_count} occurrences in metrics section: {aws_ua_header}");
+    }
+
+    #[test]
+    fn test_metrics_order_preserved() {
+        use aws_credential_types::credential_feature::AwsCredentialFeature;
+
+        let rc = RuntimeComponentsBuilder::for_tests().build().unwrap();
+        let mut context = context();
+
+        let api_metadata = ApiMetadata::new("test-service", "1.0");
+        let mut layer = Layer::new("test");
+        layer.store_put(api_metadata);
+        layer.store_append(AwsCredentialFeature::CredentialsCode);
+        layer.store_append(AwsCredentialFeature::CredentialsEnvVars);
+        layer.store_append(AwsCredentialFeature::CredentialsProfile);
+        let mut config = ConfigBag::of_layers(vec![layer]);
+
+        let interceptor = UserAgentInterceptor::new();
+        let ctx = Into::into(&context);
+        interceptor
+            .read_after_serialization(&ctx, &rc, &mut config)
+            .unwrap();
+        let mut ctx = Into::into(&mut context);
+        interceptor
+            .modify_before_signing(&mut ctx, &rc, &mut config)
+            .unwrap();
+
+        let aws_ua_header = expect_header(&context, "x-amz-user-agent");
+        let metrics_section = aws_ua_header.split(" m/").nth(1).unwrap();
+
+        assert_eq!(
+            metrics_section, "e,g,n",
+            "AwsCredentialFeature metrics should preserve order"
+        );
+    }
+
     #[test]
     fn test_app_name() {
         let rc = RuntimeComponentsBuilder::for_tests().build().unwrap();

aws/rust-runtime/aws-runtime/src/user_agent/interceptor.rs

@@ -319,6 +319,9 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null)
 
         fun smithyHttp(runtimeConfig: RuntimeConfig) = CargoDependency.smithyHttp(runtimeConfig).toType()
 
+        fun smithyHttpClientTestUtil(runtimeConfig: RuntimeConfig) =
+            CargoDependency.smithyHttpClientTestUtil(runtimeConfig).toType()
+
         fun smithyJson(runtimeConfig: RuntimeConfig) = CargoDependency.smithyJson(runtimeConfig).toType()
 
         fun smithyQuery(runtimeConfig: RuntimeConfig) = CargoDependency.smithyQuery(runtimeConfig).toType()