diff --git a/relay-event-normalization/src/replay.rs b/relay-event-normalization/src/replay.rs index 45685f17bc..250a71a421 100644 --- a/relay-event-normalization/src/replay.rs +++ b/relay-event-normalization/src/replay.rs @@ -29,12 +29,6 @@ pub enum ReplayError { #[error("invalid payload {0}")] InvalidPayload(String), - /// The Replay has consumed its segment limit. - /// - /// This is returned from [`validate`]. - #[error("invalid replay length")] - TooLong, - /// An error occurred during PII scrubbing of the Replay. /// /// This erorr is usually returned when the PII configuration fails to parse. @@ -52,19 +46,11 @@ pub fn validate(replay: &Replay) -> Result<(), ReplayError> { .value() .ok_or_else(|| ReplayError::InvalidPayload("missing replay_id".to_string()))?; - let segment_id = *replay + replay .segment_id .value() .ok_or_else(|| ReplayError::InvalidPayload("missing segment_id".to_string()))?; - // Each segment is expected to be 5 seconds in length. A cap of 1080 segments means we - // allow a replay to be up to 1.5 hours in length. - const MAX_SEGMENT_ID: u64 = 1080; - - if segment_id > MAX_SEGMENT_ID { - return Err(ReplayError::TooLong); - } - if replay .error_ids .value() @@ -372,29 +358,6 @@ mod tests { assert!(replay_value.urls.value().unwrap().len() == 100); } - #[test] - fn test_validate_segment_id() { - let replay_id = - Annotated::new(EventId("52df9022835246eeb317dbd739ccd059".parse().unwrap())); - let segment_id: Annotated = Annotated::new(1081); - let mut replay = Annotated::new(Replay { - replay_id, - segment_id, - ..Default::default() - }); - assert!(validate(replay.value_mut().as_mut().unwrap()).is_err()); - - let replay_id = - Annotated::new(EventId("52df9022835246eeb317dbd739ccd059".parse().unwrap())); - let segment_id: Annotated = Annotated::new(1080); - let mut replay = Annotated::new(Replay { - replay_id, - segment_id, - ..Default::default() - }); - assert!(validate(replay.value_mut().as_mut().unwrap()).is_ok()); - } - #[test] fn test_truncated_list_less_than_limit() { let mut replay = Annotated::new(Replay { diff --git a/relay-server/src/services/outcome.rs b/relay-server/src/services/outcome.rs index 439591f535..55a4a8b73a 100644 --- a/relay-server/src/services/outcome.rs +++ b/relay-server/src/services/outcome.rs @@ -449,7 +449,6 @@ pub enum DiscardReason { InvalidReplayEventPii, InvalidReplayRecordingEvent, InvalidReplayVideoEvent, - ReplayExceededSegmentLimit, /// (Relay) Profiling related discard reasons Profiling(&'static str), @@ -501,7 +500,6 @@ impl DiscardReason { DiscardReason::InvalidReplayEventPii => "invalid_replay_pii_scrubber_failed", DiscardReason::InvalidReplayRecordingEvent => "invalid_replay_recording", DiscardReason::InvalidReplayVideoEvent => "invalid_replay_video", - DiscardReason::ReplayExceededSegmentLimit => "replay_segment_limit_exceeded", DiscardReason::Profiling(reason) => reason, DiscardReason::InvalidSpan => "invalid_span", DiscardReason::FeatureDisabled(_) => "feature_disabled", diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 7cfba35808..5efc6d8fe4 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize}; use crate::envelope::{ContentType, ItemType}; use crate::services::outcome::DiscardReason; use crate::services::processor::{ProcessEnvelopeState, ProcessingError, ReplayGroup}; -use crate::statsd::RelayTimers; +use crate::statsd::{RelayCounters, RelayTimers}; /// Removes replays if the feature flag is not enabled. pub fn process( @@ -143,6 +143,22 @@ fn handle_replay_event_item( global_config.filters(), ) .map_err(ProcessingError::ReplayFiltered)?; + + // Log segments that exceed the hour limit so we can diagnose errant SDKs + // or exotic customer implementations. + if let Some(segment_id) = replay_type.segment_id.value() { + if *segment_id > 720 { + metric!(counter(RelayCounters::ReplayExceededSegmentLimit) += 1); + + relay_log::warn!( + ?event_id, + project_id = project_id.map(|v| v.value()), + organization_id = organization_id, + segment_id = segment_id, + "replay segment-exceeded-limit" + ); + } + } } match replay.to_json() { @@ -178,9 +194,6 @@ fn handle_replay_event_item( ReplayError::InvalidPayload(_) => { ProcessingError::InvalidReplay(DiscardReason::InvalidReplayEvent) } - ReplayError::TooLong => { - ProcessingError::InvalidReplay(DiscardReason::ReplayExceededSegmentLimit) - } }) } } diff --git a/relay-server/src/statsd.rs b/relay-server/src/statsd.rs index 001182ce8a..3788854b93 100644 --- a/relay-server/src/statsd.rs +++ b/relay-server/src/statsd.rs @@ -822,6 +822,8 @@ pub enum RelayCounters { /// This metric is tagged with: /// - `aggregator`: The name of the metrics aggregator (usually `"default"`). BucketsDropped, + /// Incremented every time a segment exceeds the expected limit. + ReplayExceededSegmentLimit, } impl CounterMetric for RelayCounters { @@ -866,6 +868,7 @@ impl CounterMetric for RelayCounters { RelayCounters::CogsUsage => "cogs.usage", RelayCounters::ProjectStateFlushMetricsNoProject => "project_state.metrics.no_project", RelayCounters::BucketsDropped => "metrics.buckets.dropped", + RelayCounters::ReplayExceededSegmentLimit => "replay.segment_limit_exceeded", } } }