-
Notifications
You must be signed in to change notification settings - Fork 224
Track observability business metric (metric 7) #4420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
3325a0b to
30648d2
Compare
…any() implementation
f9f1c7e to
0c38ac5
Compare
0c38ac5 to
02e7b83
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This change belongs to PR #4420 (observability metrics) where it's actually needed for testing, not in the endpoint override metric PR.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
e48ccee to
869532b
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
26502b9 to
4bdc5a1
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
14ab3db to
433fbe0
Compare
433fbe0 to
1fcff09
Compare
…-implementation # Conflicts: # aws/rust-runtime/Cargo.lock # rust-runtime/Cargo.lock
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
landonxjames
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start, couple of questions and suggestions. The biggest thing is figuring out what build issues you were encountering didn't allow you to test against the actual aws-smithy-observability-otel crate.
|
|
||
| // Check the provider name to detect OpenTelemetry without importing the otel crate | ||
| // This avoids compilation issues with the opentelemetry dependency | ||
| if meter_provider.provider_name() == "otel" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ok, but this could be faked. I don't know that that is something we really need to defend against though.
If we used the as_any cast along with a downcast_ref we could guarantee that it was our implementation of Otel. What were the compilation issues we are avoiding with this strategy?
| } | ||
|
|
||
| // Mock OTel meter provider for testing | ||
| // This mimics the real OtelMeterProvider's type name without requiring the broken opentelemetry crate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is it broken? It should work, we have some testing around it. The only thing I would expect to break would be it doesn't work on some of our exotic arch tests (powerpc maybe?).
I would really like to have a test that goes against the real OTel provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially used a mock because I was concerned about potential build issues with the opentelemetry crate (I had seen some comments about powerpc compatibility issues in the Cargo.toml). I'll update the test to use the actual implementation instead of the mock.
aws/sdk/integration-tests/dynamodb/tests/observability-metrics.rs
Outdated
Show resolved
Hide resolved
aws/sdk/integration-tests/dynamodb/tests/observability-metrics.rs
Outdated
Show resolved
Hide resolved
aws/sdk/integration-tests/dynamodb/tests/observability-metrics.rs
Outdated
Show resolved
Hide resolved
| /// The default implementation returns a reference to the unit type, | ||
| /// which will fail any downcast attempts. Implementations should override | ||
| /// this to return `self` for proper type inspection. | ||
| fn as_any(&self) -> &dyn std::any::Any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new method doesn't seem to be used anywhere? I think it probably should be though so we can match on the actual type instead of on a name that could be set to anything.
If we do keep this it should either be a real implementation (return &self) or not implemented so implementers of the trait have to implement it themselves. But returning an incorrect type () is not the behavior we want.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
9ed940d to
f8aa750
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
Implements business metric tracking for observability providers per SEP User Agent 2.1 specification. The metric '7' (ObservabilityOtelMetrics) must be tracked when users configure OpenTelemetry metrics providers to help AWS understand observability usage patterns.
Description
This PR adds a new ObservabilityMetricDecorator that tracks the observability metric when OpenTelemetry metrics providers are configured through the global telemetry provider.
Implementation
The decorator registers an ObservabilityFeatureTrackerInterceptor that runs during request execution. It checks the global telemetry provider and uses type downcasting to detect if an OtelMeterProvider is configured:
aws_smithy_observability::global::get_telemetry_provider()downcast_ref::<OtelMeterProvider>()for reliable type detectionas_any()method toProvideMetertrait to enable downcastingWhen an OpenTelemetry metrics provider is configured, the metric is tracked in the User-Agent header.
Checklist
Note: This is a draft PR for review. The implementation successfully tracks OpenTelemetry metrics provider usage, which covers the SEP requirements for metric 7.