Repository navigation
fix(replays): Bound recording segment decompression at ingest - #129541
Open
sentry-junior[bot] wants to merge 5 commits into
Open
sentry-junior[bot] wants to merge 5 commits into
sentry-junior[bot] wants to merge 5 commits into
Conversation
Recording segments were inflated with an uncapped `zlib.decompress` before being stored. Relay bounds the compressed payload at 10 MiB, but only applies its uncompressed bound for orgs with recording scrubbing and a non-empty PII config, so a highly compressible segment could be stored with no bound on its inflated size. Decompress incrementally against a byte cap instead, and drop segments that exceed it. The limit comes from a new `replay.consumer.max-segment-decompressed-size` option (default 100 MiB) so it can be tuned without a deploy; drops are counted under `replays.consumer.recording.oversized_segment`. A follow-up applies the same bound on the read path. Refs REPLAY-994
…replay-994-bound-segment-decompression-at-ingest # Conflicts: # src/sentry/replays/consumers/recording.py Co-Authored-By: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
Read the option where it is used instead of threading it through process_message and parse_recording_event. Refs REPLAY-994 Co-Authored-By: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
decompress_segment reads replay.consumer.max-segment-decompressed-size from the options store, which falls through to the database on a cache miss. The unit tests calling it without a DB only passed when another test had already warmed the options cache, and failed when run on their own. Refs REPLAY-994 Co-Authored-By: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
github-actions
Bot
added
the
Scope: Backend
Contributor
|
@cursor review |
DominikB2014
marked this pull request as ready for review
November 2, 2026 20:32
vjeranc
reviewed
Nov 3, 2026
vjeranc
left a comment
vjeranc
left a comment
Member
There was a problem hiding this comment.
This is a good place for the bound: it caps an existing decompression pass, and replay processing reuses the decompressed bytes.
Read replay.consumer.max-segment-decompressed-size in parse_recording_event and pass it to decompress_segment, so the helper no longer depends on the options store and its unit tests no longer need the database. Add regression tests for truncated zlib streams and corrupted checksums, since decompressobj returns partial output on truncation where zlib.decompress raised. Refs REPLAY-994
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backend only (
src/,tests/). Relay companion: getsentry/relay#6587, which applies Relay's uncompressed size limit to every recording, not only scrubbed ones.Problem
decompress_segmentinflated recording segments with an uncappedzlib.decompressbefore the consumer stored them. Relay bounds the compressed payload at 10 MiB, but only applies its 100 MiB uncompressed bound for orgs that have recording scrubbing enabled with a non-empty PII config. For everyone else nothing checked how large a segment became once inflated, and the consumer stored the client's compressed bytes verbatim.Change
decompress_segmentnow decompresses incrementally viazlib.decompressobj, requesting one byte past the limit so an oversized segment is detected without ever materializing more than one byte over the limit. Segments over the limit raiseDropSilently.replay.consumer.max-segment-decompressed-sizeoption, default 100 MiB,FLAG_AUTOMATOR_MODIFIABLE— so the limit can be raised or lowered without a deploy.replays.consumer.recording.oversized_segmentand logged with the compressed size and the limit.parse_recording_eventreads the option and passes it todecompress_segmentasmax_size, so the helper stays independent of the options store.Behavior for well-formed and already-uncompressed segments is unchanged.
zlib.decompressobjreturns partial output on a truncated stream where the one-shotzlib.decompressraised, so that case is normalized back tozlib.errorto preserve the existing handling. Regression tests cover truncated streams and corrupted checksums.Rollout note
The default matches the uncompressed bound Relay already applies to scrubbing-enabled orgs, so it should be a no-op for segments produced by SDKs. If it turns out legitimate segments sit closer to the limit than expected,
replays.consumer.recording.oversized_segmentwill show it and the option can be raised immediately. Worth watching that metric before considering this settled.Refs REPLAY-994
--
View Session [Sentry]