-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(webvitals): Add profile id, replay id, and user to standalone INP spans #10849
Changes from 2 commits
0559eab
bbdc8f0
e5f21ab
8b929bc
65bb1ac
ac749fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
/* eslint-disable max-lines */ | ||
import type { IdleTransaction } from '@sentry/core'; | ||
import { getActiveSpan } from '@sentry/core'; | ||
import { getActiveSpan, getClient, getCurrentScope } from '@sentry/core'; | ||
import { getCurrentHub } from '@sentry/core'; | ||
import { | ||
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, | ||
|
@@ -12,6 +12,7 @@ import { | |
} from '@sentry/core'; | ||
import type { | ||
Client, | ||
Integration, | ||
IntegrationFn, | ||
StartSpanOptions, | ||
Transaction, | ||
|
@@ -539,6 +540,15 @@ function registerInpInteractionListener( | |
}, | ||
): void { | ||
addPerformanceInstrumentationHandler('event', ({ entries }) => { | ||
const client = getClient(); | ||
// We need to get the replay, user, and activeTransaction from the current scope | ||
// so that we can associate replay id, profile id, and a user display to the span | ||
const replay = client?.getIntegrationByName?.('Replay') as | ||
| (Integration & { getReplayId: () => string }) | ||
| undefined; | ||
// eslint-disable-next-line deprecation/deprecation | ||
const activeTransaction = getActiveTransaction(); | ||
const user = getCurrentScope()?.getUser(); | ||
for (const entry of entries) { | ||
if (isPerformanceEventTiming(entry)) { | ||
const duration = entry.duration; | ||
|
@@ -564,6 +574,9 @@ function registerInpInteractionListener( | |
routeName, | ||
duration, | ||
parentContext, | ||
user, | ||
activeTransaction, | ||
replay, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we should pass in the id's here instead of holding references to replay integration/active transaction There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should be able to update this for replay ID. For profiles, the profiling integration only attaches the profile id to the active transaction when the transaction finishes. So we don't immediately have access to a profile id when an interaction occurs. Keeping a reference to the transaction seemed like the easiest way to grab the profile id when we're ready to send an INP. Not sure if there is a better solution, but I can think about it if this is a blocker There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh I see, yeah let's hold the transaction reference then, np |
||
}; | ||
} | ||
} | ||
|
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.
We're putting
profile_id
as a top level field because relay expects this