From f9acbbc239e69901fa77a992180991c8a490f786 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 10 Jul 2024 09:27:44 -0500 Subject: [PATCH] feat(replays): Add denylist feature handling for replay video (#3803) Adds a pre-processing step to `ReplayGroup` processing. The envelope items are iterated over and if a replay-video event is found processing exits early before the items are processed. This feature should always default to enabled. It should only return the disabled state if the feature was successfully evaluated and the organization was found to be explicitly denied access. I'm not familiar with all the implications these code paths have. If someone with more knowledge and experience could double-check my work I would appreciate it. Related: https://github.com/getsentry/getsentry/pull/14569 Related: https://github.com/getsentry/sentry/pull/73981 --------- Co-authored-by: Joris Bayer --- CHANGELOG.md | 1 + relay-dynamic-config/src/feature.rs | 5 ++ relay-server/src/services/processor/replay.rs | 19 +++++++ tests/integration/test_replay_videos.py | 57 ++++++++++++++++++- 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63acf3ceb7..4ab998df53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Add support for `all` and `any` `RuleCondition`(s). ([#3791](https://github.com/getsentry/relay/pull/3791)) - Copy root span data from `contexts.trace.data` when converting transaction events into raw spans. ([#3790](https://github.com/getsentry/relay/pull/3790)) - Remove experimental double-write from spans to transactions. ([#3801](https://github.com/getsentry/relay/pull/3801)) +- Add feature flag to disable replay-video events. ([#3803](https://github.com/getsentry/relay/pull/3803)) ## 24.6.0 diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs index 7cd0a80a47..c2ecc67fa4 100644 --- a/relay-dynamic-config/src/feature.rs +++ b/relay-dynamic-config/src/feature.rs @@ -21,6 +21,11 @@ pub enum Feature { /// Serialized as `organizations:session-replay-combined-envelope-items`. #[serde(rename = "organizations:session-replay-combined-envelope-items")] SessionReplayCombinedEnvelopeItems, + /// Disables select organizations from processing mobile replay events. + /// + /// Serialized as `organizations:session-replay-video-disabled`. + #[serde(rename = "organizations:session-replay-video-disabled")] + SessionReplayVideoDisabled, /// Enables new User Feedback ingest. /// /// TODO(jferg): rename to UserFeedbackIngest once old UserReport logic is deprecated. diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 564cc319b7..877b16ab43 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -29,6 +29,7 @@ pub fn process( let project_state = &state.project_state; let replays_enabled = project_state.has_feature(Feature::SessionReplay); let scrubbing_enabled = project_state.has_feature(Feature::SessionReplayRecordingScrubbing); + let replay_video_disabled = project_state.has_feature(Feature::SessionReplayVideoDisabled); let meta = state.envelope().meta().clone(); let client_addr = meta.client_addr(); @@ -60,6 +61,13 @@ pub fn process( return Ok(()); } + // If the replay video feature is not enabled check the envelope items for a + // replay video event. + if replay_video_disabled && count_replay_video_events(state) > 0 { + state.managed_envelope.drop_items_silently(); + return Ok(()); + } + for item in state.managed_envelope.envelope_mut().items_mut() { // Set the combined payload header to the value of the combined feature. item.set_replay_combined_payload(combined_envelope_items); @@ -300,3 +308,14 @@ fn handle_replay_video_item( )), } } + +// Pre-processors + +fn count_replay_video_events(state: &ProcessEnvelopeState) -> usize { + state + .managed_envelope + .envelope() + .items() + .filter(|item| item.ty() == &ItemType::ReplayVideo) + .count() +} diff --git a/tests/integration/test_replay_videos.py b/tests/integration/test_replay_videos.py index a16200a9ba..23ea0224ad 100644 --- a/tests/integration/test_replay_videos.py +++ b/tests/integration/test_replay_videos.py @@ -18,7 +18,8 @@ def test_replay_recording_with_video( replay_id = "515539018c9b4260a6f999572f1661ee" relay = relay_with_processing() mini_sentry.add_basic_project_config( - project_id, extra={"config": {"features": ["organizations:session-replay"]}} + project_id, + extra={"config": {"features": ["organizations:session-replay"]}}, ) replay = generate_replay_sdk_event(replay_id) replay_events_consumer = replay_events_consumer(timeout=10) @@ -76,3 +77,57 @@ def test_replay_recording_with_video( replay_recordings_consumer.assert_empty() outcomes_consumer.assert_empty() replay_events_consumer.assert_empty() + + +def test_replay_recording_with_video_denied( + mini_sentry, + relay_with_processing, + replay_recordings_consumer, + outcomes_consumer, + replay_events_consumer, +): + project_id = 42 + replay_id = "515539018c9b4260a6f999572f1661ee" + relay = relay_with_processing() + mini_sentry.add_basic_project_config( + project_id, + extra={ + "config": { + "features": [ + "organizations:session-replay", + "organizations:session-replay-video-disabled", + ] + } + }, + ) + replay = generate_replay_sdk_event(replay_id) + replay_events_consumer = replay_events_consumer(timeout=10) + replay_recordings_consumer = replay_recordings_consumer() + outcomes_consumer = outcomes_consumer() + + _recording_payload = recording_payload(b"[]") + payload = msgpack.packb( + { + "replay_event": json.dumps(replay).encode(), + "replay_recording": _recording_payload, + "replay_video": b"hello, world!", + } + ) + + envelope = Envelope( + headers=[ + [ + "event_id", + replay_id, + ], + ["attachment_type", "replay_video"], + ] + ) + envelope.add_item(Item(payload=PayloadRef(bytes=payload), type="replay_video")) + + relay.send_envelope(project_id, envelope) + + # Assert all conumers are empty. + replay_recordings_consumer.assert_empty() + outcomes_consumer.assert_empty() + replay_events_consumer.assert_empty()