diff --git a/opentelemetry/src/sdk/trace/config.rs b/opentelemetry/src/sdk/trace/config.rs index 04a4e711c2..54ca40f77d 100644 --- a/opentelemetry/src/sdk/trace/config.rs +++ b/opentelemetry/src/sdk/trace/config.rs @@ -2,6 +2,7 @@ //! //! Configuration represents the global tracing configuration, overrides //! can be set for the default OpenTelemetry limits and Sampler. +use crate::sdk::trace::span_limit::SpanLimits; use crate::{sdk, sdk::trace::Sampler, trace::IdGenerator}; use std::env; use std::str::FromStr; @@ -19,12 +20,8 @@ pub struct Config { pub sampler: Box, /// The id generator that the sdk should use pub id_generator: Box, - /// The max events that can be added to a `Span`. - pub max_events_per_span: u32, - /// The max attributes that can be added to a `Span`. - pub max_attributes_per_span: u32, - /// The max links that can be added to a `Span`. - pub max_links_per_span: u32, + /// span limit + pub span_limit: SpanLimits, /// Contains attributes representing an entity that produces telemetry. pub resource: Option>, } @@ -44,19 +41,37 @@ impl Config { /// Specify the number of events to be recorded per span. pub fn with_max_events_per_span(mut self, max_events: u32) -> Self { - self.max_events_per_span = max_events; + self.span_limit.max_events_per_span = max_events; self } /// Specify the number of attributes to be recorded per span. pub fn with_max_attributes_per_span(mut self, max_attributes: u32) -> Self { - self.max_attributes_per_span = max_attributes; + self.span_limit.max_attributes_per_span = max_attributes; self } /// Specify the number of events to be recorded per span. pub fn with_max_links_per_span(mut self, max_links: u32) -> Self { - self.max_links_per_span = max_links; + self.span_limit.max_links_per_span = max_links; + self + } + + /// Specify the number of attributes one event can have. + pub fn with_max_attributes_per_event(mut self, max_attributes: u32) -> Self { + self.span_limit.max_attributes_per_event = max_attributes; + self + } + + /// Specify the number of attributes one link can have. + pub fn with_max_attributes_per_link(mut self, max_attributes: u32) -> Self { + self.span_limit.max_attributes_per_link = max_attributes; + self + } + + /// Specify all limit via the span_limit + pub fn with_span_limit(mut self, span_limit: SpanLimits) -> Self { + self.span_limit = span_limit; self } @@ -73,9 +88,7 @@ impl Default for Config { let mut config = Config { sampler: Box::new(Sampler::ParentBased(Box::new(Sampler::AlwaysOn))), id_generator: Box::new(sdk::trace::IdGenerator::default()), - max_events_per_span: 128, - max_attributes_per_span: 128, - max_links_per_span: 128, + span_limit: SpanLimits::default(), resource: None, }; @@ -83,21 +96,21 @@ impl Default for Config { .ok() .and_then(|count_limit| u32::from_str(&count_limit).ok()) { - config.max_attributes_per_span = max_attributes_per_span; + config.span_limit.max_attributes_per_span = max_attributes_per_span; } if let Some(max_events_per_span) = env::var("OTEL_SPAN_EVENT_COUNT_LIMIT") .ok() .and_then(|max_events| u32::from_str(&max_events).ok()) { - config.max_events_per_span = max_events_per_span; + config.span_limit.max_events_per_span = max_events_per_span; } if let Some(max_links_per_span) = env::var("OTEL_SPAN_LINK_COUNT_LIMIT") .ok() .and_then(|max_links| u32::from_str(&max_links).ok()) { - config.max_links_per_span = max_links_per_span; + config.span_limit.max_links_per_span = max_links_per_span; } config diff --git a/opentelemetry/src/sdk/trace/mod.rs b/opentelemetry/src/sdk/trace/mod.rs index d465f0ed47..5744518175 100644 --- a/opentelemetry/src/sdk/trace/mod.rs +++ b/opentelemetry/src/sdk/trace/mod.rs @@ -13,6 +13,7 @@ mod id_generator; mod provider; mod sampler; mod span; +mod span_limit; mod span_processor; mod tracer; @@ -23,6 +24,7 @@ pub use id_generator::{aws::XrayIdGenerator, IdGenerator}; pub use provider::{Builder, TracerProvider}; pub use sampler::{Sampler, SamplingDecision, SamplingResult, ShouldSample}; pub use span::Span; +pub use span_limit::SpanLimits; pub use span_processor::{ BatchConfig, BatchSpanProcessor, BatchSpanProcessorBuilder, SimpleSpanProcessor, SpanProcessor, }; diff --git a/opentelemetry/src/sdk/trace/span.rs b/opentelemetry/src/sdk/trace/span.rs index 06942afb4c..a27a9e5fbf 100644 --- a/opentelemetry/src/sdk/trace/span.rs +++ b/opentelemetry/src/sdk/trace/span.rs @@ -8,6 +8,7 @@ //! start time is set to the current time on span creation. After the `Span` is created, it //! is possible to change its name, set its `Attributes`, and add `Links` and `Events`. //! These cannot be changed after the `Span`'s end time has been set. +use crate::sdk::trace::SpanLimits; use crate::trace::{Event, SpanContext, SpanId, SpanKind, StatusCode}; use crate::{sdk, trace, KeyValue}; use std::borrow::Cow; @@ -20,6 +21,7 @@ pub struct Span { span_context: SpanContext, data: Option, tracer: sdk::trace::Tracer, + span_limit: SpanLimits, } #[derive(Clone, Debug, PartialEq)] @@ -51,11 +53,13 @@ impl Span { span_context: SpanContext, data: Option, tracer: sdk::trace::Tracer, + span_limit: SpanLimits, ) -> Self { Span { span_context, data, tracer, + span_limit, } } @@ -78,9 +82,14 @@ impl crate::trace::Span for Span { &mut self, name: String, timestamp: SystemTime, - attributes: Vec, + mut attributes: Vec, ) { + let max_attributes_per_event = self.span_limit.max_attributes_per_event as usize; self.with_data(|data| { + if attributes.len() > max_attributes_per_event { + attributes.truncate(max_attributes_per_event); + } + data.message_events .push_back(Event::new(name, timestamp, attributes)) }); @@ -204,6 +213,10 @@ fn build_export_data( #[cfg(test)] mod tests { use super::*; + use crate::sdk::trace::span_limit::{ + DEFAULT_MAX_ATTRIBUTES_PER_EVENT, DEFAULT_MAX_ATTRIBUTES_PER_LINK, + }; + use crate::trace::{Link, NoopSpanExporter, TraceId, Tracer}; use crate::{core::KeyValue, trace::Span as _, trace::TracerProvider}; use std::time::Duration; @@ -217,9 +230,12 @@ mod tests { name: "opentelemetry".into(), start_time: crate::time::now(), end_time: crate::time::now(), - attributes: sdk::trace::EvictedHashMap::new(config.max_attributes_per_span, 0), - message_events: sdk::trace::EvictedQueue::new(config.max_events_per_span), - links: sdk::trace::EvictedQueue::new(config.max_links_per_span), + attributes: sdk::trace::EvictedHashMap::new( + config.span_limit.max_attributes_per_span, + 0, + ), + message_events: sdk::trace::EvictedQueue::new(config.span_limit.max_events_per_span), + links: sdk::trace::EvictedQueue::new(config.span_limit.max_links_per_span), status_code: StatusCode::Unset, status_message: "".into(), }; @@ -228,20 +244,35 @@ mod tests { fn create_span() -> Span { let (tracer, data) = init(); - Span::new(SpanContext::empty_context(), Some(data), tracer) + Span::new( + SpanContext::empty_context(), + Some(data), + tracer, + Default::default(), + ) } #[test] fn create_span_without_data() { let (tracer, _) = init(); - let mut span = Span::new(SpanContext::empty_context(), None, tracer); + let mut span = Span::new( + SpanContext::empty_context(), + None, + tracer, + Default::default(), + ); span.with_data(|_data| panic!("there are data")); } #[test] fn create_span_with_data_mut() { let (tracer, data) = init(); - let mut span = Span::new(SpanContext::empty_context(), Some(data.clone()), tracer); + let mut span = Span::new( + SpanContext::empty_context(), + Some(data.clone()), + tracer, + Default::default(), + ); span.with_data(|d| assert_eq!(*d, data)); } @@ -445,4 +476,74 @@ mod tests { span.end(); assert!(!span.is_recording()); } + + #[test] + fn exceed_event_attributes_limit() { + let exporter = NoopSpanExporter::new(); + let provider_builder = sdk::trace::TracerProvider::builder().with_simple_exporter(exporter); + let provider = provider_builder.build(); + let tracer = provider.get_tracer("opentelemetry-test", None); + + let mut event1 = Event::with_name("test event"); + for i in 0..(DEFAULT_MAX_ATTRIBUTES_PER_EVENT * 2) { + event1 + .attributes + .push(KeyValue::new(format!("key {}", i), i.to_string())) + } + let event2 = event1.clone(); + + // add event when build + let span_builder = tracer + .span_builder("test") + .with_message_events(vec![event1]); + let mut span = tracer.build(span_builder); + + // add event after build + span.add_event("another test event".into(), event2.attributes); + + let event_queue = span + .data + .clone() + .expect("span data should not be empty as we already set it before") + .message_events; + let event_vec: Vec<_> = event_queue.iter().take(2).collect(); + let processed_event_1 = event_vec.get(0).expect("should have at least two events"); + let processed_event_2 = event_vec.get(1).expect("should have at least two events"); + assert_eq!(processed_event_1.attributes.len(), 128); + assert_eq!(processed_event_2.attributes.len(), 128); + } + + #[test] + fn exceed_link_attributes_limit() { + let exporter = NoopSpanExporter::new(); + let provider_builder = sdk::trace::TracerProvider::builder().with_simple_exporter(exporter); + let provider = provider_builder.build(); + let tracer = provider.get_tracer("opentelemetry-test", None); + + let mut link = Link::new( + SpanContext::new( + TraceId::from_u128(0), + SpanId::from_u64(0), + 0, + false, + Default::default(), + ), + Vec::new(), + ); + for i in 0..(DEFAULT_MAX_ATTRIBUTES_PER_LINK * 2) { + link.attributes_mut() + .push(KeyValue::new(format!("key {}", i), i.to_string())); + } + + let span_builder = tracer.span_builder("test").with_links(vec![link]); + let span = tracer.build(span_builder); + let link_queue = span + .data + .clone() + .expect("span data should not be empty as we already set it before") + .links; + let link_vec: Vec<_> = link_queue.iter().collect(); + let processed_link = link_vec.get(0).expect("should have at least one link"); + assert_eq!(processed_link.attributes().len(), 128); + } } diff --git a/opentelemetry/src/sdk/trace/span_limit.rs b/opentelemetry/src/sdk/trace/span_limit.rs new file mode 100644 index 0000000000..77ea183c30 --- /dev/null +++ b/opentelemetry/src/sdk/trace/span_limit.rs @@ -0,0 +1,47 @@ +/// # Span limit +/// Erroneous code can add unintended attributes, events, and links to a span. If these collections +/// are unbounded, they can quickly exhaust available memory, resulting in crashes that are +/// difficult to recover from safely. +/// +/// To protected against those errors. Users can use span limit to configure +/// - Maximum allowed span attribute count +/// - Maximum allowed span event count +/// - Maximum allowed span link count +/// - Maximum allowed attribute per span event count +/// - Maximum allowed attribute per span link count +/// +/// If the limit has been breached. The attributes, events or links will be dropped based on their +/// index in the collection. The one added to collections later will be dropped first. + +pub(crate) const DEFAULT_MAX_EVENT_PER_SPAN: u32 = 128; +pub(crate) const DEFAULT_MAX_ATTRIBUTES_PER_SPAN: u32 = 128; +pub(crate) const DEFAULT_MAX_LINKS_PER_SPAN: u32 = 128; +pub(crate) const DEFAULT_MAX_ATTRIBUTES_PER_EVENT: u32 = 128; +pub(crate) const DEFAULT_MAX_ATTRIBUTES_PER_LINK: u32 = 128; + +/// Span limit configuration to keep attributes, events and links to a span in a reasonable number. +#[derive(Copy, Clone, Debug)] +pub struct SpanLimits { + /// The max events that can be added to a `Span`. + pub max_events_per_span: u32, + /// The max attributes that can be added to a `Span`. + pub max_attributes_per_span: u32, + /// The max links that can be added to a `Span`. + pub max_links_per_span: u32, + /// The max attributes that can be added into an `Event` + pub max_attributes_per_event: u32, + /// The max attributes that can be added into a `Link` + pub max_attributes_per_link: u32, +} + +impl Default for SpanLimits { + fn default() -> Self { + SpanLimits { + max_events_per_span: DEFAULT_MAX_EVENT_PER_SPAN, + max_attributes_per_span: DEFAULT_MAX_ATTRIBUTES_PER_SPAN, + max_links_per_span: DEFAULT_MAX_LINKS_PER_SPAN, + max_attributes_per_link: DEFAULT_MAX_ATTRIBUTES_PER_LINK, + max_attributes_per_event: DEFAULT_MAX_ATTRIBUTES_PER_EVENT, + } + } +} diff --git a/opentelemetry/src/sdk/trace/tracer.rs b/opentelemetry/src/sdk/trace/tracer.rs index 1ba8cb01d4..1a5a5eed1f 100644 --- a/opentelemetry/src/sdk/trace/tracer.rs +++ b/opentelemetry/src/sdk/trace/tracer.rs @@ -7,6 +7,7 @@ //! and exposes methods for creating and activating new `Spans`. //! //! Docs: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#tracer +use crate::sdk::trace::SpanLimits; use crate::sdk::{ trace::{ provider::{TracerProvider, TracerProviderInner}, @@ -125,7 +126,12 @@ impl crate::trace::Tracer for Tracer { /// Returns a span with an inactive `SpanContext`. Used by functions that /// need to return a default span like `get_active_span` if no span is present. fn invalid(&self) -> Self::Span { - Span::new(SpanContext::empty_context(), None, self.clone()) + Span::new( + SpanContext::empty_context(), + None, + self.clone(), + SpanLimits::default(), + ) } /// Starts a new `Span` with a given context. @@ -162,11 +168,17 @@ impl crate::trace::Tracer for Tracer { fn build(&self, mut builder: SpanBuilder) -> Self::Span { let provider = self.provider(); if provider.is_none() { - return Span::new(SpanContext::empty_context(), None, self.clone()); + return Span::new( + SpanContext::empty_context(), + None, + self.clone(), + SpanLimits::default(), + ); } let provider = provider.unwrap(); let config = provider.config(); + let span_limit = config.span_limit; let span_id = builder .span_id .take() @@ -253,18 +265,31 @@ impl crate::trace::Tracer for Tracer { span_trace_state = trace_state; attribute_options.append(&mut extra_attrs); let mut attributes = - EvictedHashMap::new(config.max_attributes_per_span, attribute_options.len()); + EvictedHashMap::new(span_limit.max_attributes_per_span, attribute_options.len()); for attribute in attribute_options { attributes.insert(attribute); } - let mut links = EvictedQueue::new(config.max_links_per_span); + let mut links = EvictedQueue::new(span_limit.max_links_per_span); if let Some(link_options) = &mut link_options { + for link in link_options.iter_mut() { + // make sure the attributes is less than max_attribute_per_link + let attributes = link.attributes_mut(); + if attributes.len() > span_limit.max_attributes_per_link as usize { + attributes.truncate(span_limit.max_attributes_per_link as usize); + } + } links.append_vec(link_options); } let start_time = start_time.unwrap_or_else(crate::time::now); let end_time = end_time.unwrap_or(start_time); - let mut message_events_queue = EvictedQueue::new(config.max_events_per_span); + let mut message_events_queue = EvictedQueue::new(span_limit.max_events_per_span); if let Some(mut events) = message_events { + for event in events.iter_mut() { + let attributes = &mut event.attributes; + if attributes.len() > span_limit.max_attributes_per_event as usize { + attributes.truncate(span_limit.max_attributes_per_event as usize); + } + } message_events_queue.append_vec(&mut events); } let status_code = status_code.unwrap_or(StatusCode::Unset); @@ -285,7 +310,7 @@ impl crate::trace::Tracer for Tracer { }); let span_context = SpanContext::new(trace_id, span_id, flags, false, span_trace_state); - let span = Span::new(span_context, inner, self.clone()); + let span = Span::new(span_context, inner, self.clone(), span_limit); // Call `on_start` for all processors for processor in provider.span_processors() { diff --git a/opentelemetry/src/testing/trace.rs b/opentelemetry/src/testing/trace.rs index db4c708443..dca9ce7aaf 100644 --- a/opentelemetry/src/testing/trace.rs +++ b/opentelemetry/src/testing/trace.rs @@ -46,9 +46,9 @@ pub fn new_test_export_span_data() -> SpanData { name: "opentelemetry".into(), start_time: crate::time::now(), end_time: crate::time::now(), - attributes: EvictedHashMap::new(config.max_attributes_per_span, 0), - message_events: EvictedQueue::new(config.max_events_per_span), - links: EvictedQueue::new(config.max_links_per_span), + attributes: EvictedHashMap::new(config.span_limit.max_attributes_per_span, 0), + message_events: EvictedQueue::new(config.span_limit.max_events_per_span), + links: EvictedQueue::new(config.span_limit.max_links_per_span), status_code: StatusCode::Unset, status_message: "".into(), resource: config.resource, diff --git a/opentelemetry/src/trace/link.rs b/opentelemetry/src/trace/link.rs index 8405785e82..465ac7a9d6 100644 --- a/opentelemetry/src/trace/link.rs +++ b/opentelemetry/src/trace/link.rs @@ -30,4 +30,9 @@ impl Link { pub fn attributes(&self) -> &Vec { &self.attributes } + + /// Mutable attributes of the link + pub(crate) fn attributes_mut(&mut self) -> &mut Vec { + self.attributes.as_mut() + } }