fix(vue): Update Vue trackComponents list to match components with or without <>
#13543
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,3 +122,101 @@ test('sends a pageload transaction with a route name as transaction name if avai | |
| }, | ||
| }); | ||
| }); | ||
|
|
||
| test('sends a lifecycle span for the tracked HomeView component - with `<>`', async ({ page }) => { | ||
| const transactionPromise = waitForTransaction('vue-3', async transactionEvent => { | ||
| return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; | ||
| }); | ||
|
|
||
| await page.goto(`/`); | ||
|
|
||
| const rootSpan = await transactionPromise; | ||
|
|
||
| expect(rootSpan).toMatchObject({ | ||
| contexts: { | ||
| trace: { | ||
| data: { | ||
| 'sentry.source': 'route', | ||
| 'sentry.origin': 'auto.pageload.vue', | ||
| 'sentry.op': 'pageload', | ||
| }, | ||
| op: 'pageload', | ||
| origin: 'auto.pageload.vue', | ||
| }, | ||
| }, | ||
| spans: expect.arrayContaining([ | ||
| // enabled by default | ||
| expect.objectContaining({ | ||
| data: { | ||
| 'sentry.op': 'ui.vue.render', | ||
| 'sentry.origin': 'auto.ui.vue', | ||
| }, | ||
| description: 'Application Render', | ||
| op: 'ui.vue.render', | ||
| origin: 'auto.ui.vue', | ||
| }), | ||
| // enabled by default | ||
| expect.objectContaining({ | ||
| data: { | ||
| 'sentry.op': 'ui.vue.mount', | ||
| 'sentry.origin': 'auto.ui.vue', | ||
| }, | ||
| description: 'Vue <<Root>>', | ||
| op: 'ui.vue.mount', | ||
| origin: 'auto.ui.vue', | ||
| }), | ||
| expect.objectContaining({ | ||
| data: { | ||
| 'sentry.op': 'ui.vue.mount', | ||
| 'sentry.origin': 'auto.ui.vue', | ||
| }, | ||
| description: 'Vue <<HomeView>>', | ||
|
There was a problem hiding this comment. Just making sure, are the double brackets intended? They're formatted here. There was a problem hiding this comment. Good observation! I don't think they're intended. Let's remove them. |
||
| op: 'ui.vue.mount', | ||
| origin: 'auto.ui.vue', | ||
| }), | ||
| ]), | ||
| transaction: '/', | ||
| transaction_info: { | ||
| source: 'route', | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| test('sends a lifecycle span for the tracked UserIdErrorView component - without `<>`', async ({ page }) => { | ||
| const transactionPromise = waitForTransaction('vue-3', async transactionEvent => { | ||
| return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; | ||
| }); | ||
|
|
||
| await page.goto(`/users-error/123`); | ||
|
|
||
| const rootSpan = await transactionPromise; | ||
|
|
||
| expect(rootSpan).toMatchObject({ | ||
| contexts: { | ||
| trace: { | ||
| data: { | ||
| 'sentry.source': 'route', | ||
| 'sentry.origin': 'auto.pageload.vue', | ||
| 'sentry.op': 'pageload', | ||
| }, | ||
| op: 'pageload', | ||
| origin: 'auto.pageload.vue', | ||
| }, | ||
| }, | ||
| spans: expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| data: { | ||
| 'sentry.op': 'ui.vue.mount', | ||
| 'sentry.origin': 'auto.ui.vue', | ||
| }, | ||
| description: 'Vue <<UserIdErrorView>>', | ||
| op: 'ui.vue.mount', | ||
| origin: 'auto.ui.vue', | ||
| }), | ||
| ]), | ||
| transaction: '/users-error/:id', | ||
| transaction_info: { | ||
| source: 'route', | ||
| }, | ||
| }); | ||
| }); | ||
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 |
|---|---|---|
|
|
@@ -46,14 +46,8 @@ function finishRootSpan(vm: VueSentry, timestamp: number, timeout: number): void | |
| }, timeout); | ||
| } | ||
|
|
||
| /** Find if the current component exists in the provided `TracingOptions.trackComponents` array option. */ | ||
| export function findTrackComponent(trackComponents: string[], formattedName: string): boolean { | ||
| function extractComponentName(name: string): string { | ||
| return name.replace(/^<([^\s]*)>(?: at [^\s]*)?$/, '$1'); | ||
|
There was a problem hiding this comment. Based on what you suggested @Lms24, just an added non-capture group. The ReDoS Checker passes. |
||
| } | ||
|
|
||
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.
This is a bit of an unfortunate and very intransparent limitation of our e2e test setup but we shouldn't navigate to the same route in more than one e2e test. The reason is that we execute them in parallel in CI and the
waitForTransactioncall from one test might actually resolve for a transaction in another test, depending on the condition in the callback.To avoid this confusion, we generally create a new route for each test. Suggestion: Either we create new routes for the two new tests or we integrate them in already existing tests and simply check for the spans there. I'll let you make the call, I'm fine with either option.
Also, feel free to test both kinds
trackComponentnotations in one route/test :)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.
hey, thanks for the info. i decided to go with creating a new route, and grouped the different notations together in a single test.