Skip to content

feat(attachments): Add flame graph attachment support - #6473

Open
philprime wants to merge 8 commits into
masterfrom
philprime/feat/flamegraph-attachments
Open

philprime wants to merge 8 commits into
masterfrom
philprime/feat/flamegraph-attachments

Conversation

@philprime

@philprime philprime commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Note

This is my first contribution to relay, so it's heavily assisted by agents. Please let me know how you want me to continue working on it. Happy for any guidance for working no Relay without breaking best-practices.

Recognize and validate event.flamegraph attachments against the draft contract in getsentry/sentry-docs#19774, preserving their type and original payload through Kafka and objectstore ingestion.

Validation

Required headers and association with an explicit event or transaction in the same envelope are checked through shared attachment processing, including on non-processing Relays. Invalid attachments produce invalid_flamegraph outcomes without discarding the associated event. The 10 MiB cap uses the existing size-limit path, preserving shared attachment count and byte budgets, rejection metrics, and whole-envelope HTTP 413 behavior for oversized items.

Processing Relays validate version 1 payloads using the V2 profiling frame and debug metadata schemas: non-empty tables and roots, native addresses, relative image references, frame indices, integer sample-count bounds, and child-count invariants. Debug-image IDs are indexed once to avoid quadratic lookup work. Validation leaves unknown fields and original symbolication inputs unchanged.

Boundary

Symbolication, its 2,500-input-frame budget, inline expansion, symbolication state, and rendering belong to separate Sentry backend and frontend PRs. Relay does not infer source thread identity or process provenance, and cannot determine whether an omitted absolute-address debug image was known to the producer. Existing attachment quotas and privacy rules still apply. This change does not add structured scrubbing or multipart filename inference.

Verification

make check, the all-feature build, default-feature Clippy with warnings denied, seven default-feature flamegraph Rust tests, eleven all-feature flamegraph Rust tests, and three transport integration cases passed locally. Added coverage checks shared size budgets, non-processing header validation, association outcomes, and large relative-address tables.

Processing integration remains blocked locally by unavailable Redis services. Corrected invalid-attachment integration tests to read the preserved event from the events topic and assert that the attachments topic remains empty. Kafka and objectstore payload-preservation coverage is included but was not reverified locally during this update. The full all-feature Rust suite was not rerun.

@philprime philprime self-assigned this Oct 5, 2026
Comment thread relay-profiling/src/flamegraph.rs Outdated
@philprime
philprime marked this pull request as ready for review October 13, 2026 09:00
@philprime
philprime requested a review from a team as a code owner October 13, 2026 09:00
@philprime philprime changed the title feat(attachments): Add flamegraph attachment support feat(attachments): Add flame graph attachment support Oct 13, 2026

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread relay-server/src/processing/errors/mod.rs Outdated
Comment thread relay-server/src/envelope/item.rs Outdated
Comment thread relay-server/src/processing/attachments/mod.rs Outdated
Comment thread relay-profiling/src/flamegraph.rs
Comment thread relay-server/src/utils/sizes.rs Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread relay-profiling/src/flamegraph.rs
@linear-code

linear-code Bot commented Oct 16, 2026

Copy link
Copy Markdown

COCOA-2069

@philprime

philprime commented Oct 16, 2026 •

Copy link
Copy Markdown
Member Author

Local end-to-end testing exposed a parsing limit that is not part of the flamegraph contract. The MetricKit example has 719 frame entries, 19 trees, and a maximum call-path depth of 130. It is only about 128 KiB, but its nested nodes/children exceed serde_json's default recursion limit. Relay consequently dropped the attachment while retaining the event.

The attachment specification limits serialized size to 10 MiB, not call-tree depth. Truncating the input to fit the parser would lose the diagnostic context this attachment format is intended to preserve.

Fixed in 28b2c56. Parsing uses stack growth for deep call trees, with iterative node destruction so dropping the parsed tree is also stack-safe. The byte budget is checked before parsing. Flattened frame data retains the normal JSON nesting limit, and the existing schema and sample-count validation remain in place.

Coverage includes a synthetic 1,000-frame call path and rejection of an invalid count at that depth. Replaying the untouched example through the local stacked implementation retained all 19 trees and 719 frame entries through ingestion, attachment processing, and browser rendering.

flamegraph.json

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 28b2c56. Configure here.

let mut deserializer = serde_json::Deserializer::from_slice(payload);
deserializer.disable_recursion_limit();
let raw: Flamegraph<&serde_json::value::RawValue> =
Flamegraph::deserialize(serde_stacker::Deserializer::new(&mut deserializer))?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unbounded JSON depth can exhaust memory

Medium Severity

disable_recursion_limit plus serde_stacker lets a 10 MiB flame graph nest call trees or ignored fields hundreds of thousands of levels deep. Each level keeps a grown stack segment alive until deserialize returns, so one attachment can pin hundreds of megabytes and stall or OOM a processing worker. Invalid payloads still pay this cost before rejection; the previous 128-depth cap no longer applies.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 28b2c56. Configure here.

This branch has not been deployed

No deployments
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.

4 participants