diff --git a/opentelemetry-datadog/src/exporter/model/mod.rs b/opentelemetry-datadog/src/exporter/model/mod.rs index cecc080429..f0b626a3ec 100644 --- a/opentelemetry-datadog/src/exporter/model/mod.rs +++ b/opentelemetry-datadog/src/exporter/model/mod.rs @@ -195,7 +195,7 @@ pub(crate) mod tests { }; use opentelemetry_sdk::{ self, - trace::{EvictedQueue, SpanLinks}, + trace::{SpanEvents, SpanLinks}, InstrumentationLibrary, Resource, }; use std::borrow::Cow; @@ -217,9 +217,8 @@ pub(crate) mod tests { let start_time = SystemTime::UNIX_EPOCH; let end_time = start_time.checked_add(Duration::from_secs(1)).unwrap(); - let capacity = 3; let attributes = vec![KeyValue::new("span.type", "web")]; - let events = EvictedQueue::new(capacity); + let events = SpanEvents::default(); let links = SpanLinks::default(); let resource = Resource::new(vec![KeyValue::new("host.name", "test")]); diff --git a/opentelemetry-jaeger/src/exporter/mod.rs b/opentelemetry-jaeger/src/exporter/mod.rs index 82bc7089a7..9bc1d91d3c 100644 --- a/opentelemetry-jaeger/src/exporter/mod.rs +++ b/opentelemetry-jaeger/src/exporter/mod.rs @@ -19,13 +19,11 @@ use opentelemetry::{ trace::{Event, Link, SpanKind, Status}, InstrumentationLibrary, Key, KeyValue, }; -use opentelemetry_sdk::{ - export::{ - trace::{ExportResult, SpanData, SpanExporter}, - ExportError, - }, - trace::EvictedQueue, +use opentelemetry_sdk::export::{ + trace::{ExportResult, SpanData, SpanExporter}, + ExportError, }; +use opentelemetry_sdk::trace::SpanEvents; use crate::exporter::uploader::Uploader; @@ -255,7 +253,7 @@ impl UserOverrides { } } -fn events_to_logs(events: EvictedQueue) -> Option> { +fn events_to_logs(events: SpanEvents) -> Option> { if events.is_empty() { None } else { diff --git a/opentelemetry-proto/src/transform/trace.rs b/opentelemetry-proto/src/transform/trace.rs index 1a16b070e9..616c531c75 100644 --- a/opentelemetry-proto/src/transform/trace.rs +++ b/opentelemetry-proto/src/transform/trace.rs @@ -82,7 +82,7 @@ pub mod tonic { end_time_unix_nano: to_nanos(source_span.end_time), dropped_attributes_count: source_span.dropped_attributes_count, attributes: Attributes::from(source_span.attributes).0, - dropped_events_count: source_span.events.dropped_count(), + dropped_events_count: source_span.events.dropped_count, events: source_span .events .into_iter() @@ -193,7 +193,7 @@ pub mod grpcio { end_time_unix_nano: to_nanos(source_span.end_time), dropped_attributes_count: source_span.dropped_attributes_count, attributes: Attributes::from(source_span.attributes).0, - dropped_events_count: source_span.events.dropped_count(), + dropped_events_count: source_span.events.dropped_count, events: source_span .events .into_iter() diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 2a6d9efd68..919467a998 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -5,19 +5,23 @@ ### Changed - **Breaking** -[#1313](https://github.com/open-telemetry/opentelemetry-rust/issues/1313) - Changes how Span links are stored to achieve performance gains. See below for - details: - - *Behavior Change*: When enforcing `max_links_per_span` from `SpanLimits`, - links are kept in the first-come order. The previous "eviction" based approach - is no longer performed. +[#1313](https://github.com/open-telemetry/opentelemetry-rust/pull/1313) +[#1350](https://github.com/open-telemetry/opentelemetry-rust/pull/1350) + Changes how Span links/events are stored to achieve performance gains. See + below for details: + + *Behavior Change*: When enforcing `max_links_per_span`, `max_events_per_span` + from `SpanLimits`, links/events are kept in the first-come order. The previous + "eviction" based approach is no longer performed. *Breaking Change Affecting Exporter authors*: `SpanData` now stores `links` as `SpanLinks` instead of `EvictedQueue` where `SpanLinks` is a struct with a `Vec` of links and `dropped_count`. + `SpanData` now stores `events` as `SpanEvents` instead of `EvictedQueue` where + `SpanEvents` is a struct with a `Vec` of events and `dropped_count`. + ## v0.21.0 ### Added diff --git a/opentelemetry-sdk/benches/batch_span_processor.rs b/opentelemetry-sdk/benches/batch_span_processor.rs index e5bea6dcd6..4e2301e203 100644 --- a/opentelemetry-sdk/benches/batch_span_processor.rs +++ b/opentelemetry-sdk/benches/batch_span_processor.rs @@ -5,7 +5,7 @@ use opentelemetry::trace::{ use opentelemetry_sdk::export::trace::SpanData; use opentelemetry_sdk::runtime::Tokio; use opentelemetry_sdk::testing::trace::NoopSpanExporter; -use opentelemetry_sdk::trace::{BatchSpanProcessor, EvictedQueue, SpanLinks, SpanProcessor}; +use opentelemetry_sdk::trace::{BatchSpanProcessor, SpanEvents, SpanLinks, SpanProcessor}; use opentelemetry_sdk::Resource; use std::borrow::Cow; use std::sync::Arc; @@ -29,7 +29,7 @@ fn get_span_data() -> Vec { end_time: SystemTime::now(), attributes: Vec::new(), dropped_attributes_count: 0, - events: EvictedQueue::new(12), + events: SpanEvents::default(), links: SpanLinks::default(), status: Status::Unset, resource: Cow::Owned(Resource::empty()), diff --git a/opentelemetry-sdk/src/export/trace.rs b/opentelemetry-sdk/src/export/trace.rs index c75a0dd94a..b3d99c9a13 100644 --- a/opentelemetry-sdk/src/export/trace.rs +++ b/opentelemetry-sdk/src/export/trace.rs @@ -1,7 +1,7 @@ //! Trace exporters use crate::Resource; use futures_util::future::BoxFuture; -use opentelemetry::trace::{Event, SpanContext, SpanId, SpanKind, Status, TraceError}; +use opentelemetry::trace::{SpanContext, SpanId, SpanKind, Status, TraceError}; use opentelemetry::KeyValue; use std::borrow::Cow; use std::fmt::Debug; @@ -87,7 +87,7 @@ pub struct SpanData { /// dropped. pub dropped_attributes_count: u32, /// Span events - pub events: crate::trace::EvictedQueue, + pub events: crate::trace::SpanEvents, /// Span Links pub links: crate::trace::SpanLinks, /// Span status diff --git a/opentelemetry-sdk/src/testing/trace/mod.rs b/opentelemetry-sdk/src/testing/trace/mod.rs index 98b8ef3c86..7605101d86 100644 --- a/opentelemetry-sdk/src/testing/trace/mod.rs +++ b/opentelemetry-sdk/src/testing/trace/mod.rs @@ -7,7 +7,7 @@ use crate::{ trace::{ExportResult, SpanData, SpanExporter}, ExportError, }, - trace::{Config, EvictedQueue, SpanLinks}, + trace::{Config, SpanEvents, SpanLinks}, InstrumentationLibrary, }; use async_trait::async_trait; @@ -36,7 +36,7 @@ pub fn new_test_export_span_data() -> SpanData { end_time: opentelemetry::time::now(), attributes: Vec::new(), dropped_attributes_count: 0, - events: EvictedQueue::new(config.span_limits.max_events_per_span), + events: SpanEvents::default(), links: SpanLinks::default(), status: Status::Unset, resource: config.resource, diff --git a/opentelemetry-sdk/src/trace/events.rs b/opentelemetry-sdk/src/trace/events.rs new file mode 100644 index 0000000000..fbf2bcb928 --- /dev/null +++ b/opentelemetry-sdk/src/trace/events.rs @@ -0,0 +1,37 @@ +//! # Span Events + +use std::ops::Deref; + +use opentelemetry::trace::Event; +/// Stores span events along with dropped count. +#[derive(Clone, Debug, Default, PartialEq)] +#[non_exhaustive] +pub struct SpanEvents { + /// The events stored as a vector. Could be empty if there are no events. + pub events: Vec, + /// The number of Events dropped from the span. + pub dropped_count: u32, +} + +impl Deref for SpanEvents { + type Target = [Event]; + + fn deref(&self) -> &Self::Target { + &self.events + } +} + +impl IntoIterator for SpanEvents { + type Item = Event; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.events.into_iter() + } +} + +impl SpanEvents { + pub(crate) fn add_event(&mut self, event: Event) { + self.events.push(event); + } +} diff --git a/opentelemetry-sdk/src/trace/mod.rs b/opentelemetry-sdk/src/trace/mod.rs index e064e85e4a..d031cc177f 100644 --- a/opentelemetry-sdk/src/trace/mod.rs +++ b/opentelemetry-sdk/src/trace/mod.rs @@ -7,6 +7,7 @@ //! current operation execution. //! * The [`TracerProvider`] struct which configures and produces [`Tracer`]s. mod config; +mod events; mod evicted_hash_map; mod evicted_queue; mod id_generator; @@ -19,6 +20,7 @@ mod span_processor; mod tracer; pub use config::{config, Config}; +pub use events::SpanEvents; pub use evicted_hash_map::EvictedHashMap; pub use evicted_queue::EvictedQueue; pub use id_generator::{aws::XrayIdGenerator, IdGenerator, RandomIdGenerator}; @@ -42,11 +44,12 @@ mod runtime_tests; mod tests { use super::*; use crate::{ - testing::trace::InMemorySpanExporterBuilder, trace::span_limit::DEFAULT_MAX_LINKS_PER_SPAN, + testing::trace::InMemorySpanExporterBuilder, + trace::span_limit::{DEFAULT_MAX_EVENT_PER_SPAN, DEFAULT_MAX_LINKS_PER_SPAN}, }; use opentelemetry::{ trace::{ - Link, Span, SpanBuilder, SpanContext, SpanId, TraceFlags, TraceId, Tracer, + Event, Link, Span, SpanBuilder, SpanContext, SpanId, TraceFlags, TraceId, Tracer, TracerProvider as _, }, KeyValue, @@ -140,4 +143,41 @@ mod tests { assert_eq!(span.name, "span_name"); assert_eq!(span.links.len(), DEFAULT_MAX_LINKS_PER_SPAN as usize); } + + #[test] + fn exceed_span_events_limit() { + // Arrange + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = TracerProvider::builder() + .with_span_processor(SimpleSpanProcessor::new(Box::new(exporter.clone()))) + .build(); + + // Act + let tracer = provider.tracer("test_tracer"); + + let mut events = Vec::new(); + for _i in 0..(DEFAULT_MAX_EVENT_PER_SPAN * 2) { + events.push(Event::with_name("test event")) + } + + // add events via span builder + let span_builder = SpanBuilder::from_name("span_name").with_events(events); + let mut span = tracer.build(span_builder); + + // add events using span api after building the span + span.add_event("test event again, after span builder", Vec::new()); + span.add_event("test event once again, after span builder", Vec::new()); + span.end(); + provider.force_flush(); + + // Assert + let exported_spans = exporter + .get_finished_spans() + .expect("Spans are expected to be exported."); + assert_eq!(exported_spans.len(), 1); + let span = &exported_spans[0]; + assert_eq!(span.name, "span_name"); + assert_eq!(span.events.len(), DEFAULT_MAX_EVENT_PER_SPAN as usize); + assert_eq!(span.events.dropped_count, DEFAULT_MAX_EVENT_PER_SPAN + 2); + } } diff --git a/opentelemetry-sdk/src/trace/span.rs b/opentelemetry-sdk/src/trace/span.rs index 2839c4114e..0d58061b09 100644 --- a/opentelemetry-sdk/src/trace/span.rs +++ b/opentelemetry-sdk/src/trace/span.rs @@ -11,7 +11,7 @@ use crate::trace::SpanLimits; use crate::Resource; use opentelemetry::trace::{Event, SpanContext, SpanId, SpanKind, Status}; -use opentelemetry::{trace, KeyValue}; +use opentelemetry::KeyValue; use std::borrow::Cow; use std::time::SystemTime; @@ -42,7 +42,7 @@ pub(crate) struct SpanData { /// dropped. pub(crate) dropped_attributes_count: u32, /// Span events - pub(crate) events: crate::trace::EvictedQueue, + pub(crate) events: crate::trace::SpanEvents, /// Span Links pub(crate) links: crate::trace::SpanLinks, /// Span status @@ -99,17 +99,23 @@ impl opentelemetry::trace::Span for Span { ) where T: Into>, { + let span_events_limit = self.span_limits.max_events_per_span as usize; let event_attributes_limit = self.span_limits.max_attributes_per_event as usize; self.with_data(|data| { - let dropped_attributes_count = attributes.len().saturating_sub(event_attributes_limit); - attributes.truncate(event_attributes_limit); - - data.events.push_back(Event::new( - name, - timestamp, - attributes, - dropped_attributes_count as u32, - )) + if data.events.len() < span_events_limit { + let dropped_attributes_count = + attributes.len().saturating_sub(event_attributes_limit); + attributes.truncate(event_attributes_limit); + + data.events.add_event(Event::new( + name, + timestamp, + attributes, + dropped_attributes_count as u32, + )); + } else { + data.events.dropped_count += 1; + } }); } @@ -252,17 +258,16 @@ mod tests { use crate::testing::trace::NoopSpanExporter; use crate::trace::span_limit::{ DEFAULT_MAX_ATTRIBUTES_PER_EVENT, DEFAULT_MAX_ATTRIBUTES_PER_LINK, - DEFAULT_MAX_ATTRIBUTES_PER_SPAN, DEFAULT_MAX_LINKS_PER_SPAN, + DEFAULT_MAX_ATTRIBUTES_PER_SPAN, DEFAULT_MAX_EVENT_PER_SPAN, DEFAULT_MAX_LINKS_PER_SPAN, }; - use crate::trace::SpanLinks; - use opentelemetry::trace::{Link, SpanBuilder, TraceFlags, TraceId, Tracer}; + use crate::trace::{SpanEvents, SpanLinks}; + use opentelemetry::trace::{self, Link, SpanBuilder, TraceFlags, TraceId, Tracer}; use opentelemetry::{trace::Span as _, trace::TracerProvider, KeyValue}; use std::time::Duration; use std::vec; fn init() -> (crate::trace::Tracer, SpanData) { let provider = crate::trace::TracerProvider::default(); - let config = provider.config(); let tracer = provider.tracer("opentelemetry"); let data = SpanData { parent_span_id: SpanId::from_u64(0), @@ -272,7 +277,7 @@ mod tests { end_time: opentelemetry::time::now(), attributes: Vec::new(), dropped_attributes_count: 0, - events: crate::trace::EvictedQueue::new(config.span_limits.max_events_per_span), + events: SpanEvents::default(), links: SpanLinks::default(), status: Status::Unset, }; @@ -649,6 +654,35 @@ mod tests { assert_eq!(link_vec.len(), DEFAULT_MAX_LINKS_PER_SPAN as usize); } + #[test] + fn exceed_span_events_limit() { + let exporter = NoopSpanExporter::new(); + let provider_builder = + crate::trace::TracerProvider::builder().with_simple_exporter(exporter); + let provider = provider_builder.build(); + let tracer = provider.tracer("opentelemetry-test"); + + let mut events = Vec::new(); + for _i in 0..(DEFAULT_MAX_EVENT_PER_SPAN * 2) { + events.push(Event::with_name("test event")) + } + + // add events via span builder + let span_builder = tracer.span_builder("test").with_events(events); + let mut span = tracer.build(span_builder); + + // add events using span api after building the span + span.add_event("test event again, after span builder", Vec::new()); + span.add_event("test event once again, after span builder", Vec::new()); + let span_events = span + .data + .clone() + .expect("span data should not be empty as we already set it before") + .events; + let event_vec: Vec<_> = span_events.events; + assert_eq!(event_vec.len(), DEFAULT_MAX_EVENT_PER_SPAN as usize); + } + #[test] fn test_span_exported_data() { let provider = crate::trace::TracerProvider::builder() diff --git a/opentelemetry-sdk/src/trace/span_processor.rs b/opentelemetry-sdk/src/trace/span_processor.rs index 395cdd73be..0f6811e6e0 100644 --- a/opentelemetry-sdk/src/trace/span_processor.rs +++ b/opentelemetry-sdk/src/trace/span_processor.rs @@ -721,7 +721,7 @@ mod tests { use crate::testing::trace::{ new_test_export_span_data, new_test_exporter, new_tokio_test_exporter, }; - use crate::trace::{BatchConfig, EvictedQueue, SpanLinks}; + use crate::trace::{BatchConfig, SpanEvents, SpanLinks}; use async_trait::async_trait; use opentelemetry::trace::{SpanContext, SpanId, SpanKind, Status}; use std::fmt::Debug; @@ -750,7 +750,7 @@ mod tests { end_time: opentelemetry::time::now(), attributes: Vec::new(), dropped_attributes_count: 0, - events: EvictedQueue::new(0), + events: SpanEvents::default(), links: SpanLinks::default(), status: Status::Unset, resource: Default::default(), diff --git a/opentelemetry-sdk/src/trace/tracer.rs b/opentelemetry-sdk/src/trace/tracer.rs index 4b918a2fe4..cc5497b846 100644 --- a/opentelemetry-sdk/src/trace/tracer.rs +++ b/opentelemetry-sdk/src/trace/tracer.rs @@ -11,7 +11,7 @@ use crate::{ trace::{ provider::{TracerProvider, TracerProviderInner}, span::{Span, SpanData}, - Config, EvictedQueue, SpanLimits, SpanLinks, + Config, SpanLimits, SpanLinks, }, InstrumentationLibrary, }; @@ -25,6 +25,8 @@ use opentelemetry::{ use std::fmt; use std::sync::{Arc, Weak}; +use super::SpanEvents; + /// `Tracer` implementation to create and manage spans #[derive(Clone)] pub struct Tracer { @@ -237,8 +239,10 @@ impl opentelemetry::trace::Tracer for Tracer { let start_time = start_time.unwrap_or_else(opentelemetry::time::now); let end_time = end_time.unwrap_or(start_time); - let mut events_queue = EvictedQueue::new(span_limits.max_events_per_span); - if let Some(mut events) = events { + let spans_events_limit = span_limits.max_events_per_span as usize; + let span_events: SpanEvents = if let Some(mut events) = events { + let dropped_count = events.len().saturating_sub(spans_events_limit); + events.truncate(spans_events_limit); let event_attributes_limit = span_limits.max_attributes_per_event as usize; for event in events.iter_mut() { let dropped_attributes_count = event @@ -248,8 +252,13 @@ impl opentelemetry::trace::Tracer for Tracer { event.attributes.truncate(event_attributes_limit); event.dropped_attributes_count = dropped_attributes_count as u32; } - events_queue.append_vec(&mut events); - } + SpanEvents { + events, + dropped_count: dropped_count as u32, + } + } else { + SpanEvents::default() + }; let span_context = SpanContext::new(trace_id, span_id, flags, false, trace_state); Span::new( @@ -262,7 +271,7 @@ impl opentelemetry::trace::Tracer for Tracer { end_time, attributes: attribute_options, dropped_attributes_count, - events: events_queue, + events: span_events, links: span_links, status, }), diff --git a/opentelemetry-stdout/src/trace/transform.rs b/opentelemetry-stdout/src/trace/transform.rs index ac3e34a50c..877c3190e8 100644 --- a/opentelemetry-stdout/src/trace/transform.rs +++ b/opentelemetry-stdout/src/trace/transform.rs @@ -107,7 +107,7 @@ impl From for Span { end_time: value.end_time, dropped_attributes_count: value.dropped_attributes_count, attributes: value.attributes.into_iter().map(Into::into).collect(), - dropped_events_count: value.events.dropped_count(), + dropped_events_count: value.events.dropped_count, events: value.events.into_iter().map(Into::into).collect(), dropped_links_count: value.links.dropped_count, links: value.links.iter().map(Into::into).collect(), diff --git a/opentelemetry-zipkin/src/exporter/model/span.rs b/opentelemetry-zipkin/src/exporter/model/span.rs index 1b054a1db2..6e21b52588 100644 --- a/opentelemetry-zipkin/src/exporter/model/span.rs +++ b/opentelemetry-zipkin/src/exporter/model/span.rs @@ -60,8 +60,8 @@ mod tests { use crate::exporter::model::span::{Kind, Span}; use crate::exporter::model::{into_zipkin_span, OTEL_ERROR_DESCRIPTION, OTEL_STATUS_CODE}; use opentelemetry::trace::{SpanContext, SpanId, SpanKind, Status, TraceFlags, TraceId}; - use opentelemetry_sdk::trace::SpanLinks; - use opentelemetry_sdk::{export::trace::SpanData, trace::EvictedQueue, Resource}; + use opentelemetry_sdk::trace::{SpanEvents, SpanLinks}; + use opentelemetry_sdk::{export::trace::SpanData, Resource}; use std::borrow::Cow; use std::collections::HashMap; use std::net::Ipv4Addr; @@ -163,7 +163,7 @@ mod tests { end_time: SystemTime::now(), attributes: Vec::new(), dropped_attributes_count: 0, - events: EvictedQueue::new(20), + events: SpanEvents::default(), links: SpanLinks::default(), status, resource: Cow::Owned(Resource::default()),