Skip to content

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

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Mar 12, 2024

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).

@mydea mydea requested review from lforst, Lms24 and AbhiPrasad March 12, 2024 14:52
@mydea mydea self-assigned this Mar 12, 2024
@@ -68,7 +68,6 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
{
op: 'function.nextjs',
name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`,
data,
Copy link
Contributor

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.

Copy link
Member Author

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)

@mydea mydea force-pushed the fn/start-span-options branch from 0a5ed80 to 308a8e1 Compare March 12, 2024 15:11
@@ -35,66 +33,4 @@ export interface StartSpanOptions extends TransactionContext {

/** Attributes for the span. */
attributes?: SpanAttributes;

Copy link
Member

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.

Copy link
Member

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

Copy link
Member Author

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! :)

@mydea mydea force-pushed the fn/start-span-options branch from 308a8e1 to a43374a Compare March 12, 2024 16:18
mydea added 2 commits March 13, 2024 10:48
So the public API for this is now fully aligned!
also remove unneeded stuff
@mydea mydea force-pushed the fn/start-span-options branch from a43374a to ce8d08b Compare March 13, 2024 10:48
@mydea mydea merged commit 3e1c45d into develop Mar 13, 2024
@mydea mydea deleted the fn/start-span-options branch March 13, 2024 11:22
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.

3 participants