Add business metrics for observability and endpoint override #4385

vcjana · 2025-11-06T10:06:16Z

Motivation and Context

This PR implements business metrics tracking for observability and endpoint override features.

Description

This PR adds five new business metrics to track SDK feature usage:

Observability Metrics:

m/4: Observability tracing enabled
m/5: Observability metrics enabled
m/6: OpenTelemetry tracing integration
m/7: OpenTelemetry metrics integration

Endpoint Configuration Metrics:

m/N: Custom endpoint URL override detection

Testing

Integration tests added in aws/sdk/integration-tests/s3/tests/business_metrics.rs covering all seven metrics.

Checklist

For changes to the smithy-rs codegen or runtime crates, I have created a changelog entry Markdown file in the .changelog directory, specifying "client," "server," or both in the applies_to key.
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.

github-actions · 2025-11-11T22:58:21Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2025-11-12T07:23:51Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2025-11-12T09:04:08Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

landonxjames

Good start, would like to see some E2E tests ensuring that the observability and endpoint metrics actually make it in to the user-agent

landonxjames · 2025-11-13T04:44:47Z

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

+    }
+
+    @Test
+    fun `generated code includes endpoint override interceptor registration`() {


This test doesn't quite test what the name suggests it does. It tests that it compiles, but it would compile just find if the EndpointOverride interceptor wasn't inserted at all. Same with the test below. I would try to write a test that actually ensures the interceptor is inserted, likely by making a request with an overridden endpoint and checking the user-agent on that request.

landonxjames · 2025-11-13T04:50:50Z

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

            S3Transfer => Some(BusinessMetric::S3Transfer),
            SsoLoginDevice => Some(BusinessMetric::SsoLoginDevice),
            SsoLoginAuth => Some(BusinessMetric::SsoLoginAuth),
+            ObservabilityMetrics => Some(BusinessMetric::ObservabilityMetrics),


The Observability metrics should probably all be in SmithySdkFeature instead of AwsSdkFeature. Nothing about the observability interfaces is Aws specific.

landonxjames · 2025-11-13T05:10:40Z

aws/rust-runtime/aws-runtime/src/observability_detection.rs

+                cfg.interceptor_state()
+                    .store_append(AwsSdkFeature::ObservabilityMetrics);
+
+                // PRAGMATIC APPROACH: Track tracing based on meter provider state


I would say we should probably just not emit this one since we don't actually support it, and it could be confusing.

landonxjames · 2025-11-13T05:11:35Z

aws/rust-runtime/aws-runtime/src/observability_detection.rs

+
+                    // If using OpenTelemetry for metrics, likely using it for tracing too
+                    cfg.interceptor_state()
+                        .store_append(AwsSdkFeature::ObservabilityOtelTracing);


Same with this one, and this could be especially misleading since users can hook their tracing crate traces up to OTel.

landonxjames · 2025-11-13T05:13:19Z

aws/rust-runtime/aws-runtime/src/observability_detection.rs

+    }
+
+    // Note: We cannot easily test non-noop providers without creating a custom meter provider
+    // implementation, which would require exposing internal types. The noop test above


I don't think this would require leaking any internal types? The OTel meter provider already exists so anything require to implement another one should already be pub. If something is missing we probably need to make it pub anyway so users can implement their own providers.

landonxjames · 2025-11-13T05:30:33Z

rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs

    fn resolve_endpoint<'a>(&'a self, _params: &'a EndpointResolverParams) -> EndpointFuture<'a> {
        EndpointFuture::ready(Ok(Endpoint::builder()
            .url(self.endpoint.to_string())
+            .property("is_custom_endpoint", true)


This property name doesn't seem to be referenced anywhere else in this PR?

landonxjames · 2025-11-13T05:30:52Z

gradle.properties

 smithy.rs.runtime.crate.stable.version=1.1.7
 # Version number to use for the generated unstable runtime crates
 smithy.rs.runtime.crate.unstable.version=0.60.6
+# Version number for aws-smithy-observability crate


This seems unnecessary. How is it being used?

landonxjames · 2025-11-13T05:33:52Z

aws/rust-runtime/aws-runtime/src/observability_detection.rs

+        // Try to get the global telemetry provider
+        if let Ok(provider) = aws_smithy_observability::global::get_telemetry_provider() {
+            // Check if it's not a noop provider
+            if !provider.is_noop() {


This isn't quite enough information to know that the provider is our OTel impl. This just says it isn't the noop impl. But a user could implement the traits their self and provide a different implementation (say for the metrics crate), or we could provide another non-OTel impl in the future that would break this assumption.

I think the better approach here would be to use the as_any method you added to the ProvideMeter trait. Something along the lines of provider.as_any().downcast_ref::<OtelMeterProvider>() which returns an Option<&OtelMeterProvider> and then only emit the metric if it is Some(_). That would allow us to get rid of the is_otel and is_noop stuff and wouldn't break if users were using some third implementation.

landonxjames · 2025-11-13T05:59:27Z

aws/rust-runtime/aws-runtime/src/observability_plugin.rs

The runtime plugin and the interceptor could probably just go in the same file to keep things together.

landonxjames · 2025-11-13T06:02:29Z

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

        }

+        // Also load AWS SDK features from interceptor_state (where interceptors store them)
+        let aws_sdk_features_from_interceptor = cfg.interceptor_state().load::<AwsSdkFeature>();


I think the cfg.load::<AwsSdkFeature>() should capture all of the AwsSdkFeature in the cfg bag so it shouldn't be necessary to grab them from this single layer? I might be wrong about though, will have to check in the morning, but @ysaito1001 probably knows.

Correct, it's covered by the code above from line 144 to line 149. We can remove this newly added lines of code.

ysaito1001

For the ENDPOINT_OVERRIDE feature, I'm struggling to understand the code change in this PR. That feature is tied to this service-specific endpoint URL implementation. This is how service specific endpoint is usually handled (example), but I don't see how EndpointOverrideDecorator tracks the feature accordingly?

…tion, and remove redundant code

github-actions · 2025-11-14T22:31:44Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2025-11-16T11:02:37Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

fix(observability): Replace Any supertrait with as_any() method

21a98a3

vcjana force-pushed the feature-ids-implementation branch from c3c5ac1 to 21a98a3 Compare November 12, 2025 06:58

Add default implementation for as_any()

81ee539

vcjana force-pushed the feature-ids-implementation branch from 1eb10a8 to 81ee539 Compare November 12, 2025 08:32

Merge branch 'main' into feature-ids-implementation

08203b8

smithy-lang deleted a comment from github-actions bot Nov 12, 2025

landonxjames requested changes Nov 13, 2025

View reviewed changes

ysaito1001 reviewed Nov 13, 2025

View reviewed changes

vcjana added 2 commits November 14, 2025 14:09

Address code review feedback: improve tests, use type-safe OTel detec…

903f043

…tion, and remove redundant code

Merge branch 'main' into feature-ids-implementation

41c3578

Fix clippy uninlined_format_args warning

204d064

Add business metrics for observability and endpoint override #4385

Are you sure you want to change the base?

Add business metrics for observability and endpoint override #4385

Conversation

vcjana commented Nov 6, 2025

Motivation and Context

Description

Testing

Checklist

Uh oh!

github-actions bot commented Nov 11, 2025

Uh oh!

github-actions bot commented Nov 12, 2025

Uh oh!

github-actions bot commented Nov 12, 2025

Uh oh!

landonxjames left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ysaito1001 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 14, 2025

Uh oh!

github-actions bot commented Nov 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants