From 73da670f689e87eb1fab7923b1cdf825d113d04f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 18 Aug 2023 10:04:51 +0000 Subject: [PATCH 1/2] fix: Defer tracing decision for tracing context without performance --- packages/core/src/scope.ts | 4 +--- packages/hub/test/scope.test.ts | 4 ++-- packages/node/test/integrations/http.test.ts | 5 +++-- .../tracing-internal/src/browser/browsertracing.ts | 11 +++++------ packages/types/src/tracing.ts | 2 +- packages/utils/src/tracing.ts | 5 ++--- 6 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index b72defc56016..1c5987d709d1 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines */ +import { updateSession } from './session'; import type { Attachment, Breadcrumb, @@ -33,8 +34,6 @@ import { uuid4, } from '@sentry/utils'; -import { updateSession } from './session'; - /** * Default value for maximum number of breadcrumbs added to an event. */ @@ -636,6 +635,5 @@ function generatePropagationContext(): PropagationContext { return { traceId: uuid4(), spanId: uuid4().substring(16), - sampled: false, }; } diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index 982cba766a87..4c0d830340ec 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -20,7 +20,7 @@ describe('Scope', () => { expect(scope._propagationContext).toEqual({ traceId: expect.any(String), spanId: expect.any(String), - sampled: false, + sampled: undefined, dsc: undefined, parentSpanId: undefined, }); @@ -442,7 +442,7 @@ describe('Scope', () => { expect(scope._propagationContext).toEqual({ traceId: expect.any(String), spanId: expect.any(String), - sampled: false, + sampled: undefined, }); // @ts-expect-error accessing private property expect(scope._propagationContext).not.toEqual(oldPropagationContext); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index e784eb308f54..0f1613a27345 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -208,10 +208,11 @@ describe('tracing', () => { const baggageHeader = request.getHeader('baggage') as string; const parts = sentryTraceHeader.split('-'); - expect(parts.length).toEqual(3); + + // Should not include sampling decision since we don't wanna influence the tracing decisions downstream + expect(parts.length).toEqual(2); expect(parts[0]).toEqual(traceId); expect(parts[1]).toEqual(expect.any(String)); - expect(parts[2]).toEqual('0'); expect(baggageHeader).toEqual( `sentry-environment=production,sentry-release=1.0.0,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=${traceId}`, diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index aae66bee3358..1a5cebeccd14 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,9 +1,4 @@ /* eslint-disable max-lines */ -import type { Hub, IdleTransaction } from '@sentry/core'; -import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; -import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; - import { registerBackgroundTabDetection } from './backgroundtab'; import { addPerformanceEntries, @@ -15,6 +10,10 @@ import type { RequestInstrumentationOptions } from './request'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; import { instrumentRoutingWithDefaults } from './router'; import { WINDOW } from './types'; +import type { Hub, IdleTransaction } from '@sentry/core'; +import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; +import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; @@ -363,7 +362,7 @@ export class BrowserTracing implements Integration { traceId: idleTransaction.traceId, spanId: idleTransaction.spanId, parentSpanId: idleTransaction.parentSpanId, - sampled: !!idleTransaction.sampled, + sampled: idleTransaction.sampled, }); } diff --git a/packages/types/src/tracing.ts b/packages/types/src/tracing.ts index 11c4a1658d50..7c5b02c45596 100644 --- a/packages/types/src/tracing.ts +++ b/packages/types/src/tracing.ts @@ -5,7 +5,7 @@ export type TracePropagationTargets = (string | RegExp)[]; export interface PropagationContext { traceId: string; spanId: string; - sampled: boolean; + sampled?: boolean; parentSpanId?: string; dsc?: DynamicSamplingContext; } diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index f879b856f9f2..845430fdc209 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -1,7 +1,6 @@ -import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; - import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { uuid4 } from './misc'; +import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; export const TRACEPARENT_REGEXP = new RegExp( '^[ \\t]*' + // whitespace @@ -61,7 +60,7 @@ export function tracingContextFromHeaders( const propagationContext: PropagationContext = { traceId: traceId || uuid4(), spanId: uuid4().substring(16), - sampled: parentSampled === undefined ? false : parentSampled, + sampled: parentSampled, }; if (parentSpanId) { From 7b51e1b242042f5d63db34875444db64245d78af Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 18 Aug 2023 10:12:05 +0000 Subject: [PATCH 2/2] Format --- packages/core/src/scope.ts | 3 ++- packages/tracing-internal/src/browser/browsertracing.ts | 9 +++++---- packages/utils/src/tracing.ts | 3 ++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 1c5987d709d1..8875f1c996f2 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -1,5 +1,4 @@ /* eslint-disable max-lines */ -import { updateSession } from './session'; import type { Attachment, Breadcrumb, @@ -34,6 +33,8 @@ import { uuid4, } from '@sentry/utils'; +import { updateSession } from './session'; + /** * Default value for maximum number of breadcrumbs added to an event. */ diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 1a5cebeccd14..ef40adc147ec 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,4 +1,9 @@ /* eslint-disable max-lines */ +import type { Hub, IdleTransaction } from '@sentry/core'; +import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; +import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; + import { registerBackgroundTabDetection } from './backgroundtab'; import { addPerformanceEntries, @@ -10,10 +15,6 @@ import type { RequestInstrumentationOptions } from './request'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request'; import { instrumentRoutingWithDefaults } from './router'; import { WINDOW } from './types'; -import type { Hub, IdleTransaction } from '@sentry/core'; -import { addTracingExtensions, getActiveTransaction, startIdleTransaction, TRACING_DEFAULTS } from '@sentry/core'; -import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils'; export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index 845430fdc209..6ad839f2b920 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -1,6 +1,7 @@ +import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; + import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { uuid4 } from './misc'; -import type { DynamicSamplingContext, PropagationContext, TraceparentData } from '@sentry/types'; export const TRACEPARENT_REGEXP = new RegExp( '^[ \\t]*' + // whitespace