Skip to content
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

Update KeyValue to avoid copying non-static string slices #2089

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions opentelemetry-otlp/examples/basic-otlp-http/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use opentelemetry::{
global,
metrics::MetricsError,
trace::{TraceContextExt, TraceError, Tracer, TracerProvider as _},
Key, KeyValue,
KeyValue,
};
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
use opentelemetry_otlp::Protocol;
Expand Down Expand Up @@ -146,7 +146,7 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
let span = cx.span();
span.add_event(
"Nice operation!".to_string(),
vec![Key::new("bogons").i64(100)],
vec![KeyValue::new("bogons", 100)],
);
span.set_attribute(KeyValue::new("another.key", "yes"));

Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-otlp/examples/basic-otlp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use opentelemetry::metrics::MetricsError;
use opentelemetry::trace::{TraceError, TracerProvider};
use opentelemetry::{
trace::{TraceContextExt, Tracer},
Key, KeyValue,
KeyValue,
};
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
use opentelemetry_otlp::{ExportConfig, WithExportConfig};
Expand Down Expand Up @@ -136,7 +136,7 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
let span = cx.span();
span.add_event(
"Nice operation!".to_string(),
vec![Key::new("bogons").i64(100)],
vec![KeyValue::new("bogons", 100)],
);
span.set_attribute(KeyValue::new("another.key", "yes"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub async fn traces() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
let span = cx.span();
span.add_event(
"Nice operation!".to_string(),
vec![Key::new("bogons").i64(100)],
vec![KeyValue::new("bogons", 100)],
);
span.set_attribute(KeyValue::new(ANOTHER_KEY, "yes"));

Expand Down
22 changes: 11 additions & 11 deletions opentelemetry-proto/src/transform/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@
#[derive(Default, Debug)]
pub struct Attributes(pub ::std::vec::Vec<crate::proto::tonic::common::v1::KeyValue>);

impl From<Vec<opentelemetry::KeyValue>> for Attributes {
fn from(kvs: Vec<opentelemetry::KeyValue>) -> Self {
impl From<Vec<opentelemetry::KeyValue<'static>>> for Attributes {
fn from(kvs: Vec<opentelemetry::KeyValue<'static>>) -> Self {
Attributes(
kvs.into_iter()
.map(|api_kv| KeyValue {
Expand All @@ -139,19 +139,19 @@
}
}

impl From<Value> for AnyValue {
impl From<Value<'static>> for AnyValue {
fn from(value: Value) -> Self {
AnyValue {
value: match value {
Value::Bool(val) => Some(any_value::Value::BoolValue(val)),
Value::I64(val) => Some(any_value::Value::IntValue(val)),
Value::F64(val) => Some(any_value::Value::DoubleValue(val)),
Value::Bool(val) => Some(any_value::Value::BoolValue(*val)),
Value::I64(val) => Some(any_value::Value::IntValue(*val)),
Value::F64(val) => Some(any_value::Value::DoubleValue(*val)),

Check warning on line 148 in opentelemetry-proto/src/transform/common.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/common.rs#L146-L148

Added lines #L146 - L148 were not covered by tests
Value::String(val) => Some(any_value::Value::StringValue(val.to_string())),
Value::Array(array) => Some(any_value::Value::ArrayValue(match array {
Array::Bool(vals) => array_into_proto(vals),
Array::I64(vals) => array_into_proto(vals),
Array::F64(vals) => array_into_proto(vals),
Array::String(vals) => array_into_proto(vals),
Array::Bool(vals) => array_into_proto(vals.to_vec()),
Array::I64(vals) => array_into_proto(vals.to_vec()),
Array::F64(vals) => array_into_proto(vals.to_vec()),
Array::String(vals) => array_into_proto(vals.to_vec()),

Check warning on line 154 in opentelemetry-proto/src/transform/common.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/common.rs#L151-L154

Added lines #L151 - L154 were not covered by tests
})),
},
}
Expand All @@ -160,7 +160,7 @@

fn array_into_proto<T>(vals: Vec<T>) -> ArrayValue
where
Value: From<T>,
Value<'static>: From<T>,

Check warning on line 163 in opentelemetry-proto/src/transform/common.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/common.rs#L163

Added line #L163 was not covered by tests
{
let values = vals
.into_iter()
Expand Down
8 changes: 4 additions & 4 deletions opentelemetry-proto/src/transform/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,17 @@
}
}

impl From<(&Key, &Value)> for KeyValue {
fn from(kv: (&Key, &Value)) -> Self {
impl From<(&Key, &Value<'static>)> for KeyValue {
fn from(kv: (&Key, &Value<'static>)) -> Self {

Check warning on line 77 in opentelemetry-proto/src/transform/metrics.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/metrics.rs#L77

Added line #L77 was not covered by tests
KeyValue {
key: kv.0.to_string(),
value: Some(kv.1.clone().into()),
}
}
}

impl From<&opentelemetry::KeyValue> for KeyValue {
fn from(kv: &opentelemetry::KeyValue) -> Self {
impl From<&opentelemetry::KeyValue<'static>> for KeyValue {
fn from(kv: &opentelemetry::KeyValue<'static>) -> Self {

Check warning on line 86 in opentelemetry-proto/src/transform/metrics.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/metrics.rs#L86

Added line #L86 was not covered by tests
KeyValue {
key: kv.key.to_string(),
value: Some(kv.value.clone().into()),
Expand Down
44 changes: 22 additions & 22 deletions opentelemetry-sdk/benches/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use criterion::{criterion_group, criterion_main, Criterion};
use futures_util::future::BoxFuture;
use opentelemetry::{
trace::{Span, Tracer, TracerProvider},
Key,
KeyValue,
};
use opentelemetry_sdk::{
export::trace::{ExportResult, SpanData, SpanExporter},
Expand All @@ -16,42 +16,42 @@ fn criterion_benchmark(c: &mut Criterion) {

trace_benchmark_group(c, "start-end-span-4-attrs", |tracer| {
let mut span = tracer.start("foo");
span.set_attribute(Key::new("key1").bool(false));
span.set_attribute(Key::new("key2").string("hello"));
span.set_attribute(Key::new("key4").f64(123.456));
span.set_attribute(KeyValue::new("key1", false));
span.set_attribute(KeyValue::new("key2", "hello"));
span.set_attribute(KeyValue::new("key4", 123.456));
span.end();
});

trace_benchmark_group(c, "start-end-span-8-attrs", |tracer| {
let mut span = tracer.start("foo");
span.set_attribute(Key::new("key1").bool(false));
span.set_attribute(Key::new("key2").string("hello"));
span.set_attribute(Key::new("key4").f64(123.456));
span.set_attribute(Key::new("key11").bool(false));
span.set_attribute(Key::new("key12").string("hello"));
span.set_attribute(Key::new("key14").f64(123.456));
span.set_attribute(KeyValue::new("key1", false));
span.set_attribute(KeyValue::new("key2", "hello"));
span.set_attribute(KeyValue::new("key4", 123.456));
span.set_attribute(KeyValue::new("key11", false));
span.set_attribute(KeyValue::new("key12", "hello"));
span.set_attribute(KeyValue::new("key14", 123.456));
span.end();
});

trace_benchmark_group(c, "start-end-span-all-attr-types", |tracer| {
let mut span = tracer.start("foo");
span.set_attribute(Key::new("key1").bool(false));
span.set_attribute(Key::new("key2").string("hello"));
span.set_attribute(Key::new("key3").i64(123));
span.set_attribute(Key::new("key5").f64(123.456));
span.set_attribute(KeyValue::new("key1", false));
span.set_attribute(KeyValue::new("key2", "hello"));
span.set_attribute(KeyValue::new("key3", 123));
span.set_attribute(KeyValue::new("key5", 123.456));
span.end();
});

trace_benchmark_group(c, "start-end-span-all-attr-types-2x", |tracer| {
let mut span = tracer.start("foo");
span.set_attribute(Key::new("key1").bool(false));
span.set_attribute(Key::new("key2").string("hello"));
span.set_attribute(Key::new("key3").i64(123));
span.set_attribute(Key::new("key5").f64(123.456));
span.set_attribute(Key::new("key11").bool(false));
span.set_attribute(Key::new("key12").string("hello"));
span.set_attribute(Key::new("key13").i64(123));
span.set_attribute(Key::new("key15").f64(123.456));
span.set_attribute(KeyValue::new("key1", false));
span.set_attribute(KeyValue::new("key2", "hello"));
span.set_attribute(KeyValue::new("key3", 123));
span.set_attribute(KeyValue::new("key5", 123.456));
span.set_attribute(KeyValue::new("key11", false));
span.set_attribute(KeyValue::new("key12", "hello"));
span.set_attribute(KeyValue::new("key13", 123));
span.set_attribute(KeyValue::new("key15", 123.456));
span.end();
});
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/export/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub struct SpanData {
/// Span end time
pub end_time: SystemTime,
/// Span attributes
pub attributes: Vec<KeyValue>,
pub attributes: Vec<KeyValue<'static>>,
/// The number of attributes that were above the configured limit, and thus
/// dropped.
pub dropped_attributes_count: u32,
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/logs/log_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl opentelemetry::logs::LoggerProvider for LoggerProvider {
name: impl Into<Cow<'static, str>>,
version: Option<Cow<'static, str>>,
schema_url: Option<Cow<'static, str>>,
attributes: Option<Vec<opentelemetry::KeyValue>>,
attributes: Option<Vec<opentelemetry::KeyValue<'static>>>,
) -> Logger {
let name = name.into();

Expand Down
8 changes: 4 additions & 4 deletions opentelemetry-sdk/src/metrics/data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for Sum<T> {
pub struct DataPoint<T> {
/// Attributes is the set of key value pairs that uniquely identify the
/// time series.
pub attributes: Vec<KeyValue>,
pub attributes: Vec<KeyValue<'static>>,
/// The time when the time series was started.
pub start_time: Option<SystemTime>,
/// The time when the time series was recorded.
Expand Down Expand Up @@ -143,7 +143,7 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for Histogram<T> {
#[derive(Debug)]
pub struct HistogramDataPoint<T> {
/// The set of key value pairs that uniquely identify the time series.
pub attributes: Vec<KeyValue>,
pub attributes: Vec<KeyValue<'static>>,
/// The time when the time series was started.
pub start_time: SystemTime,
/// The time when the time series was recorded.
Expand Down Expand Up @@ -210,7 +210,7 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for ExponentialHistogram
#[derive(Debug)]
pub struct ExponentialHistogramDataPoint<T> {
/// The set of key value pairs that uniquely identify the time series.
pub attributes: Vec<KeyValue>,
pub attributes: Vec<KeyValue<'static>>,
/// When the time series was started.
pub start_time: SystemTime,
/// The time when the time series was recorded.
Expand Down Expand Up @@ -273,7 +273,7 @@ pub struct ExponentialBucket {
pub struct Exemplar<T> {
/// The attributes recorded with the measurement but filtered out of the
/// time series' aggregated data.
pub filtered_attributes: Vec<KeyValue>,
pub filtered_attributes: Vec<KeyValue<'static>>,
/// The time when the measurement was recorded.
pub time: SystemTime,
/// The measured value.
Expand Down
10 changes: 5 additions & 5 deletions opentelemetry-sdk/src/metrics/instrument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,31 +253,31 @@ pub(crate) struct ResolvedMeasures<T> {
}

impl<T: Copy + 'static> SyncCounter<T> for ResolvedMeasures<T> {
fn add(&self, val: T, attrs: &[KeyValue]) {
fn add(&self, val: T, attrs: &[KeyValue<'_>]) {
for measure in &self.measures {
measure.call(val, attrs)
}
}
}

impl<T: Copy + 'static> SyncUpDownCounter<T> for ResolvedMeasures<T> {
fn add(&self, val: T, attrs: &[KeyValue]) {
fn add(&self, val: T, attrs: &[KeyValue<'_>]) {
for measure in &self.measures {
measure.call(val, attrs)
}
}
}

impl<T: Copy + 'static> SyncGauge<T> for ResolvedMeasures<T> {
fn record(&self, val: T, attrs: &[KeyValue]) {
fn record(&self, val: T, attrs: &[KeyValue<'_>]) {
for measure in &self.measures {
measure.call(val, attrs)
}
}
}

impl<T: Copy + 'static> SyncHistogram<T> for ResolvedMeasures<T> {
fn record(&self, val: T, attrs: &[KeyValue]) {
fn record(&self, val: T, attrs: &[KeyValue<'_>]) {
for measure in &self.measures {
measure.call(val, attrs)
}
Expand All @@ -296,7 +296,7 @@ impl<T> Observable<T> {
}

impl<T: Copy + Send + Sync + 'static> AsyncInstrument<T> for Observable<T> {
fn observe(&self, measurement: T, attrs: &[KeyValue]) {
fn observe(&self, measurement: T, attrs: &[KeyValue<'_>]) {
for measure in &self.measures {
measure.call(measurement, attrs)
}
Expand Down
22 changes: 11 additions & 11 deletions opentelemetry-sdk/src/metrics/internal/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ pub(crate) fn is_under_cardinality_limit(size: usize) -> bool {

/// Receives measurements to be aggregated.
pub(crate) trait Measure<T>: Send + Sync + 'static {
fn call(&self, measurement: T, attrs: &[KeyValue]);
fn call(&self, measurement: T, attrs: &[KeyValue<'_>]);
}

impl<F, T> Measure<T> for F
where
F: Fn(T, &[KeyValue]) + Send + Sync + 'static,
F: Fn(T, &[KeyValue<'_>]) + Send + Sync + 'static,
{
fn call(&self, measurement: T, attrs: &[KeyValue]) {
fn call(&self, measurement: T, attrs: &[KeyValue<'_>]) {
self(measurement, attrs)
}
}
Expand Down Expand Up @@ -69,7 +69,7 @@ pub(crate) struct AggregateBuilder<T> {
_marker: marker::PhantomData<T>,
}

type Filter = Arc<dyn Fn(&KeyValue) -> bool + Send + Sync>;
type Filter = Arc<dyn Fn(&KeyValue<'_>) -> bool + Send + Sync>;

impl<T: Number<T>> AggregateBuilder<T> {
pub(crate) fn new(temporality: Option<Temporality>, filter: Option<Filter>) -> Self {
Expand All @@ -83,9 +83,9 @@ impl<T: Number<T>> AggregateBuilder<T> {
/// Wraps the passed in measure with an attribute filtering function.
fn filter(&self, f: impl Measure<T>) -> impl Measure<T> {
let filter = self.filter.clone();
move |n, attrs: &[KeyValue]| {
move |n, attrs: &[KeyValue<'_>]| {
if let Some(filter) = &filter {
let filtered_attrs: Vec<KeyValue> =
let filtered_attrs: Vec<KeyValue<'_>> =
attrs.iter().filter(|kv| filter(kv)).cloned().collect();
f.call(n, &filtered_attrs);
} else {
Expand All @@ -101,7 +101,7 @@ impl<T: Number<T>> AggregateBuilder<T> {
let t = self.temporality;

(
self.filter(move |n, a: &[KeyValue]| lv_filter.measure(n, a)),
self.filter(move |n, a: &[KeyValue<'_>]| lv_filter.measure(n, a)),
move |dest: Option<&mut dyn Aggregation>| {
let g = dest.and_then(|d| d.as_mut().downcast_mut::<Gauge<T>>());
let mut new_agg = if g.is_none() {
Expand Down Expand Up @@ -135,7 +135,7 @@ impl<T: Number<T>> AggregateBuilder<T> {
let t = self.temporality;

(
self.filter(move |n, a: &[KeyValue]| s.measure(n, a)),
self.filter(move |n, a: &[KeyValue<'_>]| s.measure(n, a)),
move |dest: Option<&mut dyn Aggregation>| match t {
Some(Temporality::Delta) => agg_sum.delta(dest),
_ => agg_sum.cumulative(dest),
Expand All @@ -150,7 +150,7 @@ impl<T: Number<T>> AggregateBuilder<T> {
let t = self.temporality;

(
self.filter(move |n, a: &[KeyValue]| s.measure(n, a)),
self.filter(move |n, a: &[KeyValue<'_>]| s.measure(n, a)),
move |dest: Option<&mut dyn Aggregation>| match t {
Some(Temporality::Delta) => agg_sum.delta(dest),
_ => agg_sum.cumulative(dest),
Expand All @@ -170,7 +170,7 @@ impl<T: Number<T>> AggregateBuilder<T> {
let t = self.temporality;

(
self.filter(move |n, a: &[KeyValue]| h.measure(n, a)),
self.filter(move |n, a: &[KeyValue<'_>]| h.measure(n, a)),
move |dest: Option<&mut dyn Aggregation>| match t {
Some(Temporality::Delta) => agg_h.delta(dest),
_ => agg_h.cumulative(dest),
Expand All @@ -196,7 +196,7 @@ impl<T: Number<T>> AggregateBuilder<T> {
let t = self.temporality;

(
self.filter(move |n, a: &[KeyValue]| h.measure(n, a)),
self.filter(move |n, a: &[KeyValue<'_>]| h.measure(n, a)),
move |dest: Option<&mut dyn Aggregation>| match t {
Some(Temporality::Delta) => agg_h.delta(dest),
_ => agg_h.cumulative(dest),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ impl<T: Number<T>> ExpoHistogram<T> {
}
}

pub(crate) fn measure(&self, value: T, attrs: &[KeyValue]) {
pub(crate) fn measure(&self, value: T, attrs: &[KeyValue<'_>]) {
let f_value = value.into_float();
// Ignore NaN and infinity.
if f_value.is_infinite() || f_value.is_nan() {
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/metrics/internal/histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<T: Number<T>> Histogram<T> {
histogram
}

pub(crate) fn measure(&self, measurement: T, attrs: &[KeyValue]) {
pub(crate) fn measure(&self, measurement: T, attrs: &[KeyValue<'_>]) {
let f = measurement.into_float();

// This search will return an index in the range `[0, bounds.len()]`, where
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/metrics/internal/last_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl<T: Number<T>> LastValue<T> {
}
}

pub(crate) fn measure(&self, measurement: T, attrs: &[KeyValue]) {
pub(crate) fn measure(&self, measurement: T, attrs: &[KeyValue<'_>]) {
// The argument index is not applicable to LastValue.
self.value_map.measure(measurement, attrs, 0);
}
Expand Down
Loading
Loading