Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(replays): Add ReplayVideo envelope item type #3105

Merged
merged 87 commits into from
Feb 27, 2024

Conversation

cmanallen
Copy link
Member

@cmanallen cmanallen commented Feb 13, 2024

Add support for replay-video event types.

Changes:

  • Refactors event validation in the processor. This is to DRY the processing logic between the three replay event types.
  • Adds new replay_video field to replay-recording Kafka message.
  • Add outcomes.
  • Configure rate-limits.

Relates to:

relay-server/src/envelope.rs Outdated Show resolved Hide resolved
relay-server/src/services/processor/replay.rs Outdated Show resolved Hide resolved
relay-server/src/envelope.rs Show resolved Hide resolved
cmanallen and others added 5 commits February 21, 2024 08:04
@cmanallen cmanallen requested review from jjbayer and TBS1996 February 22, 2024 14:33
CHANGELOG.md Outdated
@@ -28,6 +28,7 @@
**Features**:

- Add protobuf support for ingesting OpenTelemetry spans and use official `opentelemetry-proto` generated structs. ([#3044](https://github.com/getsentry/relay/pull/3044))
- Adds ReplayVideo envelope-item type. ([#3105](https://github.com/getsentry/relay/pull/3105))
Copy link
Member

Choose a reason for hiding this comment

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

Please move this to the ## Unreleased section.

@@ -97,6 +96,7 @@ reqwest = { version = "0.11.1", features = [
rmp-serde = "1.1.1"
rust-embed = { version = "8.0.0", optional = true }
serde = { workspace = true }
serde_bytes = "0.11"
Copy link
Member

Choose a reason for hiding this comment

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

This is a crate by the creators of serde, so adding it as dependency should be fine.

relay-server/src/services/store.rs Outdated Show resolved Hide resolved
Comment on lines 944 to 951
self.produce_replay_event(
event_id.ok_or(StoreError::NoEventId)?,
scoping.organization_id,
scoping.project_id,
start_time,
retention,
Bytes::copy_from_slice(replay_event),
)?;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to produce the event a second time? It's already published as part of the replay recording, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its being published to a different consumer. We're producing one event to the replay_recording consumer and another event to the replay_event consumer. The duplicated data being sent to the recording consumer for reasons other than storing the replay_event bytes. Refer to this PR #3035 where we merged the payloads.

In the future we will drop the produce_replay_event function from Relay but we're not ready yet.

relay-server/src/services/store.rs Outdated Show resolved Hide resolved
@cmanallen cmanallen enabled auto-merge (squash) February 27, 2024 12:57
@cmanallen cmanallen merged commit 6ec6039 into master Feb 27, 2024
20 checks passed
@cmanallen cmanallen deleted the cmanallen/replays-add-video-envelope-item branch February 27, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants