Skip to content

Commit 71cd2ef

Browse files
landonxjameslandonxjames
committed
PR feedback
1 parent d27317c commit 71cd2ef

File tree

9 files changed

+111
-91
lines changed

9 files changed

+111
-91
lines changed

‎rust-runtime/aws-smithy-observability-otel/src/attributes.rs

+16-22
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,16 @@ pub(crate) fn option_attr_from_kv(input: &[KeyValue]) -> Option<Attributes> {
4444
impl From<AttributesWrap> for Vec<KeyValue> {
4545
fn from(value: AttributesWrap) -> Self {
4646
value
47-
.attributes()
48-
.iter()
47+
.0
48+
.into_attributes()
4949
.map(|(k, v)| {
5050
KeyValue::new(
51-
k.clone(),
51+
k,
5252
match v {
53-
AttributeValue::I64(val) => Value::I64(*val),
54-
AttributeValue::F64(val) => Value::F64(*val),
55-
AttributeValue::String(val) => Value::String(val.clone().into()),
56-
AttributeValue::Bool(val) => Value::Bool(*val),
53+
AttributeValue::I64(val) => Value::I64(val),
54+
AttributeValue::F64(val) => Value::F64(val),
55+
AttributeValue::String(val) => Value::String(val.into()),
56+
AttributeValue::Bool(val) => Value::Bool(val),
5757
_ => Value::String("UNSUPPORTED ATTRIBUTE VALUE TYPE".into()),
5858
},
5959
)
@@ -68,7 +68,7 @@ impl From<&[KeyValue]> for AttributesWrap {
6868

6969
value.iter().for_each(|kv| {
7070
attrs.set(
71-
kv.key.clone().into(),
71+
kv.key.clone(),
7272
match &kv.value {
7373
Value::Bool(val) => AttributeValue::Bool(*val),
7474
Value::I64(val) => AttributeValue::I64(*val),
@@ -96,13 +96,10 @@ mod tests {
9696
#[test]
9797
fn attr_to_kv() {
9898
let mut attrs = Attributes::new();
99-
attrs.set("I64".into(), AttributeValue::I64(64));
100-
attrs.set("F64".into(), AttributeValue::F64(64.0));
101-
attrs.set(
102-
"String".into(),
103-
AttributeValue::String("I AM A STRING".into()),
104-
);
105-
attrs.set("Bool".into(), AttributeValue::Bool(true));
99+
attrs.set("I64", AttributeValue::I64(64));
100+
attrs.set("F64", AttributeValue::F64(64.0));
101+
attrs.set("String", AttributeValue::String("I AM A STRING".into()));
102+
attrs.set("Bool", AttributeValue::Bool(true));
106103

107104
let kv = kv_from_option_attr(Some(&attrs));
108105

@@ -130,15 +127,12 @@ mod tests {
130127
];
131128

132129
let attrs = option_attr_from_kv(&kvs).unwrap();
130+
assert_eq!(attrs.get("Bool").unwrap(), &AttributeValue::Bool(true));
133131
assert_eq!(
134-
attrs.get("Bool".into()).unwrap(),
135-
&AttributeValue::Bool(true)
136-
);
137-
assert_eq!(
138-
attrs.get("String".into()).unwrap(),
132+
attrs.get("String").unwrap(),
139133
&AttributeValue::String("I AM A STRING".into())
140134
);
141-
assert_eq!(attrs.get("I64".into()).unwrap(), &AttributeValue::I64(64));
142-
assert_eq!(attrs.get("F64".into()).unwrap(), &AttributeValue::F64(64.0));
135+
assert_eq!(attrs.get("I64").unwrap(), &AttributeValue::I64(64));
136+
assert_eq!(attrs.get("F64").unwrap(), &AttributeValue::F64(64.0));
143137
}
144138
}

‎rust-runtime/aws-smithy-observability-otel/src/lib.rs

+17-11
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@ pub mod meter;
2626
mod tests {
2727

2828
use crate::meter::AwsSdkOtelMeterProvider;
29-
use aws_smithy_observability::global::{
30-
get_global_telemetry_provider, set_global_telemetry_provider,
31-
};
29+
use aws_smithy_observability::global::{get_telemetry_provider, set_telemetry_provider};
3230
use aws_smithy_observability::provider::TelemetryProvider;
3331
use opentelemetry_sdk::metrics::{data::Sum, PeriodicReader, SdkMeterProvider};
3432
use opentelemetry_sdk::runtime::Tokio;
@@ -44,13 +42,11 @@ mod tests {
4442

4543
// Create the SDK metrics types from the OTel objects
4644
let sdk_mp = AwsSdkOtelMeterProvider::new(otel_mp);
47-
let sdk_tp = TelemetryProvider::builder()
48-
.meter_provider(Box::new(sdk_mp))
49-
.build();
45+
let sdk_tp = TelemetryProvider::builder().meter_provider(sdk_mp).build();
5046

5147
// Set the global TelemetryProvider and then get it back out
52-
let _ = set_global_telemetry_provider(Some(sdk_tp));
53-
let global_tp = get_global_telemetry_provider();
48+
let _ = set_telemetry_provider(sdk_tp);
49+
let global_tp = get_telemetry_provider();
5450

5551
// Create an instrument and record a value
5652
let global_meter = global_tp
@@ -62,7 +58,13 @@ mod tests {
6258
mono_counter.add(4, None, None);
6359

6460
// Flush metric pipeline and extract metrics from exporter
65-
global_tp.meter_provider().flush().unwrap();
61+
global_tp
62+
.meter_provider()
63+
.as_any()
64+
.downcast_ref::<AwsSdkOtelMeterProvider>()
65+
.unwrap()
66+
.shutdown()
67+
.unwrap();
6668
let finished_metrics = exporter.get_finished_metrics().unwrap();
6769

6870
let extracted_mono_counter_data = &finished_metrics[0].scope_metrics[0].metrics[0]
@@ -75,7 +77,11 @@ mod tests {
7577
assert_eq!(extracted_mono_counter_data, &4);
7678

7779
// Get the OTel TP out and shut it down
78-
let otel_tp = set_global_telemetry_provider(None);
79-
otel_tp.meter_provider().shutdown().unwrap();
80+
let foo = global_tp
81+
.meter_provider()
82+
.as_any()
83+
.downcast_ref::<AwsSdkOtelMeterProvider>()
84+
.unwrap();
85+
foo.shutdown().unwrap();
8086
}
8187
}

‎rust-runtime/aws-smithy-observability-otel/src/meter.rs

+31-20
Original file line numberDiff line numberDiff line change
@@ -266,28 +266,34 @@ impl AwsSdkOtelMeterProvider {
266266
meter_provider: otel_meter_provider,
267267
}
268268
}
269-
}
270269

271-
impl MeterProvider for AwsSdkOtelMeterProvider {
272-
fn get_meter(&self, scope: &'static str, _attributes: Option<&Attributes>) -> Box<dyn Meter> {
273-
Box::new(MeterWrap(self.meter_provider.meter(scope)))
274-
}
275-
276-
fn flush(&self) -> Result<(), ObservabilityError> {
270+
/// Flush the metric pipeline.
271+
pub fn flush(&self) -> Result<(), ObservabilityError> {
277272
match self.meter_provider.force_flush() {
278273
Ok(_) => Ok(()),
279274
Err(err) => Err(ObservabilityError::new(ErrorKind::MetricsFlush, err)),
280275
}
281276
}
282277

283-
fn shutdown(&self) -> Result<(), ObservabilityError> {
278+
/// Gracefully shutdown the metric pipeline.
279+
pub fn shutdown(&self) -> Result<(), ObservabilityError> {
284280
match self.meter_provider.force_flush() {
285281
Ok(_) => Ok(()),
286282
Err(err) => Err(ObservabilityError::new(ErrorKind::MetricsShutdown, err)),
287283
}
288284
}
289285
}
290286

287+
impl MeterProvider for AwsSdkOtelMeterProvider {
288+
fn get_meter(&self, scope: &'static str, _attributes: Option<&Attributes>) -> Box<dyn Meter> {
289+
Box::new(MeterWrap(self.meter_provider.meter(scope)))
290+
}
291+
292+
fn as_any(&self) -> &dyn std::any::Any {
293+
self
294+
}
295+
}
296+
291297
#[cfg(test)]
292298
mod tests {
293299

@@ -313,9 +319,7 @@ mod tests {
313319

314320
// Create the SDK metrics types from the OTel objects
315321
let sdk_mp = AwsSdkOtelMeterProvider::new(otel_mp);
316-
let sdk_tp = TelemetryProvider::builder()
317-
.meter_provider(Box::new(sdk_mp))
318-
.build();
322+
let sdk_tp = TelemetryProvider::builder().meter_provider(sdk_mp).build();
319323

320324
// Get the dyn versions of the SDK metrics objects
321325
let dyn_sdk_mp = sdk_tp.meter_provider();
@@ -332,7 +336,12 @@ mod tests {
332336
histogram.record(1.234, None, None);
333337

334338
// Gracefully shutdown the metrics provider so all metrics are flushed through the pipeline
335-
dyn_sdk_mp.shutdown().unwrap();
339+
dyn_sdk_mp
340+
.as_any()
341+
.downcast_ref::<AwsSdkOtelMeterProvider>()
342+
.unwrap()
343+
.shutdown()
344+
.unwrap();
336345

337346
// Extract the metrics from the exporter and assert that they are what we expect
338347
let finished_metrics = exporter.get_finished_metrics().unwrap();
@@ -373,9 +382,7 @@ mod tests {
373382

374383
// Create the SDK metrics types from the OTel objects
375384
let sdk_mp = AwsSdkOtelMeterProvider::new(otel_mp);
376-
let sdk_tp = TelemetryProvider::builder()
377-
.meter_provider(Box::new(sdk_mp))
378-
.build();
385+
let sdk_tp = TelemetryProvider::builder().meter_provider(sdk_mp).build();
379386

380387
// Get the dyn versions of the SDK metrics objects
381388
let dyn_sdk_mp = sdk_tp.meter_provider();
@@ -388,7 +395,7 @@ mod tests {
388395
Box::new(|measurement: &dyn AsyncMeasurement<Value = f64>| {
389396
let mut attrs = Attributes::new();
390397
attrs.set(
391-
"TestGaugeAttr".into(),
398+
"TestGaugeAttr",
392399
AttributeValue::String("TestGaugeAttr".into()),
393400
);
394401
measurement.record(6.789, Some(&attrs), None);
@@ -403,7 +410,7 @@ mod tests {
403410
Box::new(|measurement: &dyn AsyncMeasurement<Value = i64>| {
404411
let mut attrs = Attributes::new();
405412
attrs.set(
406-
"TestAsyncUpDownCounterAttr".into(),
413+
"TestAsyncUpDownCounterAttr",
407414
AttributeValue::String("TestAsyncUpDownCounterAttr".into()),
408415
);
409416
measurement.record(12, Some(&attrs), None);
@@ -418,7 +425,7 @@ mod tests {
418425
Box::new(|measurement: &dyn AsyncMeasurement<Value = u64>| {
419426
let mut attrs = Attributes::new();
420427
attrs.set(
421-
"TestAsyncMonoCounterAttr".into(),
428+
"TestAsyncMonoCounterAttr",
422429
AttributeValue::String("TestAsyncMonoCounterAttr".into()),
423430
);
424431
measurement.record(123, Some(&attrs), None);
@@ -429,8 +436,12 @@ mod tests {
429436
async_mono_counter.record(4, None, None);
430437

431438
// Gracefully shutdown the metrics provider so all metrics are flushed through the pipeline
432-
dyn_sdk_mp.flush().unwrap();
433-
dyn_sdk_mp.shutdown().unwrap();
439+
dyn_sdk_mp
440+
.as_any()
441+
.downcast_ref::<AwsSdkOtelMeterProvider>()
442+
.unwrap()
443+
.shutdown()
444+
.unwrap();
434445

435446
// Extract the metrics from the exporter
436447
let finished_metrics = exporter.get_finished_metrics().unwrap();

‎rust-runtime/aws-smithy-observability/src/attributes.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,24 @@ impl Attributes {
3939
}
4040

4141
/// Set an attribute.
42-
pub fn set(&mut self, key: String, value: AttributeValue) {
43-
self.attrs.insert(key, value);
42+
pub fn set(&mut self, key: impl Into<String>, value: impl Into<AttributeValue>) {
43+
self.attrs.insert(key.into(), value.into());
4444
}
4545

4646
/// Get an attribute.
47-
pub fn get(&self, key: String) -> Option<&AttributeValue> {
48-
self.attrs.get(&key)
47+
pub fn get(&self, key: impl Into<String>) -> Option<&AttributeValue> {
48+
self.attrs.get(&key.into())
4949
}
5050

5151
/// Get all of the attribute key value pairs.
5252
pub fn attributes(&self) -> &HashMap<String, AttributeValue> {
5353
&self.attrs
5454
}
55+
56+
/// Get an owned [Iterator] of ([String], [AttributeValue]).
57+
pub fn into_attributes(self) -> impl Iterator<Item = (String, AttributeValue)> {
58+
self.attrs.into_iter()
59+
}
5560
}
5661

5762
impl Default for Attributes {

‎rust-runtime/aws-smithy-observability/src/global.rs

+12-17
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,26 @@ use crate::provider::{GlobalTelemetryProvider, TelemetryProvider};
1717
static GLOBAL_TELEMETRY_PROVIDER: Lazy<RwLock<GlobalTelemetryProvider>> =
1818
Lazy::new(|| RwLock::new(GlobalTelemetryProvider::new(TelemetryProvider::default())));
1919

20-
/// Set the current global [TelemetryProvider]. If [None] is supplied then a noop provider is set.
21-
/// The previous [TelemetryProvider] is returned in an [Arc] so appropriate cleanup can be done if necessary.
22-
pub fn set_global_telemetry_provider(
23-
new_provider: Option<TelemetryProvider>,
24-
) -> Arc<TelemetryProvider> {
20+
/// Set the current global [TelemetryProvider].
21+
///
22+
/// This is meant to be run once at the beginning of an application. It will panic if two threads
23+
/// attempt to call it at the same time.
24+
pub fn set_telemetry_provider(new_provider: TelemetryProvider) {
2525
// TODO(smithyObservability): would probably be nicer to return a Result here, but the Guard held by the error from
2626
// .try_write is not Send so I struggled to build an ObservabilityError from it
2727
let mut old_provider = GLOBAL_TELEMETRY_PROVIDER
2828
.try_write()
2929
.expect("GLOBAL_TELEMETRY_PROVIDER RwLock Poisoned");
3030

31-
let new_global_provider = if let Some(tp) = new_provider {
32-
GlobalTelemetryProvider::new(tp)
33-
} else {
34-
GlobalTelemetryProvider::new(TelemetryProvider::default())
35-
};
31+
let new_global_provider = GlobalTelemetryProvider::new(new_provider);
3632

37-
mem::replace(&mut *old_provider, new_global_provider).telemetry_provider
33+
let _ = mem::replace(&mut *old_provider, new_global_provider);
3834
}
3935

4036
/// Get an [Arc] reference to the current global [TelemetryProvider].
41-
pub fn get_global_telemetry_provider() -> Arc<TelemetryProvider> {
37+
///
38+
/// This can panic if called when another thread is calling [set_telemetry_provider].
39+
pub fn get_telemetry_provider() -> Arc<TelemetryProvider> {
4240
// TODO(smithyObservability): would probably be nicer to return a Result here, but the Guard held by the error from
4341
// .try_read is not Send so I struggled to build an ObservabilityError from it
4442
GLOBAL_TELEMETRY_PROVIDER
@@ -62,16 +60,13 @@ mod tests {
6260
let my_provider = TelemetryProvider::default();
6361

6462
// Set the new counter and get a reference to the old one
65-
let old_provider = set_global_telemetry_provider(Some(my_provider));
66-
67-
// Call shutdown on the old meter provider
68-
let _old_meter = old_provider.meter_provider().shutdown().unwrap();
63+
set_telemetry_provider(my_provider);
6964
}
7065

7166
#[test]
7267
#[serial]
7368
fn can_get_global_telemetry_provider() {
74-
let curr_provider = get_global_telemetry_provider();
69+
let curr_provider = get_telemetry_provider();
7570

7671
// Use the global provider to create an instrument and record a value with it
7772
let curr_mp = curr_provider.meter_provider();

‎rust-runtime/aws-smithy-observability/src/lib.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@
1414
)]
1515

1616
//! Smithy Observability
17-
//TODO(smithyobservability): once we have finalized everything and integrated metrics with our runtime
17+
// TODO(smithyobservability): once we have finalized everything and integrated metrics with our runtime
1818
// libraries update this with detailed usage docs and examples
1919

2020
pub mod attributes;
21+
pub use attributes::{AttributeValue, Attributes};
2122
pub mod error;
23+
pub use error::{ErrorKind, ObservabilityError};
2224
pub mod global;
2325
pub mod meter;
2426
mod noop;
2527
pub mod provider;
28+
pub use provider::{TelemetryProvider, TelemetryProviderBuilder};

‎rust-runtime/aws-smithy-observability/src/meter.rs

+2-10
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,14 @@
77
//! real time.
88
99
use crate::attributes::{Attributes, Context};
10-
use crate::error::ObservabilityError;
1110

1211
/// Provides named instances of [Meter].
1312
pub trait MeterProvider {
1413
/// Get or create a named [Meter].
1514
fn get_meter(&self, scope: &'static str, attributes: Option<&Attributes>) -> Box<dyn Meter>;
1615

17-
/// Optional method to flush the metrics pipeline, default is noop
18-
fn flush(&self) -> Result<(), ObservabilityError> {
19-
Ok(())
20-
}
21-
22-
/// Optional method to shutdown the metrics provider, default is noop
23-
fn shutdown(&self) -> Result<(), ObservabilityError> {
24-
Ok(())
25-
}
16+
/// Foo
17+
fn as_any(&self) -> &dyn std::any::Any;
2618
}
2719

2820
/// The entry point to creating instruments. A grouping of related metrics.

‎rust-runtime/aws-smithy-observability/src/noop.rs

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ impl MeterProvider for NoopMeterProvider {
1717
fn get_meter(&self, _scope: &'static str, _attributes: Option<&Attributes>) -> Box<dyn Meter> {
1818
Box::new(NoopMeter)
1919
}
20+
21+
fn as_any(&self) -> &dyn std::any::Any {
22+
self
23+
}
2024
}
2125

2226
pub(crate) struct NoopMeter;

0 commit comments

Comments
 (0)