Skip to content

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 AbhiPrasad, Lms24 and lforst March 12, 2024 14:52
@mydea mydea self-assigned this Mar 12, 2024
{
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

/** 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.

4 participants