Add aws-smithy-observability and aws-smithy-observability-otel crates
#3986
Conversation
Also fix all warnings in both observability crates
The updates caused CI to fail since it depended on the newly added crates, but those were not yet present in the main lockfile that is copied around for tests
* Update some clippy style issues * Config the aws-smithy-observability-otel crate to not compile on powerpc * Run tests for global module serially so they don't poison the RwLock
Pin version of value-bag in otel crate Powerpc dep updates Pin more transitive deps
Co-authored-by: Zelda Hessler <[email protected]>
Aligns with standard introduced in #3065 Update doc comments External types updates
Remove non-public type from docs
Update doc comments
|
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. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| // TODO(smithyobservability): once we have finalized everything and integrated metrics with our runtime | ||
| // libraries update this with detailed usage docs and examples | ||
|
|
||
| pub mod attributes; |
There was a problem hiding this comment.
if you have a pub use for the things inside do we need the module itself public?
| pub fn set_telemetry_provider<PM: ProvideMeter + 'static>( | ||
| new_provider: TelemetryProvider<PM>, | ||
| ) -> Result<(), ObservabilityError> { | ||
| if let Ok(mut old_provider) = GLOBAL_TELEMETRY_PROVIDER.try_write() { |
There was a problem hiding this comment.
question: Why try_write? Why not just lock and block the thread?
|
|
||
| /// Delineates a logical scope that has some beginning and end | ||
| /// (e.g. a function or block of code). | ||
| pub trait Scope: Send + Sync + Debug { |
There was a problem hiding this comment.
probably move these traits (context, scope, context manager) either into lib.rs directly or a context module.
| } | ||
|
|
||
| /// Get an owned [Iterator] of ([String], [AttributeValue]). | ||
| pub fn into_attributes(self) -> impl Iterator<Item = (String, AttributeValue)> { |
There was a problem hiding this comment.
implementing into_iter would probably be more idiomatic but I question why we need this?
There was a problem hiding this comment.
It was added to make call sites more ergonomic, otherwise they would have to clone String unnecessarily even though they have an owned Attributes passed to them (discussion)
|
|
||
| /// Get an [Arc] reference to the current global [TelemetryProvider]. Will return an [Err] if the | ||
| /// [RwLock] holding the global [TelemetryProvider] is locked or poisoned. | ||
| pub fn get_telemetry_provider<PM: ProvideMeter + 'static>( |
There was a problem hiding this comment.
question/correctness: How are we going to instrument the runtime if we have to know the concrete type of PM here? The runtime can't possibly know the type so I'm not sure how instrumentation is going to work with this.
| type Context = ContextWrap; | ||
| type Gauge = GaugeWrap; | ||
| type UpDownCounter = UpDownCounterWrap; | ||
| type AsyncUDC = AsyncUpDownCounterWrap; | ||
| type MonotonicCounter = MonotonicCounterWrap; | ||
| type AsyncMC = AsyncMonotonicCounterWrap; | ||
| type Histogram = HistogramWrap; | ||
| type GaugeCallback<'a> = fn(&Self::GaugeCallbackInput<'_>); | ||
| type GaugeCallbackInput<'a> = AsyncInstrumentWrap<'a, f64>; | ||
| type AsyncUDCCallback<'a> = fn(&Self::AsyncUDCCallbackInput<'_>); | ||
| type AsyncUDCCallbackInput<'a> = AsyncInstrumentWrap<'a, i64>; | ||
| type AsyncMCCallback<'a> = fn(&Self::AsyncMCCallbackInput<'_>); | ||
| type AsyncMCCallbackInput<'a> = AsyncInstrumentWrap<'a, u64>; |
There was a problem hiding this comment.
These are a quite a few of associated types 🤔
There was a problem hiding this comment.
Other than pending comments, given I approved #3912, this looks good. We probably just need to settle on either dyn trait or associated types.
|
Overtaken by #4003 |
Motivation and Context
This PR is an updated version of #3912 featuring an Associated Type based implementation of of the SRA metrics interfaces instead of the previous
dyn Traitbased implementations.Description
Add two new crates
aws-smithy-observability- contains traits for our observability solution and aglobalmodule for managing the global telemetry provideraws-smithy-observability-otel- contains an OpenTelemetry based implementation of the traitsTesting
Various unit tests in both new crates.
Checklist
Note: leaving out a changelog entry for now since these crates won't offer much value until we instrument our runtime crates
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.