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(native): add a compressed minidump endpoint example #11304

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

supervacuus
Copy link
Collaborator

@supervacuus supervacuus commented Sep 10, 2024

DESCRIBE YOUR PR

The docs currently do not describe how the minidump endpoint can be used when sending compressed payloads. This is often necessary when customers produce minidumps bigger than our upload limits.

Fixes #7575.

IS YOUR CHANGE URGENT?

Help us prioritize incoming PRs by letting us know when the change needs to go live.

  • Urgent deadline (GA date, etc.):
  • Other deadline:
  • None: Not urgent, can wait up to 1 week+

SLA

  • Teamwork makes the dream work, so please add a reviewer to your PRs.
  • Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it.
    Thanks in advance for your help!

PRE-MERGE CHECKLIST

Make sure you've checked the following before merging your changes:

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs (subject matter experts)
  • PR was reviewed and approved by a member of the Sentry docs team

Copy link

vercel bot commented Sep 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
changelog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 8:59pm
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 8:59pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
develop-docs ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 8:59pm

@supervacuus
Copy link
Collaborator Author

I added @markushi and @JoshuaMoelans since the basic compressed example is relatively simple, but adding meta-data is quite involved.

I would love to know if the steps and reasoning are understandable, provided the user is technically sophisticated enough to use curl with a multipart endpoint.

docs/platforms/native/guides/minidumps/index.mdx Outdated Show resolved Hide resolved
docs/platforms/native/guides/minidumps/index.mdx Outdated Show resolved Hide resolved
supervacuus and others added 2 commits September 10, 2024 22:25
Co-authored-by: vivianyentran <20403606+vivianyentran@users.noreply.github.com>
Co-authored-by: vivianyentran <20403606+vivianyentran@users.noreply.github.com>
Copy link

codecov bot commented Sep 10, 2024

Bundle Report

Changes will decrease total bundle size by 15 bytes (-0.0%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server 7.44MB 6 bytes ⬇️
sentry-docs-edge-server 254.43kB 3 bytes ⬇️
sentry-docs-client 6.38MB 6 bytes ⬇️

Copy link
Member

@JoshuaMoelans JoshuaMoelans left a comment

Choose a reason for hiding this comment

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

Steps are reasonable (even for someone who's new to this).

Error below was resolved
(turns out I used ” instead of ")


I am wondering if this warning on the dashboard below is supposed to show up?
Screenshot 2024-09-11 at 14 34 54


# Add additional data via JSON or using the bracket syntax (the latter requiring a field for each entry)
printf -- 'Content-Disposition: form-data; name="sentry"\r\n\r\n' >> multipart_body.txt
printf -- '{"release":"my-project-name@2.3.12","tags":{"mytag":"value"}}\r\n' >> multipart_body.txt
Copy link
Member

Choose a reason for hiding this comment

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

My first time going through these steps I didn't change this "release" to the actual version that the .dmp file was built for, which I think is the reason I got an {"detail":"invalid minidump"}% result.

Not sure if the advanced users that would use this method of sending minidumps would make the same mistake, but still might be something worth highlighting.

Other than that, these instructions seem fine as a beginner's guide.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My first time going through these steps I didn't change this "release" to the actual version that the .dmp file was built for, which I think is the reason I got an {"detail":"invalid minidump"}% result.

The main reason you could get an "invalid minidump" (besides the obvious attaching an invalid minidump file 😅) is probably due to bad escaping or delimiting characters because those could prevent the receiving parser from "seeing" the minidump field.

Correctly formed meta-data shouldn't affect the validity of the minidump (i.e., Sentry doesn't cross-check the release in the sentry field with the contents of the minidump).

This is why documenting this as a solution is a double-edged sword. Manually constructing a multipart body can quickly introduce encoding errors like turns out I used ” instead of ". Thankfully, that error appeared when parsing the JSON metadata (with a slightly better error message).

But if the encoding error appears beyond the field value boundary (forgetting the correct number of \r\n and similar), you might get the results of a wrongly parsed HTTP request.

I think the best solution to the problem would be to let relay

  • check if the file uploaded as upload_file_minidump is a gzip and in that case
  • unzip the file and then validate it as a minidump and continue processing as it does now

This would allow users to apply the same upload methods as described in the previous sections, but instead of uploading a mini.dmp, they would upload a mini.dmp.gz (identified via the first n-byte header).

Until that solution is implemented and deployed, users have a documented approach for uploading minidumps manually.

@supervacuus
Copy link
Collaborator Author

Following the conversation here: #11304 (comment) I created a Relay-PR (getsentry/relay#4029) that would reduce this documentation change to:

If you want to compress your minidump, you can gzip it and upload the *.gz instead. All previous examples apply.

Let's wait before merging if the Relay folk consider this a sensible idea.

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.

Add compressed minidump example.
3 participants