-
Notifications
You must be signed in to change notification settings - Fork 0
Bump sentry-sdk from 0.16.1 to 1.14.0 #296
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
Conversation
Bumps [sentry-sdk](https://github.com/getsentry/sentry-python) from 0.16.1 to 1.14.0. - [Release notes](https://github.com/getsentry/sentry-python/releases) - [Changelog](https://github.com/getsentry/sentry-python/blob/master/CHANGELOG.md) - [Commits](getsentry/sentry-python@0.16.1...1.14.0) --- updated-dependencies: - dependency-name: sentry-sdk dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## development #296 +/- ##
============================================
Coverage 67.59% 67.59%
============================================
Files 31 31
Lines 4154 4154
============================================
Hits 2808 2808
Misses 1346 1346 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
jlchang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eweitz if the build passes, what else is necessary before merging this PR? sentry-sdk is used in monitor.py but I don't know if we have any tests that explicitly check for Sentry reporting in this repo.
If we approve this PR, do we need to test in staging to ensure that an expected ingest failure mode gets reported in non-prod Sentry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we approve this PR, do we need to test in staging to ensure that an expected ingest failure mode gets reported in non-prod Sentry?
We could, but I don't think we need to. The worst case I can reasonably envision from deferring such a check is that Ingest Pipeline stops logging unhandled errors to Sentry. I see no errors logged from Ingest to Sentry in the last 90 days. And IIUC, we also log unhandled errors in delocalized GCS files and Rails local logs, and log handled errors / user validation errors in Mixpanel, so the segment of unlogged errors seems like it would be small (if anything) in that worst case. That worst case strikes me as unlikely; breaking changes in 1.0.0 don't seem likely to affect our usage.
Given that, while addressing within 30 days this security issue marked as high severity is important for security compliance, specific manual regression testing here seems less essential.
Reading the security report indicates the issue requires multiple conditions that aren't used in this project. So while merging and deploying this update seems like it'd be fine, it seems unlikely we'd ever meet the conditions for this issue to manifest -- e.g. we're prohibited from setting sendDefaultPII set to True for pre-existing reasons.
jlchang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eweitz Thanks for walking me through how to evaluate a security update PR. It's completely obvious, in retrospect, that this PR can be evaluated in the context of 1. what breaking changes are involved in the update and 2. what the security report is concerned about.
I really appreciate the link to the Sentry report showing that we no longer get logger:ingest-pipeline errors - I'll make a note to investigate as the mixpanel data suggests those error still occur. At least we won't attribute those errors to this update!
Bumps sentry-sdk from 0.16.1 to 1.14.0.
Release notes
Sourced from sentry-sdk's releases.
... (truncated)
Changelog
Sourced from sentry-sdk's changelog.
... (truncated)
Commits
8c4a19aUpdated changelogf095df7release: 1.14.0d27808fRemoved code coverage target (#1862)cd2f51bfeat(profiling): Add profile context to transaction (#1860)d515233Always remove Django session related cookies. (#1842)032ea57Make sure to noop when there is no DSN (#1852)086e385feat(profiling): Use co_qualname in python 3.11 (#1831)0714d9fFix middleware being patched multiple times when using FastAPI (#1841)1ac27c8fix(opentelemetry): Use dict for sentry-trace context instead of tuple (#1847)504188cfix extra dependency (#1825)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)You can disable automated security fix PRs for this repo from the Security Alerts page.