feat(vue): Implement vue browserTracingIntegration()
#10477
+549
−126
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { | ||
| browserTracingIntegration as originalBrowserTracingIntegration, | ||
| startBrowserTracingNavigationSpan, | ||
| } from '@sentry/browser'; | ||
| import type { Integration, StartSpanOptions } from '@sentry/types'; | ||
| import { instrumentVueRouter } from './router'; | ||
|
|
||
| // The following type is an intersection of the Route type from VueRouter v2, v3, and v4. | ||
| // This is not great, but kinda necessary to make it work with all versions at the same time. | ||
| export type Route = { | ||
| /** Unparameterized URL */ | ||
| path: string; | ||
| /** | ||
| * Query params (keys map to null when there is no value associated, e.g. "?foo" and to an array when there are | ||
| * multiple query params that have the same key, e.g. "?foo&foo=bar") | ||
| */ | ||
| query: Record<string, string | null | (string | null)[]>; | ||
| /** Route name (VueRouter provides a way to give routes individual names) */ | ||
| name?: string | symbol | null | undefined; | ||
| /** Evaluated parameters */ | ||
| params: Record<string, string | string[]>; | ||
| /** All the matched route objects as defined in VueRouter constructor */ | ||
| matched: { path: string }[]; | ||
| }; | ||
|
|
||
| interface VueRouter { | ||
| onError: (fn: (err: Error) => void) => void; | ||
| beforeEach: (fn: (to: Route, from: Route, next?: () => void) => void) => void; | ||
| } | ||
|
|
||
| type VueBrowserTracingIntegrationOptions = Parameters<typeof originalBrowserTracingIntegration>[0] & { | ||
| /** | ||
| * If a router is specified, navigation spans will be created based on the router. | ||
| */ | ||
| router?: VueRouter; | ||
|
|
||
| /** | ||
| * What to use for route labels. | ||
| * By default, we use route.name (if set) and else the path. | ||
| * | ||
| * Default: 'name' | ||
| */ | ||
| routeLabel?: 'name' | 'path'; | ||
| }; | ||
|
|
||
| /** | ||
| * A custom BrowserTracing integration for Vue. | ||
| */ | ||
| export function browserTracingIntegration(options: VueBrowserTracingIntegrationOptions = {}): Integration { | ||
| // If router is not passed, we just use the normal implementation | ||
| if (!options.router) { | ||
| return originalBrowserTracingIntegration(options); | ||
| } | ||
|
|
||
| const integration = originalBrowserTracingIntegration({ | ||
| ...options, | ||
| instrumentNavigation: false, | ||
| }); | ||
|
|
||
| const { router, instrumentNavigation = true, instrumentPageLoad = true, routeLabel = 'name' } = options; | ||
|
|
||
| return { | ||
| ...integration, | ||
| afterAllSetup(client) { | ||
| integration.afterAllSetup(client); | ||
|
|
||
| const startNavigationSpan = (options: StartSpanOptions): void => { | ||
| startBrowserTracingNavigationSpan(client, options); | ||
| }; | ||
|
|
||
| instrumentVueRouter(router, { routeLabel, instrumentNavigation, instrumentPageLoad }, startNavigationSpan); | ||
| }, | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { WINDOW, captureException } from '@sentry/browser'; | ||
| import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core'; | ||
| import type { SpanAttributes, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; | ||
|
|
||
| import { getActiveTransaction } from './tracing'; | ||
|
|
||
|
|
@@ -50,6 +50,8 @@ interface VueRouter { | |
| * * `routeLabel`: Set this to `route` to opt-out of using `route.name` for transaction names. | ||
| * | ||
| * @param router The Vue Router instance that is used | ||
| * | ||
| * @deprecated Use `browserTracingIntegration()` from `@sentry/vue` instead - this includes the vue router instrumentation. | ||
| */ | ||
| export function vueRouterInstrumentation( | ||
| router: VueRouter, | ||
|
|
@@ -60,88 +62,114 @@ export function vueRouterInstrumentation( | |
| startTransactionOnPageLoad: boolean = true, | ||
| startTransactionOnLocationChange: boolean = true, | ||
| ) => { | ||
| // We have to start the pageload transaction as early as possible (before the router's `beforeEach` hook | ||
| // is called) to not miss child spans of the pageload. | ||
| // We check that window & window.location exists in order to not run this code in SSR environments. | ||
| if (startTransactionOnPageLoad && WINDOW && WINDOW.location) { | ||
| startTransaction({ | ||
| name: WINDOW.location.pathname, | ||
| op: 'pageload', | ||
| attributes: { | ||
|
There was a problem hiding this comment. I'd say it's ok to stop sending this as a tag and start sending it as an attribute - I don't think anybody will depend on this too much...? Does not seem too relevant for a user. There was a problem hiding this comment. agreed. I made the same change in the SvelteKit There was a problem hiding this comment. IMO we can just remove it now that we have That was the purpose of having the |
||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| instrumentVueRouter( | ||
| router, | ||
| { | ||
| routeLabel: options.routeLabel || 'name', | ||
| instrumentNavigation: startTransactionOnLocationChange, | ||
| instrumentPageLoad: startTransactionOnPageLoad, | ||
| }, | ||
| startTransaction, | ||
| ); | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Instrument the Vue router to create navigation spans. | ||
| */ | ||
| export function instrumentVueRouter( | ||
| router: VueRouter, | ||
| options: { | ||
| routeLabel: 'name' | 'path'; | ||
| instrumentPageLoad: boolean; | ||
| instrumentNavigation: boolean; | ||
| }, | ||
| startNavigationSpanFn: (context: TransactionContext) => void, | ||
| ): void { | ||
| router.onError(error => captureException(error, { mechanism: { handled: false } })); | ||
|
|
||
| router.beforeEach((to, from, next) => { | ||
| // According to docs we could use `from === VueRouter.START_LOCATION` but I couldnt get it working for Vue 2 | ||
| // https://router.vuejs.org/api/#router-start-location | ||
| // https://next.router.vuejs.org/api/#start-location | ||
|
|
||
| // from.name: | ||
| // - Vue 2: null | ||
| // - Vue 3: undefined | ||
| // hence only '==' instead of '===', because `undefined == null` evaluates to `true` | ||
| const isPageLoadNavigation = from.name == null && from.matched.length === 0; | ||
|
|
||
| const attributes: SpanAttributes = { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.vue', | ||
| }; | ||
|
|
||
| for (const key of Object.keys(to.params)) { | ||
| attributes[`params.${key}`] = to.params[key]; | ||
| } | ||
| for (const key of Object.keys(to.query)) { | ||
| const value = to.query[key]; | ||
| if (value) { | ||
| attributes[`query.${key}`] = value; | ||
| } | ||
| } | ||
|
|
||
| // Determine a name for the routing transaction and where that name came from | ||
| let transactionName: string = to.path; | ||
| let transactionSource: TransactionSource = 'url'; | ||
| if (to.name && options.routeLabel !== 'path') { | ||
| transactionName = to.name.toString(); | ||
| transactionSource = 'custom'; | ||
| } else if (to.matched[0] && to.matched[0].path) { | ||
| transactionName = to.matched[0].path; | ||
| transactionSource = 'route'; | ||
| } | ||
|
Comment on lines
+132
to
+139
There was a problem hiding this comment. I think something here is slightly off but it's also off in the original implementation. See #10476 (comment) Should we just always send There was a problem hiding this comment. yeah, good point 🤔 do we have any place where we rely on some behavior that if There was a problem hiding this comment. afaik the only important distinction in Relay is between source There was a problem hiding this comment. I asked the ingest team if there's a difference. There was a problem hiding this comment. I guess internally we also only attach transaction names if they are not URLs to baggage. I don't know the Relay specifics either, it'll be good to double check them. There was a problem hiding this comment. I'll merge this as is (as it is the same behavior as before) and as Lukas said, we can change it in a separate PR where we can be clear about the reasoning etc! |
||
|
|
||
| if (options.instrumentPageLoad && isPageLoadNavigation) { | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| const pageloadTransaction = getActiveTransaction(); | ||
| if (pageloadTransaction) { | ||
| const existingAttributes = spanToJSON(pageloadTransaction).data || {}; | ||
| if (existingAttributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] !== 'custom') { | ||
| pageloadTransaction.updateName(transactionName); | ||
| pageloadTransaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, transactionSource); | ||
| } | ||
| // Set router attributes on the existing pageload transaction | ||
| // This will the origin, and add params & query attributes | ||
| pageloadTransaction.setAttributes({ | ||
| ...attributes, | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.vue', | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (options.instrumentNavigation && !isPageLoadNavigation) { | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = transactionSource; | ||
| startNavigationSpanFn({ | ||
| name: transactionName, | ||
| op: 'navigation', | ||
| attributes, | ||
| }); | ||
| } | ||
|
|
||
| // Vue Router 4 no longer exposes the `next` function, so we need to | ||
| // check if it's available before calling it. | ||
| // `next` needs to be called in Vue Router 3 so that the hook is resolved. | ||
| if (next) { | ||
| next(); | ||
| } | ||
| }); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@Lms24 I think I fixed the "issue" from this comment here 😅 (I check this both in the "old" format and in the new one, seems to be working in both now, hopefully.