Skip to content

Instrument LOG_EXCEPTION to observe a new Sentry disposition exception detail - #7185

Merged
jtolio merged 1 commit into
mainfrom
jolio/instrument-l-lziro4
Sep 4, 2026
Merged

Instrument LOG_EXCEPTION to observe a new Sentry disposition exception detail#7185
jtolio merged 1 commit into
mainfrom
jolio/instrument-l-lziro4

Conversation

@jtolio

@jtolio jtolio commented Aug 29, 2026

Copy link
Copy Markdown
Member

This will allow us to set NOSENTRY or SENTRY_DO or SENTRY_RT or similar on specific messages, designating how we handle these errors or what development team they go to, without adding strings to exception messages.

Also tie wdErrId fields more directly to the appropriate name.

@jtolio
jtolio requested review from a team as code owners August 29, 2026 03:58
Comment thread src/workerd/util/sentry.h Outdated
@ask-bonk

ask-bonk Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

I'm Bonk, and I've done a quick review of your PR.

Adds Sentry disposition details to LOG_EXCEPTION output and tests their logging behavior.

  1. Medium src/workerd/util/sentry.h:55 removes the compile-time-only context guard, allowing dynamic Sentry fingerprints. Posted one inline suggestion.

github run

@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch from 7004dd4 to 926896e Compare August 29, 2026 04:06
Base automatically changed from jolio/catch-leaked-kfg27x to main August 31, 2026 15:06
@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch from 926896e to a7cc7a3 Compare August 31, 2026 15:06
@ask-bonk

ask-bonk Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch from a7cc7a3 to 40c987b Compare September 1, 2026 16:34
Comment thread src/workerd/util/sentry.h Outdated
Comment thread src/workerd/util/sentry.h Outdated
Comment thread src/workerd/util/sentry.h Outdated
Comment thread src/workerd/util/sentry-test.c++ Outdated
@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch from 40c987b to 3f3578b Compare September 3, 2026 03:08
@jclee

jclee commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Seems OK to me, at least. I guess maybe worth noting that the change will probably affect the sentry fingerprints of existing exceptions, such that events will be recorded in new issues, but that's true of many code changes.

Might be worth adding a few samples of the new exception content to the internal sentry-handler library tests to verify that they submit the expected content.

Comment thread src/workerd/util/sentry.h
@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch 2 times, most recently from e7fdc98 to 329def1 Compare September 3, 2026 21:47
@jtolio

jtolio commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Seems OK to me, at least. I guess maybe worth noting that the change will probably affect the sentry fingerprints of existing exceptions, such that events will be recorded in new issues, but that's true of many code changes.

Might be worth adding a few samples of the new exception content to the internal sentry-handler library tests to verify that they submit the expected content.

Both issues fixed (fingerprint changes avoided and tests added) on this internal MR: https://gitlab.cfdata.org/cloudflare/ew/edgeworker/-/merge_requests/14737

(sorry to outside viewers for not getting to see)

@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch from 329def1 to 6ad4b16 Compare September 4, 2026 02:55
…n detail

This will allow us to set NOSENTRY or SENTRY_DO or SENTRY_RT or
similar on specific messages, designating how we handle these
errors or what development team they go to, without adding strings
to exception messages.

Also tie wdErrId fields more directly to the appropriate name.
@jtolio
jtolio force-pushed the jolio/instrument-l-lziro4 branch from 6ad4b16 to 699bbd3 Compare September 4, 2026 09:58
@jtolio

jtolio commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

trivial rebases ^

@jtolio
jtolio merged commit 7726c8e into main Sep 4, 2026
31 of 33 checks passed
@jtolio
jtolio deleted the jolio/instrument-l-lziro4 branch September 4, 2026 10:53
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.

4 participants