Skip to content
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

- Add new frame fields for MetricKit flamegraphs. ([#5539](https://github.com/getsentry/relay/pull/5539))
- Apply clock drift correction to logs and trace metrics. ([#5609](https://github.com/getsentry/relay/pull/5609))
- Trim spans with a new EAP trimming processor. ([#5616](https://github.com/getsentry/relay/pull/5616))

**Internal**:

Expand Down
16 changes: 16 additions & 0 deletions relay-config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,16 @@ pub struct Limits {
max_replay_uncompressed_size: ByteSize,
/// The maximum size for a replay recording Kafka message.
pub max_replay_message_size: ByteSize,
/// The byte size limit up to which Relay will retain
/// keys of invalid/removed attributes.
///
/// This is only relevant for EAP items (spans, logs, …).
/// In principle, we want to record all deletions of attributes,
/// but we have to institute some limit to protect our infrastructure
/// against excessive metadata sizes.
///
/// Defaults to 10KiB.
pub max_removed_attribute_key_size: ByteSize,
/// The maximum number of threads to spawn for CPU and web work, each.
///
/// The total number of threads spawned will roughly be `2 * max_thread_count`. Defaults to
Expand Down Expand Up @@ -735,6 +745,7 @@ impl Default for Limits {
idle_timeout: None,
max_connections: None,
tcp_listen_backlog: 1024,
max_removed_attribute_key_size: ByteSize::kibibytes(10),
}
}
}
Expand Down Expand Up @@ -2469,6 +2480,11 @@ impl Config {
self.values.limits.max_concurrent_queries
}

/// Returns the maximum combined size of keys of invalid attributes.
pub fn max_removed_attribute_key_size(&self) -> usize {
self.values.limits.max_removed_attribute_key_size.as_bytes()
}

/// The maximum number of seconds a query is allowed to take across retries.
pub fn query_timeout(&self) -> Duration {
Duration::from_secs(self.values.limits.query_timeout)
Expand Down
30 changes: 30 additions & 0 deletions relay-dynamic-config/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ pub struct ProjectConfig {
/// Retention settings for different products.
#[serde(default, skip_serializing_if = "RetentionsConfig::is_empty")]
pub retentions: RetentionsConfig,
/// Trimming settings for different products.
#[serde(default, skip_serializing_if = "TrimmingConfigs::is_empty")]
pub trimming: TrimmingConfigs,
/// Usage quotas for this project.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub quotas: Vec<Quota>,
Expand Down Expand Up @@ -159,6 +162,7 @@ impl Default for ProjectConfig {
event_retention: None,
downsampled_event_retention: None,
retentions: Default::default(),
trimming: Default::default(),
quotas: Vec::new(),
sampling: None,
measurements: None,
Expand Down Expand Up @@ -205,6 +209,8 @@ pub struct LimitedProjectConfig {
pub filter_settings: ProjectFiltersConfig,
#[serde(skip_serializing_if = "DataScrubbingConfig::is_disabled")]
pub datascrubbing_settings: DataScrubbingConfig,
#[serde(skip_serializing_if = "TrimmingConfigs::is_empty")]
pub trimming: TrimmingConfigs,
#[serde(skip_serializing_if = "Option::is_none")]
pub sampling: Option<ErrorBoundary<SamplingConfig>>,
#[serde(skip_serializing_if = "SessionMetricsConfig::is_disabled")]
Expand Down Expand Up @@ -277,6 +283,30 @@ impl RetentionsConfig {
}
}

/// Per-category settings for item trimming.
#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct TrimmingConfig {
/// The maximum size in bytes above which an item should be trimmed.
pub max_size: u32,
}

/// Settings for item trimming.
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
#[serde(rename_all = "camelCase")]
pub struct TrimmingConfigs {
/// Trimming settings for spans.
#[serde(skip_serializing_if = "Option::is_none")]
pub span: Option<TrimmingConfig>,
}

impl TrimmingConfigs {
fn is_empty(&self) -> bool {
let Self { span } = self;
span.is_none()
}
}

fn is_false(value: &bool) -> bool {
!*value
}
Expand Down
2 changes: 1 addition & 1 deletion relay-event-normalization/src/eap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ mod ai;
mod size;
pub mod time;
pub mod trace_metric;
#[allow(unused)]
mod trimming;

pub use self::ai::normalize_ai;
pub use self::size::*;
pub use self::trimming::TrimmingProcessor;

/// Infers the sentry.op attribute and inserts it into [`Attributes`] if not already set.
pub fn normalize_sentry_op(attributes: &mut Annotated<Attributes>) {
Expand Down
6 changes: 6 additions & 0 deletions relay-event-normalization/src/eap/trimming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,12 @@ impl Processor for TrimmingProcessor {
return Ok(());
}

// This counts the lengths of all attribute keys regardless of whether
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object trimming drops last remarked key via split_off

Medium Severity

The process_object split logic sets i = key.as_str() before the match and never advances it afterward, so split_off(&i) removes the last key that received WithRemark. In contrast, process_array and process_attributes both increment i after the match, correctly preserving remarked entries. When all remaining keys fit within removed_key_byte_budget, the final remarked key is silently dropped. This code was previously gated behind #[allow(unused)] and is now activated for span processing.

Additional Locations (1)

Fix in Cursor Fix in Web

// the attribute itself is valid or invalid. Strictly speaking, this is
// inconsistent with the trimming logic, which only counts keys of valid
// attributes. However, this value is only used to set the `original_value`
// on the attributes collection for documentation purposes, we accept this
// discrepancy for now. In any case this is fine to change.
let original_length = size::attributes_size(attributes);

// Sort attributes by key + value size so small attributes are more likely to be preserved.
Expand Down
18 changes: 10 additions & 8 deletions relay-event-schema/src/protocol/span_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,39 @@ pub struct SpanV2 {
pub trace_id: Annotated<TraceId>,

/// The ID of the span enclosing this span.
#[metastructure(trim = false)]
pub parent_span_id: Annotated<SpanId>,

/// The Span ID.
#[metastructure(required = true, nonempty = true, trim = false)]
pub span_id: Annotated<SpanId>,

/// Span type (see `OperationType` docs).
#[metastructure(required = true)]
#[metastructure(required = true, trim = false)]
pub name: Annotated<OperationType>,

/// The span's status.
#[metastructure(required = true)]
#[metastructure(required = true, trim = false)]
pub status: Annotated<SpanV2Status>,

/// Whether this span is the root span of a segment.
#[metastructure(trim = false)]
pub is_segment: Annotated<bool>,

/// Timestamp when the span started.
#[metastructure(required = true)]
#[metastructure(required = true, trim = false)]
pub start_timestamp: Annotated<Timestamp>,

/// Timestamp when the span was ended.
#[metastructure(required = true)]
#[metastructure(required = true, trim = false)]
pub end_timestamp: Annotated<Timestamp>,

/// Links from this span to other spans.
#[metastructure(pii = "maybe")]
#[metastructure(pii = "maybe", trim = true)]
pub links: Annotated<Array<SpanV2Link>>,

/// Arbitrary attributes on a span.
#[metastructure(pii = "true", trim = false)]
#[metastructure(pii = "true", trim = true)]
pub attributes: Annotated<Attributes>,

/// Additional arbitrary fields for forwards compatibility.
Expand Down Expand Up @@ -177,11 +179,11 @@ pub struct SpanV2Link {
pub sampled: Annotated<bool>,

/// Span link attributes, similar to span attributes/data.
#[metastructure(pii = "maybe", trim = false)]
#[metastructure(pii = "maybe", trim = true)]
pub attributes: Annotated<Attributes>,

/// Additional arbitrary fields for forwards compatibility.
#[metastructure(additional_properties, pii = "maybe", trim = false)]
#[metastructure(additional_properties, pii = "maybe")]
pub other: Object<Value>,
}

Expand Down
21 changes: 17 additions & 4 deletions relay-server/src/processing/spans/process.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use std::collections::BTreeMap;
use std::time::Duration;

use relay_event_normalization::{
GeoIpLookup, RequiredMode, SchemaProcessor, TrimmingProcessor, eap,
};
use relay_event_normalization::{GeoIpLookup, RequiredMode, SchemaProcessor, eap};
use relay_event_schema::processor::{ProcessingState, ValueType, process_value};
use relay_event_schema::protocol::{Span, SpanId, SpanV2};
use relay_protocol::Annotated;
Expand Down Expand Up @@ -190,7 +188,22 @@ fn normalize_span(
eap::write_legacy_attributes(&mut span.attributes);
};

process_value(span, &mut TrimmingProcessor::new(), ProcessingState::root())?;
// Set a max_bytes value on the root state if it's defined in the project config.
// This causes the whole item to be trimmed down to the limit.
let max_bytes = ctx
.project_info
.config()
.trimming
.span
.map(|cfg| cfg.max_size as usize);
let trimming_root = ProcessingState::root_builder().max_bytes(max_bytes).build();

process_value(
span,
&mut eap::TrimmingProcessor::new(ctx.config.max_removed_attribute_key_size()),
&trimming_root,
)?;

process_value(
span,
&mut SchemaProcessor::new().with_required(RequiredMode::DeleteParent),
Expand Down
Loading
Loading