From 7c2dcbf9faa75492ce4a5d6ea2ada1688e2335a2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 12 Jul 2022 11:31:44 +0000 Subject: [PATCH] feat(tracing): Try parameterizing URLs in default routing instrumentation --- packages/tracing/src/browser/router.ts | 93 +++++++++++++- packages/tracing/test/browser/router.test.ts | 125 ++++++++++++++++++- 2 files changed, 212 insertions(+), 6 deletions(-) diff --git a/packages/tracing/src/browser/router.ts b/packages/tracing/src/browser/router.ts index b66037f38512..ee16ab9d1380 100644 --- a/packages/tracing/src/browser/router.ts +++ b/packages/tracing/src/browser/router.ts @@ -3,6 +3,63 @@ import { addInstrumentationHandler, getGlobalObject, logger } from '@sentry/util const global = getGlobalObject(); +const PATHNAME_PARAMETER_PATTERNS = { + number: /^\d+$/, + 'sha1-hash': /^[0-9a-f]{40}$/i, + 'md-hash': /^[0-9a-f]{32}$/i, + uuid: /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i, +}; + +/** + * Tries to parameterize a provided pathname based on some heuristics. + * + * @param pathname - A pathname, usually obtained via `window.location.pathname`. + * @returns a parameterized version of the pathname alongside the values behind the parameters + */ +function extractPathnameParametersWithHeuristics(pathname: string): { + parameterizedPathname: string; + pathnameParameterValues: Record; + hasParameters: boolean; +} { + /** + * Keeps track of the number of occurences of each pattern in the provided pathname. + */ + const patternCounts: Partial> = {}; + + /** + * Keeps track of what found parameters in the URL evaluate to, e.g: + * { "uuid-1": "2778216e-b40c-46...", "uuid-2": "86024957-1789-47ec-be7..." } + */ + const pathnameParameterValues: Record = {}; + + const parameterizedPathname = pathname + .split('/') + .map(originalPart => { + for (const patternName of Object.keys( + PATHNAME_PARAMETER_PATTERNS, + ) as (keyof typeof PATHNAME_PARAMETER_PATTERNS)[]) { + if (originalPart.match(PATHNAME_PARAMETER_PATTERNS[patternName])) { + // Set patternCounts to 1 if it hasn't been set yet + patternCounts[patternName] = (patternCounts[patternName] || 0) + 1; + + // Record what the parameter evaluated to + pathnameParameterValues[`${patternName}-${patternCounts[patternName]}`] = originalPart; + + return `{${patternName}}`; + } + } + + return originalPart; + }) + .join('/'); + + return { + parameterizedPathname, + pathnameParameterValues, + hasParameters: Object.keys(pathnameParameterValues).length > 0, + }; +} + /** * Default function implementing pageload and navigation transactions */ @@ -20,9 +77,25 @@ export function instrumentRoutingWithDefaults( let activeTransaction: T | undefined; if (startTransactionOnPageLoad) { + const { parameterizedPathname, pathnameParameterValues, hasParameters } = extractPathnameParametersWithHeuristics( + global.location.pathname, + ); + activeTransaction = customStartTransaction({ - name: global.location.pathname, + name: parameterizedPathname, op: 'pageload', + data: hasParameters + ? { + params: pathnameParameterValues, + originalPathname: global.location.pathname, + } + : undefined, + // For now, we will not define the transaction source as 'route' - even when we find a parameterizable + // pattern in the URL. Reason for that is, that we might run into URLs which both have parts that we identify + // as parameters, as well as parts that we didn't identify as such, even though they would be. + // An example: 'https://sentry.io/organization/some-random-org/user/14' + // Here we would identify '14' as parameter but not 'some-random-org'. To be on the safe side regarding + // transaction name cardinality, we still fall back to the source being 'url'. metadata: { source: 'url' }, }); } @@ -50,9 +123,25 @@ export function instrumentRoutingWithDefaults( // If there's an open transaction on the scope, we need to finish it before creating an new one. activeTransaction.finish(); } + + const { parameterizedPathname, pathnameParameterValues, hasParameters } = + extractPathnameParametersWithHeuristics(global.location.pathname); + activeTransaction = customStartTransaction({ - name: global.location.pathname, + name: parameterizedPathname, op: 'navigation', + data: hasParameters + ? { + params: pathnameParameterValues, + originalPathname: global.location.pathname, + } + : undefined, + // For now, we will not define the transaction source as 'route' - even when we find a parameterizable + // pattern in the URL. Reason for that is, that we might run into URLs which both have parts that we identify + // as parameters, as well as parts that we didn't identify as such, even though they would be. + // An example: 'https://sentry.io/organization/some-random-org/user/14' + // Here we would identify '14' as parameter but not 'some-random-org'. To be on the safe side regarding + // transaction name cardinality, we still fall back to the source being 'url'. metadata: { source: 'url' }, }); } diff --git a/packages/tracing/test/browser/router.test.ts b/packages/tracing/test/browser/router.test.ts index f0e3fec29084..ac80892ee3db 100644 --- a/packages/tracing/test/browser/router.test.ts +++ b/packages/tracing/test/browser/router.test.ts @@ -16,11 +16,15 @@ jest.mock('@sentry/utils', () => { }; }); +const DEFAULT_PAGE_URL = 'https://www.example.com/'; + +const dom = new JSDOM(undefined, { url: DEFAULT_PAGE_URL }); + describe('instrumentRoutingWithDefaults', () => { const mockFinish = jest.fn(); const customStartTransaction = jest.fn().mockReturnValue({ finish: mockFinish }); beforeEach(() => { - const dom = new JSDOM(); + dom.reconfigure({ url: DEFAULT_PAGE_URL }); // @ts-ignore need to override global document global.document = dom.window.document; // @ts-ignore need to override global document @@ -43,7 +47,7 @@ describe('instrumentRoutingWithDefaults', () => { instrumentRoutingWithDefaults(customStartTransaction); expect(customStartTransaction).toHaveBeenCalledTimes(1); expect(customStartTransaction).toHaveBeenLastCalledWith({ - name: 'blank', + name: '/', op: 'pageload', metadata: { source: 'url' }, }); @@ -54,6 +58,119 @@ describe('instrumentRoutingWithDefaults', () => { expect(customStartTransaction).toHaveBeenCalledTimes(0); }); + it.each([ + ['https://example.com', '/', undefined], + ['https://example.com/', '/', undefined], + ['https://example.com/home', '/home', undefined], + ['https://example.com/organization/some-org-slug', '/organization/some-org-slug', undefined], + // numbers + [ + 'https://example.com/organization/01337', + '/organization/{number}', + { params: { 'number-1': '01337' }, originalPathname: '/organization/01337' }, + ], + [ + 'https://example.com/organization/01337/user/42', + '/organization/{number}/user/{number}', + { params: { 'number-1': '01337', 'number-2': '42' }, originalPathname: '/organization/01337/user/42' }, + ], + [ + 'https://example.com/organization/01337/user/42/', + '/organization/{number}/user/{number}/', + { params: { 'number-1': '01337', 'number-2': '42' }, originalPathname: '/organization/01337/user/42/' }, + ], + // SHA + [ + 'https://example.com/organization/3da541559918a808c2402bba5012f6c60b27661c', + '/organization/{sha1-hash}', + { + params: { 'sha1-hash-1': '3da541559918a808c2402bba5012f6c60b27661c' }, + originalPathname: '/organization/3da541559918a808c2402bba5012f6c60b27661c', + }, + ], + [ + 'https://example.com/organization/3da541559918a808c2402bba5012f6c60b27661c/user/01ce26dc69094af9246ea7e7ce9970aff2b81cc9/', + '/organization/{sha1-hash}/user/{sha1-hash}/', + { + params: { + 'sha1-hash-1': '3da541559918a808c2402bba5012f6c60b27661c', + 'sha1-hash-2': '01ce26dc69094af9246ea7e7ce9970aff2b81cc9', + }, + originalPathname: + '/organization/3da541559918a808c2402bba5012f6c60b27661c/user/01ce26dc69094af9246ea7e7ce9970aff2b81cc9/', + }, + ], + // MD + [ + 'https://example.com/organization/1bee69a46ba811185c194762abaeae90', + '/organization/{md-hash}', + { + params: { 'md-hash-1': '1bee69a46ba811185c194762abaeae90' }, + originalPathname: '/organization/1bee69a46ba811185c194762abaeae90', + }, + ], + [ + 'https://example.com/organization/1bee69a46ba811185c194762abaeae90/user/b86e130ce7028da59e672d56ad0113df/', + '/organization/{md-hash}/user/{md-hash}/', + { + params: { + 'md-hash-1': '1bee69a46ba811185c194762abaeae90', + 'md-hash-2': 'b86e130ce7028da59e672d56ad0113df', + }, + originalPathname: '/organization/1bee69a46ba811185c194762abaeae90/user/b86e130ce7028da59e672d56ad0113df/', + }, + ], + // UUID + [ + 'https://example.com/organization/7591173e-01c7-11ed-b939-0242ac120002', + '/organization/{uuid}', + { + params: { 'uuid-1': '7591173e-01c7-11ed-b939-0242ac120002' }, + originalPathname: '/organization/7591173e-01c7-11ed-b939-0242ac120002', + }, + ], + [ + 'https://example.com/organization/7591173e-01c7-11ed-b939-0242ac120002/user/908607df-e5cb-4cc4-b61a-d6a534c43ec7/', + '/organization/{uuid}/user/{uuid}/', + { + params: { + 'uuid-1': '7591173e-01c7-11ed-b939-0242ac120002', + 'uuid-2': '908607df-e5cb-4cc4-b61a-d6a534c43ec7', + }, + originalPathname: + '/organization/7591173e-01c7-11ed-b939-0242ac120002/user/908607df-e5cb-4cc4-b61a-d6a534c43ec7/', + }, + ], + // mixed + [ + 'https://example.com/organization/0012301/user/908607df-e5cb-4cc4-b61a-d6a534c43ec7/setting/424242/value/1bee69a46ba811185c194762abaeae90', + '/organization/{number}/user/{uuid}/setting/{number}/value/{md-hash}', + { + params: { + 'md-hash-1': '1bee69a46ba811185c194762abaeae90', + 'number-1': '0012301', + 'number-2': '424242', + 'uuid-1': '908607df-e5cb-4cc4-b61a-d6a534c43ec7', + }, + originalPathname: + '/organization/0012301/user/908607df-e5cb-4cc4-b61a-d6a534c43ec7/setting/424242/value/1bee69a46ba811185c194762abaeae90', + }, + ], + ])( + 'should start a transaction with parameterization (url = %s)', + (simulatedUrl, expectedTransactionName, expectedTransactionData) => { + dom.reconfigure({ url: simulatedUrl }); + instrumentRoutingWithDefaults(customStartTransaction); + expect(customStartTransaction).toHaveBeenCalledTimes(1); + expect(customStartTransaction).toHaveBeenLastCalledWith({ + name: expectedTransactionName, + op: 'pageload', + data: expectedTransactionData, + metadata: { source: 'url' }, + }); + }, + ); + describe('navigation transaction', () => { beforeEach(() => { mockChangeHistory = () => undefined; @@ -63,7 +180,7 @@ describe('instrumentRoutingWithDefaults', () => { it('it is not created automatically', () => { instrumentRoutingWithDefaults(customStartTransaction); expect(customStartTransaction).not.toHaveBeenLastCalledWith({ - name: 'blank', + name: '/', op: 'navigation', metadata: { source: 'url' }, }); @@ -76,7 +193,7 @@ describe('instrumentRoutingWithDefaults', () => { expect(customStartTransaction).toHaveBeenCalledTimes(2); expect(customStartTransaction).toHaveBeenLastCalledWith({ - name: 'blank', + name: '/', op: 'navigation', metadata: { source: 'url' }, });