Skip to content

Commit

Permalink
fix(replays): Remove timestamp validation for dates from the future (#…
Browse files Browse the repository at this point in the history
…4196)

A few customers were significantly impacted by this. We're reverting for
now. We may re-introduce later with some grace period appended (e.g. an
hour or a day) to make sure any clock drift on Relay doesn't impact
ingestion.

Reverts #4163

#skip-changelog
  • Loading branch information
cmanallen authored Oct 30, 2024
1 parent 857c2b1 commit f6139ef
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 56 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
**Bug Fixes**

- Allow profile chunks without release. ([#4155](https://github.com/getsentry/relay/pull/4155))
- Add validation for timestamps sent from the future. ([#4163](https://github.com/getsentry/relay/pull/4163))

**Features:**

Expand Down
49 changes: 1 addition & 48 deletions relay-event-normalization/src/replay.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Validation and normalization of [`Replay`] events.
use chrono::Utc;
use std::net::IpAddr as StdIpAddr;

use relay_event_schema::processor::{self, ProcessingState, Processor};
Expand Down Expand Up @@ -36,10 +35,6 @@ pub enum ReplayError {
/// This erorr is usually returned when the PII configuration fails to parse.
#[error("failed to scrub PII: {0}")]
CouldNotScrub(String),

/// Date in the future.
#[error("date was from the future")]
DateInTheFuture,
}

/// Checks if the Replay event is structurally valid.
Expand Down Expand Up @@ -81,14 +76,6 @@ pub fn validate(replay: &Replay) -> Result<(), ReplayError> {
));
}

if replay
.timestamp
.value()
.map_or(false, |v| v.into_inner() > Utc::now())
{
return Err(ReplayError::DateInTheFuture);
}

Ok(())
}

Expand Down Expand Up @@ -184,7 +171,7 @@ fn normalize_type(replay: &mut Replay) {
mod tests {
use std::net::{IpAddr, Ipv4Addr};

use chrono::{Duration, TimeZone, Utc};
use chrono::{TimeZone, Utc};
use insta::assert_json_snapshot;
use relay_protocol::{assert_annotated_snapshot, get_value, SerializableAnnotated};
use uuid::Uuid;
Expand Down Expand Up @@ -452,40 +439,6 @@ mod tests {
assert!(validation_result.is_err());
}

#[test]
fn test_future_timestamp_validation() {
let future_time = Utc::now() + Duration::hours(1);
let json = format!(
r#"{{
"event_id": "52df9022835246eeb317dbd739ccd059",
"replay_id": "52df9022835246eeb317dbd739ccd059",
"segment_id": 0,
"replay_type": "session",
"error_sample_rate": 0.5,
"session_sample_rate": 0.5,
"timestamp": {},
"replay_start_timestamp": 946684800.0,
"urls": ["localhost:9000"],
"error_ids": ["test"],
"trace_ids": [],
"platform": "myplatform",
"release": "myrelease",
"dist": "mydist",
"environment": "myenv",
"tags": [
[
"tag",
"value"
]
]
}}"#,
future_time.timestamp()
);
let mut replay = Annotated::<Replay>::from_json(&json).unwrap();
let validation_result = validate(replay.value_mut().as_mut().unwrap());
assert!(validation_result.is_err());
}

#[test]
fn test_trace_id_validation() {
// NOTE: Interfaces will be tested separately.
Expand Down
4 changes: 0 additions & 4 deletions relay-server/src/services/outcome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,6 @@ pub enum DiscardReason {
InvalidReplayRecordingEvent,
InvalidReplayVideoEvent,

/// (Relay) The event's timestamp was too far in the future.
DateInTheFuture,

/// (Relay) Profiling related discard reasons
Profiling(&'static str),

Expand Down Expand Up @@ -506,7 +503,6 @@ impl DiscardReason {
DiscardReason::Profiling(reason) => reason,
DiscardReason::InvalidSpan => "invalid_span",
DiscardReason::FeatureDisabled(_) => "feature_disabled",
DiscardReason::DateInTheFuture => "date_in_the_future",
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions relay-server/src/services/processor/replay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,6 @@ fn handle_replay_event_item(
ReplayError::InvalidPayload(_) => {
ProcessingError::InvalidReplay(DiscardReason::InvalidReplayEvent)
}
ReplayError::DateInTheFuture => {
ProcessingError::InvalidReplay(DiscardReason::DateInTheFuture)
}
})
}
}
Expand Down

0 comments on commit f6139ef

Please sign in to comment.