-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref: Remove deprecated properties from startSpan
options
#11054
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
@@ -68,7 +68,6 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a | |||
{ | |||
op: 'function.nextjs', | |||
name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`, | |||
data, |
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.
This looked sus but I saw that we also set it as extra on the scope.
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.
jup, that's why I removed it, this should remain the same on the event as far as I can tell! (it is also not flat so cannot be attributes directly, we'd need to flatten this but I don't think that's worth it here)
0a5ed80
to
308a8e1
Compare
@@ -35,66 +33,4 @@ export interface StartSpanOptions extends TransactionContext { | |||
|
|||
/** Attributes for the span. */ | |||
attributes?: SpanAttributes; | |||
|
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/m: Is there a reason for not yet removing origin
? It's also deprecated.
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.
btw totally fine if you wanna tackle this in a follow-up PR; Just noticed it
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.
Yeah I'll do that in a follow up I think as we have more usages of this I believe! :)
308a8e1
to
a43374a
Compare
So the public API for this is now fully aligned!
also remove unneeded stuff
a43374a
to
ce8d08b
Compare
So the public API for this is now fully aligned!
In nextjs, I moved places where we passed
request
as metadata to put this on the isolation scope (in some places we already did that anyhow).