From 35e2fc56092ac8f7ea2f27f5f7303c8ed7fc3e85 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 29 Jun 2023 13:37:06 -0400 Subject: [PATCH 1/2] ref(tracing): Extract propagation context from meta tags --- .../src/browser/browsertracing.ts | 38 +++++++++---------- .../tracing-internal/src/browser/request.ts | 4 +- .../test/browser/browsertracing.test.ts | 12 +++--- .../test/browser/metrics/index.test.ts | 1 + .../test/browser/request.test.ts | 1 + .../test/browser/router.test.ts | 1 + 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 4d1b1bca7c97..9dcd99f3dbe0 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,14 +1,8 @@ /* eslint-disable max-lines */ import type { Hub, IdleTransaction } from '@sentry/core'; -import { - addTracingExtensions, - extractTraceparentData, - getActiveTransaction, - startIdleTransaction, - TRACING_DEFAULTS, -} from '@sentry/core'; +import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { baggageHeaderToDynamicSamplingContext, getDomElement, logger } from '@sentry/utils'; +import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; import { registerBackgroundTabDetection } from './backgroundtab'; import { @@ -297,24 +291,29 @@ export class BrowserTracing implements Integration { return undefined; } + const hub = this._getCurrentHub(); + const { beforeNavigate, idleTimeout, finalTimeout, heartbeatInterval } = this.options; const isPageloadTransaction = context.op === 'pageload'; - const sentryTraceMetaTagValue = isPageloadTransaction ? getMetaContent('sentry-trace') : null; - const baggageMetaTagValue = isPageloadTransaction ? getMetaContent('baggage') : null; - - const traceParentData = sentryTraceMetaTagValue ? extractTraceparentData(sentryTraceMetaTagValue) : undefined; - const dynamicSamplingContext = baggageMetaTagValue - ? baggageHeaderToDynamicSamplingContext(baggageMetaTagValue) - : undefined; + const sentryTrace = isPageloadTransaction ? getMetaContent('sentry-trace') : ''; + const baggage = isPageloadTransaction ? getMetaContent('baggage') : ''; + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTrace, + baggage, + ); + // Only set propagation context on pageload transactions. + if (isPageloadTransaction) { + hub.getScope().setPropagationContext(propagationContext); + } const expandedContext: TransactionContext = { ...context, - ...traceParentData, + ...traceparentData, metadata: { ...context.metadata, - dynamicSamplingContext: traceParentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, trimEnd: true, }; @@ -341,7 +340,6 @@ export class BrowserTracing implements Integration { __DEBUG_BUILD__ && logger.log(`[Tracing] Starting ${finalContext.op} transaction on scope`); - const hub = this._getCurrentHub(); const { location } = WINDOW; const idleTransaction = startIdleTransaction( @@ -424,11 +422,11 @@ export class BrowserTracing implements Integration { } /** Returns the value of a meta tag */ -export function getMetaContent(metaName: string): string | null { +export function getMetaContent(metaName: string): string | undefined { // Can't specify generic to `getDomElement` because tracing can be used // in a variety of environments, have to disable `no-unsafe-member-access` // as a result. const metaTag = getDomElement(`meta[name=${metaName}]`); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return metaTag ? metaTag.getAttribute('content') : null; + return metaTag ? metaTag.getAttribute('content') : undefined; } diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index d7e397ae01ac..9a5bbb330b6b 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -214,7 +214,7 @@ export function shouldAttachHeaders(url: string, tracePropagationTargets: (strin * * @returns Span if a span was created, otherwise void. */ -function fetchCallback( +export function fetchCallback( handlerData: FetchData, shouldCreateSpan: (url: string) => boolean, shouldAttachHeaders: (url: string) => boolean, @@ -364,7 +364,7 @@ export function addTracingHeadersToFetchRequest( * * @returns Span if a span was created, otherwise void. */ -function xhrCallback( +export function xhrCallback( handlerData: XHRData, shouldCreateSpan: (url: string) => boolean, shouldAttachHeaders: (url: string) => boolean, diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts index c7cec8a54735..156203dc40e0 100644 --- a/packages/tracing-internal/test/browser/browsertracing.test.ts +++ b/packages/tracing-internal/test/browser/browsertracing.test.ts @@ -94,7 +94,6 @@ describe('BrowserTracing', () => { const browserTracing = createBrowserTracing(); expect(browserTracing.options).toEqual({ - _experiments: {}, enableLongTask: true, ...TRACING_DEFAULTS, markBackgroundTransactions: true, @@ -113,9 +112,6 @@ describe('BrowserTracing', () => { }); expect(browserTracing.options).toEqual({ - _experiments: { - enableLongTask: false, - }, enableLongTask: false, ...TRACING_DEFAULTS, markBackgroundTransactions: true, @@ -123,6 +119,9 @@ describe('BrowserTracing', () => { startTransactionOnLocationChange: true, startTransactionOnPageLoad: true, ...defaultRequestInstrumentationOptions, + _experiments: { + enableLongTask: false, + }, }); }); @@ -132,7 +131,6 @@ describe('BrowserTracing', () => { }); expect(browserTracing.options).toEqual({ - _experiments: {}, enableLongTask: false, ...TRACING_DEFAULTS, markBackgroundTransactions: true, @@ -248,6 +246,7 @@ describe('BrowserTracing', () => { traceFetch: true, traceXHR: true, tracePropagationTargets: ['something'], + _experiments: {}, }); }); @@ -261,6 +260,7 @@ describe('BrowserTracing', () => { }); expect(instrumentOutgoingRequestsMock).toHaveBeenCalledWith({ + _experiments: {}, traceFetch: true, traceXHR: true, tracePropagationTargets: ['something-else'], @@ -546,7 +546,7 @@ describe('BrowserTracing', () => { document.head.innerHTML = ''; const metaTagValue = getMetaContent('dogpark'); - expect(metaTagValue).toBe(null); + expect(metaTagValue).toBe(undefined); }); it('can pick the correct tag out of multiple options', () => { diff --git a/packages/tracing-internal/test/browser/metrics/index.test.ts b/packages/tracing-internal/test/browser/metrics/index.test.ts index d325815bde31..c196fbd2467c 100644 --- a/packages/tracing-internal/test/browser/metrics/index.test.ts +++ b/packages/tracing-internal/test/browser/metrics/index.test.ts @@ -14,6 +14,7 @@ describe('_addMeasureSpans', () => { name: 'measure-1', duration: 10, startTime: 12, + detail: undefined, }; const timeOrigin = 100; diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index ee714e111a8b..c0c544506a09 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -240,6 +240,7 @@ describe('callbacks', () => { expect(finishedSpan.data).toEqual({ 'http.response_content_length': 123, 'http.method': 'GET', + 'http.response.status_code': 404, type: 'fetch', url: 'http://dogs.are.great/', }); diff --git a/packages/tracing-internal/test/browser/router.test.ts b/packages/tracing-internal/test/browser/router.test.ts index 65ce7e90af48..cfa2179386d9 100644 --- a/packages/tracing-internal/test/browser/router.test.ts +++ b/packages/tracing-internal/test/browser/router.test.ts @@ -46,6 +46,7 @@ describe('instrumentRoutingWithDefaults', () => { name: 'blank', op: 'pageload', metadata: { source: 'url' }, + startTimestamp: expect.any(Number), }); }); From 7450665f12ec6ff9356d57c1c27578765b1c7ee2 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 29 Jun 2023 15:45:28 -0400 Subject: [PATCH 2/2] improve logic for navigation transactions --- .../src/browser/browsertracing.ts | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 9dcd99f3dbe0..4be633821a1a 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -303,10 +303,6 @@ export class BrowserTracing implements Integration { sentryTrace, baggage, ); - // Only set propagation context on pageload transactions. - if (isPageloadTransaction) { - hub.getScope().setPropagationContext(propagationContext); - } const expandedContext: TransactionContext = { ...context, @@ -351,6 +347,24 @@ export class BrowserTracing implements Integration { { location }, // for use in the tracesSampler heartbeatInterval, ); + + const scope = hub.getScope(); + + // If it's a pageload and there is a meta tag set + // use the traceparentData as the propagation context + if (isPageloadTransaction && traceparentData) { + scope.setPropagationContext(propagationContext); + } else { + // Navigation transactions should set a new propagation context based on the + // created idle transaction. + scope.setPropagationContext({ + traceId: idleTransaction.traceId, + spanId: idleTransaction.spanId, + parentSpanId: idleTransaction.parentSpanId, + sampled: !!idleTransaction.sampled, + }); + } + idleTransaction.registerBeforeFinishCallback(transaction => { this._collectWebVitals(); addPerformanceEntries(transaction);