fix(sentry): read the logentry key the installed SDK fills, and mark its floor - #251
Merged
Merged
Conversation
…its floor skip_sentry=True silently stopped suppressing events at the declared floor: sentry-sdk fills logentry.formatted on newer versions but logentry.message at 2.1, and the enrichment read only the former, so the log reached Sentry with nothing failing. It now reads whichever key is present and writes back to it. sentry-sdk 2.1 also predates Python 3.13's FrameLocalsProxy, which the SDK fails to pickle while capturing a request, and 3.14 needs later fixes again. Marked per interpreter so 3.10-3.12 keep 2.1. The four remaining failures were test-side: they read _sentry_logs_handler and passed sentry_logs_level, neither of which exists below sentry-sdk 2.25. The version guard in the instrument was already correct. Refs #242
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finishes the lower-bound work on #242. That issue's two named defects were fixed by #244 and #246; this clears what was left, and the suite now passes at the declared floors on every supported interpreter.
A real bug, not just a test problem
skip_sentry=Truesilently stopped suppressing events at the declared floor.enrich_sentry_event_from_structlog_logreads the log text fromevent["logentry"]["formatted"]. That key is not universal — atsentry-sdk==2.1, which>=2.1resolves to, a log event carries its text inlogentry.messageand has noformattedkey at all:So the guard fell through, the payload was never parsed, and a log marked
skip_sentry=Truereached Sentry anyway. Nothing raised and nothing failed — the only symptom was the envelope count in one test, which is why it survived.Now reads whichever key is present and writes the rewritten message back to that same key, so the enrichment lands where the SDK will render it.
sentry-sdk's floor is wrong on 3.13 and 3.14
Separately,
sentry-sdk>=2.1predates Python 3.13'sFrameLocalsProxy:raised while the SDK captures a request. Bisected against the suite, with sanity checks either side of each boundary:
Marked rather than raised, so 3.10–3.12 keep the lower floor.
The rest was test-side
The version guard in the instrument was already correct —
SENTRY_LOGS_LEVEL_SUPPORTEDinspects the signature, so production code works at 2.1. Four tests did not:integration._sentry_logs_handler, which does not exist below sentry-sdk 2.25LoggingIntegration(sentry_logs_level=...)directly, which raises theretest_sentry_passes_sentry_logs_level_only_when_the_sdk_accepts_itforced the flagTrueand then read that same attribute — it could only ever run on an SDK that already had the parameter, which is the opposite of what it claims to checkThat last one now records the kwargs the instrument passes instead of inspecting the constructed object, so it exercises both branches on any installed SDK. The handler assertion is guarded on the flag, and the user-supplied-integration test drops a kwarg that was incidental to what it asserts.
Tests
Written first, red before the change:
Both parametrised over
formattedandmessage, so they pass on the current SDK and fail on the floor — which is the shape that would have caught the original bug.Verification
uv pip install --resolution lowest-directwith every extra, then the full suite:just lint-ciclean. Normal resolution: 329 passed.At the top of the range nothing changes — the lock still picks sentry-sdk 2.69.2 — so the floor edits are inert for anyone not installing at the lower bound.
Scope
Touches
sentry_instrument.py, its tests, and thesentry-sdkfloor only. No overlap with the Litestar work on #248.This was the last of the 14 repos in the org-wide audit (modern-python/.github#107) still failing at its lower bound.