-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Move console integration into core and add to cloudflare/vercel-edge #16024
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
size-limit report 📦
|
Co-authored-by: Francesco Gringl-Novy <[email protected]>
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.
I think this is a worthwhile change! Nice!
it('handles different console levels correctly', () => { | ||
const levels = ['debug', 'info', 'warn', 'error'] as const; |
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.
l/nit: I'd rather use it.each
so that we'd know for which specific level the test failed.
Also q: I noticed this pattern in quite a few tests (not yours, more generally in the repo) and was wondering: Am I maybe missing something why this is preferrable over it.each
? 😅
Again, not a blocker, feel free to ignore
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.
Also q: I noticed this pattern in quite a few tests (not yours, more generally in the repo) and was wondering: Am I maybe missing something why this is preferrable over it.each? 😅
I've been leaning less toward using it.each
because it's easier for me to it.only
test one at a time, but I agree the table test is the better pattern here. Will change.
resolves #15439
resolves #4532
resolves https://linear.app/getsentry/issue/JSC-192
Supercedes #16021
Our console instrumentation was always inconsistent, but adding breadcrumbs should not really be. This PR adds a unified console instrumentation to
@sentry/core
, and makes the Node SDK use that. We also add this to thevercel-edge
andcloudflare
SDKs.I also left todo comments in the deno and browser SDKs for us to unify into this single integration afterwards.