From 71d30857a12148bd2d23bb03ce1830ce426795af Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Jul 2022 12:28:04 -0400 Subject: [PATCH 1/5] ref(tracing): Add transaction source to default router --- packages/tracing/src/browser/router.ts | 12 ++++++++++-- packages/tracing/test/browser/router.test.ts | 18 +++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/tracing/src/browser/router.ts b/packages/tracing/src/browser/router.ts index 20add62319fd..b66037f38512 100644 --- a/packages/tracing/src/browser/router.ts +++ b/packages/tracing/src/browser/router.ts @@ -20,7 +20,11 @@ export function instrumentRoutingWithDefaults( let activeTransaction: T | undefined; if (startTransactionOnPageLoad) { - activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'pageload' }); + activeTransaction = customStartTransaction({ + name: global.location.pathname, + op: 'pageload', + metadata: { source: 'url' }, + }); } if (startTransactionOnLocationChange) { @@ -46,7 +50,11 @@ export function instrumentRoutingWithDefaults( // If there's an open transaction on the scope, we need to finish it before creating an new one. activeTransaction.finish(); } - activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'navigation' }); + activeTransaction = customStartTransaction({ + name: global.location.pathname, + op: 'navigation', + metadata: { source: 'url' }, + }); } }); } diff --git a/packages/tracing/test/browser/router.test.ts b/packages/tracing/test/browser/router.test.ts index 9d1ae86f4e01..f0e3fec29084 100644 --- a/packages/tracing/test/browser/router.test.ts +++ b/packages/tracing/test/browser/router.test.ts @@ -42,7 +42,11 @@ describe('instrumentRoutingWithDefaults', () => { it('starts a pageload transaction', () => { instrumentRoutingWithDefaults(customStartTransaction); expect(customStartTransaction).toHaveBeenCalledTimes(1); - expect(customStartTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'pageload' }); + expect(customStartTransaction).toHaveBeenLastCalledWith({ + name: 'blank', + op: 'pageload', + metadata: { source: 'url' }, + }); }); it('does not start a pageload transaction if startTransactionOnPageLoad is false', () => { @@ -58,7 +62,11 @@ describe('instrumentRoutingWithDefaults', () => { it('it is not created automatically', () => { instrumentRoutingWithDefaults(customStartTransaction); - expect(customStartTransaction).not.toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); + expect(customStartTransaction).not.toHaveBeenLastCalledWith({ + name: 'blank', + op: 'navigation', + metadata: { source: 'url' }, + }); }); it('is created on location change', () => { @@ -67,7 +75,11 @@ describe('instrumentRoutingWithDefaults', () => { expect(addInstrumentationHandlerType).toBe('history'); expect(customStartTransaction).toHaveBeenCalledTimes(2); - expect(customStartTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); + expect(customStartTransaction).toHaveBeenLastCalledWith({ + name: 'blank', + op: 'navigation', + metadata: { source: 'url' }, + }); }); it('is not created if startTransactionOnLocationChange is false', () => { From d265a85e1cb43b190186bd74d30b2e901293928e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Jul 2022 12:55:49 -0400 Subject: [PATCH 2/5] add integration test for router --- .../tracing/browsertracing/source/test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 packages/integration-tests/suites/tracing/browsertracing/source/test.ts diff --git a/packages/integration-tests/suites/tracing/browsertracing/source/test.ts b/packages/integration-tests/suites/tracing/browsertracing/source/test.ts new file mode 100644 index 000000000000..9fa444fb43e5 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/source/test.ts @@ -0,0 +1,17 @@ +import { expect } from '@playwright/test'; +import { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; + +sentryTest('transactions should contain transaction source', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); + const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); + + expect(pageloadRequest).toEqual({}); + + expect(pageloadRequest.transaction_info?.source).toEqual('url'); + expect(navigationRequest.transaction_info?.source).toEqual('url'); +}); From fdc43aaebe62242ea4f68cb534137779fc6e8c85 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Jul 2022 14:10:56 -0400 Subject: [PATCH 3/5] remove uneeded check --- .../suites/tracing/browsertracing/source/test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/integration-tests/suites/tracing/browsertracing/source/test.ts b/packages/integration-tests/suites/tracing/browsertracing/source/test.ts index 9fa444fb43e5..bca937ada24d 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/source/test.ts +++ b/packages/integration-tests/suites/tracing/browsertracing/source/test.ts @@ -10,8 +10,6 @@ sentryTest('transactions should contain transaction source', async ({ getLocalTe const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - expect(pageloadRequest).toEqual({}); - expect(pageloadRequest.transaction_info?.source).toEqual('url'); expect(navigationRequest.transaction_info?.source).toEqual('url'); }); From 3371d3edfeecfc749da1b1c10c2593ae214dd7c1 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Jul 2022 14:20:54 -0400 Subject: [PATCH 4/5] re-arrange tests --- .../tracing/browsertracing/navigation/test.ts | 2 ++ .../tracing/browsertracing/pageload/test.ts | 1 + .../suites/tracing/browsertracing/source/test.ts | 15 --------------- 3 files changed, 3 insertions(+), 15 deletions(-) delete mode 100644 packages/integration-tests/suites/tracing/browsertracing/source/test.ts diff --git a/packages/integration-tests/suites/tracing/browsertracing/navigation/test.ts b/packages/integration-tests/suites/tracing/browsertracing/navigation/test.ts index 57c035a96b85..622ecdd55173 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/navigation/test.ts +++ b/packages/integration-tests/suites/tracing/browsertracing/navigation/test.ts @@ -13,6 +13,8 @@ sentryTest('should create a navigation transaction on page navigation', async ({ expect(pageloadRequest.contexts?.trace.op).toBe('pageload'); expect(navigationRequest.contexts?.trace.op).toBe('navigation'); + expect(navigationRequest.transaction_info?.source).toEqual('url'); + const pageloadTraceId = pageloadRequest.contexts?.trace.trace_id; const navigationTraceId = navigationRequest.contexts?.trace.trace_id; diff --git a/packages/integration-tests/suites/tracing/browsertracing/pageload/test.ts b/packages/integration-tests/suites/tracing/browsertracing/pageload/test.ts index bb8707ff51f8..6c5877c62e57 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/pageload/test.ts +++ b/packages/integration-tests/suites/tracing/browsertracing/pageload/test.ts @@ -11,4 +11,5 @@ sentryTest('should create a pageload transaction', async ({ getLocalTestPath, pa expect(eventData.contexts?.trace?.op).toBe('pageload'); expect(eventData.spans?.length).toBeGreaterThan(0); + expect(eventData.transaction_info?.source).toEqual('url'); }); diff --git a/packages/integration-tests/suites/tracing/browsertracing/source/test.ts b/packages/integration-tests/suites/tracing/browsertracing/source/test.ts deleted file mode 100644 index bca937ada24d..000000000000 --- a/packages/integration-tests/suites/tracing/browsertracing/source/test.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { expect } from '@playwright/test'; -import { Event } from '@sentry/types'; - -import { sentryTest } from '../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; - -sentryTest('transactions should contain transaction source', async ({ getLocalTestPath, page }) => { - const url = await getLocalTestPath({ testDir: __dirname }); - - const pageloadRequest = await getFirstSentryEnvelopeRequest(page, url); - const navigationRequest = await getFirstSentryEnvelopeRequest(page, `${url}#foo`); - - expect(pageloadRequest.transaction_info?.source).toEqual('url'); - expect(navigationRequest.transaction_info?.source).toEqual('url'); -}); From 2a070e8ab8689643c48b87b81e2ea844c5325a6a Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Jul 2022 14:28:58 -0400 Subject: [PATCH 5/5] fix metadata not getting set --- packages/tracing/src/browser/browsertracing.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 75e45b35e3da..ee927cf8a779 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -208,6 +208,12 @@ export class BrowserTracing implements Integration { const expandedContext = { ...context, ...parentContextFromHeader, + ...(parentContextFromHeader && { + metadata: { + ...context.metadata, + ...parentContextFromHeader.metadata, + }, + }), trimEnd: true, }; const modifiedContext = typeof beforeNavigate === 'function' ? beforeNavigate(expandedContext) : expandedContext;