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: Read thread names from minidumps and Apple crash reports #834

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Jun 14, 2022

fixes #833

@loewenheim loewenheim requested a review from a team June 14, 2022 13:47
@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2022

Warnings
⚠️ Snapshot changes likely affect Sentry tests. If the Sentry-Symbolicator Integration Tests in CI are failing for your PR, please check the symbolicator test suite in Sentry and update snapshots as needed.
Instructions for snapshot changes

Sentry runs a symbolicator integration test suite located at tests/symbolicator/. Changes in this PR will likely result in snapshot diffs in Sentry, which will break the master branch and in-progress PRs.

Follow these steps to update snapshots in Sentry:

  1. Check out latest Sentry master and enable the virtualenv.
  2. Enable symbolicator (symbolicator: true) in sentry via ~/.sentry/config.yml.
  3. Make sure your other devservices are running via sentry devservices up --exclude symbolicator. If
    they're already running, stop symbolicator with sentry devservices down symbolicator. You want to use your
    own development symbolicator to update the snapshots.
  4. Run your development symbolicator on port 3021, or whatever port symbolicator is configured to use
    in ~/.sentry/config.yml.
  5. Export SENTRY_SNAPSHOTS_WRITEBACK=1 to automatically update the existing snapshots with your new
    results and run symbolicator tests with pytest (pytest tests/symbolicator).
  6. Review snapshot changes locally, then create a PR to Sentry.
  7. Merge the Symbolicator PR, then merge the Sentry PR.

Generated by 🚫 dangerJS against 96f734f

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

🎉 lets add an integration test as a followup once we actually have a minidump with thread names in it ;-)

@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2022

Codecov Report

Merging #834 (96f734f) into master (5452162) will increase coverage by 0.04%.
The diff coverage is 73.92%.

@@            Coverage Diff             @@
##           master     #834      +/-   ##
==========================================
+ Coverage   75.08%   75.13%   +0.04%     
==========================================
  Files          49       50       +1     
  Lines       10862    11095     +233     
==========================================
+ Hits         8156     8336     +180     
- Misses       2706     2759      +53     
Impacted Files Coverage Δ
crates/symbolicator/src/services/bitcode.rs 83.25% <ø> (-0.08%) ⬇️
...rates/symbolicator/src/services/download/sentry.rs 22.11% <0.00%> (ø)
crates/symbolicator/src/services/shared_cache.rs 42.70% <ø> (+0.09%) ⬆️
crates/symbolicator/src/sources.rs 88.70% <0.00%> (-0.04%) ⬇️
crates/symbolicator/src/types/mod.rs 58.84% <ø> (ø)
crates/symbolicator/src/services/il2cpp.rs 71.26% <71.26%> (ø)
crates/symbolicator/src/utils/paths.rs 75.19% <80.00%> (+0.24%) ⬆️
crates/symbolicator/src/services/symcaches/mod.rs 93.26% <85.41%> (-1.26%) ⬇️
crates/symbolicator/src/cache.rs 96.87% <100.00%> (+0.03%) ⬆️
crates/symbolicator/src/services/mod.rs 95.58% <100.00%> (+0.13%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 626221b...96f734f. Read the comment docs.

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.

Extract Thread Names from Minidumps
3 participants