From f60fb3622f184f254ae45a81a99df63c3a3b24e5 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Thu, 31 Oct 2024 11:06:18 -0400 Subject: [PATCH] bug-1920739: update sentry_sdk to 2.17.0 (#6778) This updates sentry_sdk to 2.17.0. In order to do that, we had to do two fundamental changes: 1. Switch `sentry_sdk.push_scope()` calls to `sentry_sdk.new_scope()` which does roughly the same thing. 2. Switch from using `sentry_sdk.add_extra()` to `sentry_sdk.set_context()`. `add_extra()` was nice in that it allowed us to add additional things to the sentry event payload that gave context to the error. We used this to capture the processor rule, ruleset, crash_id, signature generation rule, etc in an additive way. `add_extra()` is deprecated and Sentry suggests to switch to `set_context()` which is not an additive kind of thing. So now we have to set a new context section everywhere we set the context. That's a little annoying, but it'll be fine. I updated the code and tests accordingly. --- requirements.in | 2 +- requirements.txt | 6 +++--- socorro/processor/pipeline.py | 4 ++-- socorro/processor/processor_app.py | 11 ++++++++--- socorro/processor/rules/mozilla.py | 4 ++-- socorro/stage_submitter/submitter.py | 4 ++-- socorro/tests/processor/rules/test_mozilla.py | 11 ++++++----- socorro/tests/processor/test_cache_manager.py | 3 --- socorro/tests/processor/test_pipeline.py | 6 +++--- socorro/tests/processor/test_processor_app.py | 5 ++++- 10 files changed, 31 insertions(+), 25 deletions(-) diff --git a/requirements.in b/requirements.in index 8eba4c451f..162a3f7e19 100644 --- a/requirements.in +++ b/requirements.in @@ -48,7 +48,7 @@ requests==2.32.3 requests-mock==1.12.1 ruff==0.7.1 semver==3.0.2 -sentry-sdk==2.8.0 +sentry-sdk==2.17.0 Sphinx==8.1.3 sphinx_rtd_theme==3.0.1 statsd==4.0.1 diff --git a/requirements.txt b/requirements.txt index 3aabf0656c..66bebcf863 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1236,9 +1236,9 @@ semver==3.0.2 \ --hash=sha256:6253adb39c70f6e51afed2fa7152bcd414c411286088fb4b9effb133885ab4cc \ --hash=sha256:b1ea4686fe70b981f85359eda33199d60c53964284e0cfb4977d243e37cf4bf4 # via -r requirements.in -sentry-sdk==2.8.0 \ - --hash=sha256:6051562d2cfa8087bb8b4b8b79dc44690f8a054762a29c07e22588b1f619bfb5 \ - --hash=sha256:aa4314f877d9cd9add5a0c9ba18e3f27f99f7de835ce36bd150e48a41c7c646f +sentry-sdk==2.17.0 \ + --hash=sha256:625955884b862cc58748920f9e21efdfb8e0d4f98cca4ab0d3918576d5b606ad \ + --hash=sha256:dd0a05352b78ffeacced73a94e86f38b32e2eae15fff5f30ca5abb568a72eacf # via # -r requirements.in # fillmore diff --git a/socorro/processor/pipeline.py b/socorro/processor/pipeline.py index c5379385f4..c840ba0191 100644 --- a/socorro/processor/pipeline.py +++ b/socorro/processor/pipeline.py @@ -87,8 +87,8 @@ def process_crash(self, ruleset_name, raw_crash, dumps, processed_crash, tmpdir) # Apply rules; if a rule fails, capture the error and continue onward for rule in ruleset: - with sentry_sdk.push_scope() as scope: - scope.set_extra("rule", rule.name) + with sentry_sdk.new_scope() as scope: + scope.set_context("processor_pipeline", {"rule": rule.name}) try: rule.act( diff --git a/socorro/processor/processor_app.py b/socorro/processor/processor_app.py index 8444e03ac4..ba775315a2 100755 --- a/socorro/processor/processor_app.py +++ b/socorro/processor/processor_app.py @@ -130,9 +130,14 @@ def transform(self, task, finished_func=(lambda: None)): with METRICS.timer( "processor.process_crash", tags=[f"ruleset:{ruleset_name}"] ): - with sentry_sdk.push_scope() as scope: - scope.set_extra("crash_id", crash_id) - scope.set_extra("ruleset", ruleset_name) + with sentry_sdk.new_scope() as scope: + scope.set_context( + "processor", + { + "crash_id": crash_id, + "ruleset": ruleset_name, + }, + ) # Create temporary directory context with tempfile.TemporaryDirectory(dir=self.temporary_path) as tmpdir: diff --git a/socorro/processor/rules/mozilla.py b/socorro/processor/rules/mozilla.py index a0217dc51b..938fc190ee 100644 --- a/socorro/processor/rules/mozilla.py +++ b/socorro/processor/rules/mozilla.py @@ -1041,8 +1041,8 @@ def __init__(self): def _error_handler(self, crash_data, exc_info, extra): """Captures errors from signature generation""" - with sentry_sdk.push_scope() as scope: - scope.set_extra("signature_rule", extra["rule"]) + with sentry_sdk.new_scope() as scope: + scope.set_context("signature_generator", {"signature_rule": extra["rule"]}) sentry_sdk.capture_exception(exc_info) def action(self, raw_crash, dumps, processed_crash, tmpdir, status): diff --git a/socorro/stage_submitter/submitter.py b/socorro/stage_submitter/submitter.py index 8f51168669..a5ba7a4229 100644 --- a/socorro/stage_submitter/submitter.py +++ b/socorro/stage_submitter/submitter.py @@ -333,11 +333,11 @@ def sample(self, destinations): def process(self, crash): with METRICS.timer("submitter.process"): - with sentry_sdk.push_scope() as scope: + with sentry_sdk.new_scope() as scope: crash_id = crash.crash_id self.logger.debug(f"processing {crash}") - scope.set_extra("crash_id", crash) + scope.set_context("submitter", {"crash_id": crash_id}) # sample and determine destinations destinations = [] diff --git a/socorro/tests/processor/rules/test_mozilla.py b/socorro/tests/processor/rules/test_mozilla.py index e589ac2eed..ac36529e99 100644 --- a/socorro/tests/processor/rules/test_mozilla.py +++ b/socorro/tests/processor/rules/test_mozilla.py @@ -2054,7 +2054,7 @@ def predicate(self, raw_crash, processed_crash): # doesn't configure Sentry the way the processor does so we shouldn't test # whether things are scrubbed correctly with sentry_helper.init() as sentry_client: - # Override the regular SigntureGenerator with one with a BadRule + # Override the regular SignatureGenerator with one with a BadRule # in the pipeline rule.generator = SignatureGenerator( ruleset=[BadRule], error_handler=rule._error_handler @@ -2073,10 +2073,11 @@ def predicate(self, raw_crash, processed_crash): assert status.notes == ["BadRule: Rule failed: Cough"] (event,) = sentry_client.envelope_payloads - # NOTE(willkg): Some of the extra bits come from the processor app and since - # we're testing SignatureGenerator in isolation, those don't get added to - # the sentry scope - assert event["extra"] == {"signature_rule": "BadRule", "sys.argv": mock.ANY} + + # Assert that the rule that threw an error is captured in the context. + assert event["contexts"]["signature_generator"] == { + "signature_rule": "BadRule" + } assert event["exception"]["values"][0]["type"] == "Exception" diff --git a/socorro/tests/processor/test_cache_manager.py b/socorro/tests/processor/test_cache_manager.py index 75c3d9ddfc..af4d958ca4 100644 --- a/socorro/tests/processor/test_cache_manager.py +++ b/socorro/tests/processor/test_cache_manager.py @@ -470,9 +470,6 @@ def mock_make_room(*args, **kwargs): (event,) = sentry_client.envelope_payloads - # Drop the "_meta" bit because we don't want to compare that. - del event["_meta"] - # Assert that the event is what we expected differences = diff_structure(event, BROKEN_EVENT) assert differences == [] diff --git a/socorro/tests/processor/test_pipeline.py b/socorro/tests/processor/test_pipeline.py index 42f714ca73..46bca9ca1e 100644 --- a/socorro/tests/processor/test_pipeline.py +++ b/socorro/tests/processor/test_pipeline.py @@ -36,6 +36,9 @@ def action(self, *args, **kwargs): "span_id": ANY, "trace_id": ANY, }, + "processor_pipeline": { + "rule": "socorro.tests.processor.test_pipeline.BadRule", + }, }, "environment": "production", "event_id": ANY, @@ -86,9 +89,6 @@ def action(self, *args, **kwargs): } ] }, - "extra": { - "rule": "socorro.tests.processor.test_pipeline.BadRule", - }, "level": "error", "modules": ANY, "platform": "python", diff --git a/socorro/tests/processor/test_processor_app.py b/socorro/tests/processor/test_processor_app.py index 53e18296c1..704222f2e5 100644 --- a/socorro/tests/processor/test_processor_app.py +++ b/socorro/tests/processor/test_processor_app.py @@ -176,6 +176,10 @@ def test_transform_unexpected_exception(self, processor_settings): "span_id": ANY, "trace_id": ANY, }, + "processor": { + "crash_id": ANY, + "ruleset": "default", + }, }, "environment": "production", "event_id": ANY, @@ -234,7 +238,6 @@ def test_transform_unexpected_exception(self, processor_settings): } ] }, - "extra": {"crash_id": "930b08ba-e425-49bf-adbd-7c9172220721", "ruleset": "default"}, "level": "error", "modules": ANY, "platform": "python",