Conversation
size-limit report 📦
|
Lms24
left a comment
There was a problem hiding this comment.
I like this new and simplified API a lot!
| */ | ||
| export function trace<T>( | ||
| context: TransactionContext, | ||
| callback: (span: Span) => T, |
There was a problem hiding this comment.
super-l: WDYT about renaming callback to something like traceTarget / traceSubject or something along these lines? The way I understand this function, we want to start a txn or a span for this callback so I'd say we should give it a good name. callback sounds a little generic to me.
But ofc we can (and probably will) iterate on this function in the future, so really, feel free to disregard this comment.
There was a problem hiding this comment.
I'm going to use callback for now since this is the language we are using for the cross SDK discussions about this, but you do make a good point. I'll come back and re-visit this after we chat about it as a group.
|
|
||
| it('creates a transaction', async () => { | ||
| let ref: any = undefined; | ||
| client.on('finishTransaction', transaction => { |
There was a problem hiding this comment.
Making good use of hooks - I love it 😄
|
Just saw this! I’m really glad you were able to refine some of the ideas from the gist I posted back a few weeks ago in #7387 (comment). Are there any plans to eventually make this public? I would love to be using this built-in functionality instead of hacking it in from the outside like I’ve been doing. |
Sentry.traceis a wrapper that abstracts away much of the complexities that come with managing spans/transaction. You can wrap a method, and not worry about things being sync/async, all of it will be handled for you.You can also pass in a
onErrorcallback, that can process errors (but do not actually suppress the error - users can manually do that themselves).This was introduced to make adding Sveltekit performance monitoring more simple. As I was adding code for #7536 and #7537, I noticed that we duplicate this span creation/finishing logic all over the code base.
look at something like
sentry-javascript/packages/tracing-internal/src/node/integrations/graphql.ts
Line 37 in 72dca3e
or
sentry-javascript/packages/tracing-internal/src/node/integrations/mysql.ts
Line 44 in 72dca3e
It is marked as internal and private API - so we should have the freedom to break this, but I suspect the API will not change.
Ideally this means every single implementation here can just use the
Sentry.tracemethod: https://github.com/getsentry/sentry-javascript/tree/develop/packages/tracing-internal/src/node/integrationsThis breaks for client-side environments since scope propagation is busted in browser, but with domains is fine for node, so lets