Skip to content

Commit

Permalink
bug-1920739: update sentry_sdk to 2.17.0 (#6778)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
willkg authored Oct 31, 2024
1 parent d6a4fba commit f60fb36
Show file tree
Hide file tree
Showing 10 changed files with 31 additions and 25 deletions.
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions socorro/processor/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
11 changes: 8 additions & 3 deletions socorro/processor/processor_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions socorro/processor/rules/mozilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions socorro/stage_submitter/submitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
11 changes: 6 additions & 5 deletions socorro/tests/processor/rules/test_mozilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"


Expand Down
3 changes: 0 additions & 3 deletions socorro/tests/processor/test_cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == []
Expand Down
6 changes: 3 additions & 3 deletions socorro/tests/processor/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -86,9 +89,6 @@ def action(self, *args, **kwargs):
}
]
},
"extra": {
"rule": "socorro.tests.processor.test_pipeline.BadRule",
},
"level": "error",
"modules": ANY,
"platform": "python",
Expand Down
5 changes: 4 additions & 1 deletion socorro/tests/processor/test_processor_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit f60fb36

Please sign in to comment.