Skip to content

ref: attachment manifest - #2108

Open
jpnurmi wants to merge 1 commit into
masterfrom
jpnurmi/ref/attachment-manifest
Open

jpnurmi wants to merge 1 commit into
masterfrom
jpnurmi/ref/attachment-manifest

Conversation

@jpnurmi

@jpnurmi jpnurmi commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Use a shared MessagePack stream for attachment manifests. The native backend uses the manifest for attachments generally, while both out-of-process crash handlers will need to use it for crash-time hint attachments (#2099) that cannot cross backend IPC. This lets Crashpad consume those attachments with the existing MessagePack reader, without having to introduce JSON support/dependencies.

Stream attachment objects directly to avoid format-specific conversion and buffer the complete manifest for a single file write. Median release benchmarks measured write/read improvements over legacy JSON of 17%/6% for one attachment, 74%/17% for 10, and 86%/24% for 100:

1 attachment 10 attachments 100 attachments
MessagePack write 2.3 us 2.4 us 11.8 us
JSON write 2.7 us 8.9 us 82.6 us
MessagePack read 1.3 us 5.6 us 54.1 us
JSON read 1.4 us 6.8 us 71.1 us

Keep legacy JSON compatibility private to the native daemon so it can still consume manifests written by older SDK versions.

@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.

Reviewed by Cursor Bugbot for commit d8eb6bb. Configure here.

Comment thread src/backends/sentry_backend_native.c Outdated
Comment thread src/sentry_attachment.c
Use a shared MessagePack stream for attachment manifests. The native
backend uses the manifest for attachments generally, while both
out-of-process crash handlers can also use it for crash-time hint
attachments that cannot cross backend IPC. This lets Crashpad consume
those attachments without adding JSON support.

Stream attachment objects directly to avoid format-specific conversion
and buffer the complete manifest for a single file write. Median release
benchmarks measured write/read improvements over legacy JSON of 17%/6%
for one attachment, 74%/17% for 10, and 86%/24% for 100:

                    1 attachment   10 attachments   100 attachments
  MessagePack write       2.3 us           2.4 us            11.8 us
  JSON write              2.7 us           8.9 us            82.6 us
  MessagePack read        1.3 us           5.6 us            54.1 us
  JSON read               1.4 us           6.8 us            71.1 us

Keep legacy JSON compatibility private to the native daemon so it can
still consume manifests written by older SDK versions.
@jpnurmi
jpnurmi force-pushed the jpnurmi/ref/attachment-manifest branch from d8eb6bb to f0d7bcf Compare September 17, 2026 16:51
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.61538% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.53%. Comparing base (4d570f3) to head (f0d7bcf).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2108      +/-   ##
==========================================
- Coverage   74.83%   74.53%   -0.30%     
==========================================
  Files         103      103              
  Lines       27386    27398      +12     
  Branches     4944     4955      +11     
==========================================
- Hits        20494    20421      -73     
- Misses       5543     5641      +98     
+ Partials     1349     1336      -13     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant