From 7736af6478516603a9e52d5a49ca60cb898f5042 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 20 Mar 2025 21:15:00 -0400 Subject: [PATCH 1/4] perf(core): Remove usage of addNonEnumerableProperty from span utils --- packages/core/src/tracing/utils.ts | 18 +++---- packages/core/src/utils/spanUtils.ts | 47 +++++++++---------- packages/core/test/lib/tracing/trace.test.ts | 6 +-- .../opentelemetry/src/utils/contextData.ts | 9 ++-- 4 files changed, 36 insertions(+), 44 deletions(-) diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index 61e2dcce2bc1..c5ddd296e7e3 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -1,20 +1,14 @@ import type { Scope } from '../scope'; import type { Span } from '../types-hoist'; -import { addNonEnumerableProperty } from '../utils-hoist/object'; -const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; -const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope'; - -type SpanWithScopes = Span & { - [SCOPE_ON_START_SPAN_FIELD]?: Scope; - [ISOLATION_SCOPE_ON_START_SPAN_FIELD]?: Scope; -}; +const SPAN_TO_SCOPE_MAP = new WeakMap(); +const SPAN_TO_ISOLATION_SCOPE_MAP = new WeakMap(); /** Store the scope & isolation scope for a span, which can the be used when it is finished. */ export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, isolationScope: Scope): void { if (span) { - addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, isolationScope); - addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope); + SPAN_TO_SCOPE_MAP.set(span, scope); + SPAN_TO_ISOLATION_SCOPE_MAP.set(span, isolationScope); } } @@ -23,7 +17,7 @@ export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, is */ export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationScope?: Scope } { return { - scope: (span as SpanWithScopes)[SCOPE_ON_START_SPAN_FIELD], - isolationScope: (span as SpanWithScopes)[ISOLATION_SCOPE_ON_START_SPAN_FIELD], + scope: SPAN_TO_SCOPE_MAP.get(span), + isolationScope: SPAN_TO_ISOLATION_SCOPE_MAP.get(span), }; } diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 0a46ba600e0c..a4a163509d3a 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -21,7 +21,7 @@ import type { } from '../types-hoist'; import type { SpanLink, SpanLinkJSON } from '../types-hoist/link'; import { consoleSandbox } from '../utils-hoist/logger'; -import { addNonEnumerableProperty, dropUndefinedKeys } from '../utils-hoist/object'; +import { dropUndefinedKeys } from '../utils-hoist/object'; import { generateSpanId } from '../utils-hoist/propagationContext'; import { timestampInSeconds } from '../utils-hoist/time'; import { generateSentryTraceHeader } from '../utils-hoist/tracing'; @@ -223,55 +223,54 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef return status.message || 'unknown_error'; } -const CHILD_SPANS_FIELD = '_sentryChildSpans'; -const ROOT_SPAN_FIELD = '_sentryRootSpan'; - -type SpanWithPotentialChildren = Span & { - [CHILD_SPANS_FIELD]?: Set; - [ROOT_SPAN_FIELD]?: Span; -}; +const SPAN_TO_ROOT_SPAN_MAP = new WeakMap(); +const SPAN_TO_CHILD_SPANS_MAP = new WeakMap>(); /** * Adds an opaque child span reference to a span. */ -export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void { +export function addChildSpanToSpan(span: Span, childSpan: Span): void { // We store the root span reference on the child span // We need this for `getRootSpan()` to work - const rootSpan = span[ROOT_SPAN_FIELD] || span; - addNonEnumerableProperty(childSpan as SpanWithPotentialChildren, ROOT_SPAN_FIELD, rootSpan); + const rootSpan = SPAN_TO_ROOT_SPAN_MAP.get(span) || span; + SPAN_TO_ROOT_SPAN_MAP.set(childSpan, rootSpan); // We store a list of child spans on the parent span // We need this for `getSpanDescendants()` to work - if (span[CHILD_SPANS_FIELD]) { - span[CHILD_SPANS_FIELD].add(childSpan); + const childSpans = SPAN_TO_CHILD_SPANS_MAP.get(span); + if (childSpans) { + childSpans.add(childSpan); } else { - addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([childSpan])); + SPAN_TO_CHILD_SPANS_MAP.set(span, new Set([childSpan])); } } /** This is only used internally by Idle Spans. */ -export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void { - if (span[CHILD_SPANS_FIELD]) { - span[CHILD_SPANS_FIELD].delete(childSpan); +export function removeChildSpanFromSpan(span: Span, childSpan: Span): void { + const childSpans = SPAN_TO_CHILD_SPANS_MAP.get(span); + if (childSpans) { + childSpans.delete(childSpan); } } /** * Returns an array of the given span and all of its descendants. */ -export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] { +export function getSpanDescendants(span: Span): Span[] { const resultSet = new Set(); - function addSpanChildren(span: SpanWithPotentialChildren): void { + function addSpanChildren(span: Span): void { // This exit condition is required to not infinitely loop in case of a circular dependency. if (resultSet.has(span)) { return; // We want to ignore unsampled spans (e.g. non recording spans) } else if (spanIsSampled(span)) { resultSet.add(span); - const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; - for (const childSpan of childSpans) { - addSpanChildren(childSpan); + const childSpans = SPAN_TO_CHILD_SPANS_MAP.get(span); + if (childSpans) { + for (const childSpan of childSpans) { + addSpanChildren(childSpan); + } } } } @@ -284,8 +283,8 @@ export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] { /** * Returns the root span of a given span. */ -export function getRootSpan(span: SpanWithPotentialChildren): Span { - return span[ROOT_SPAN_FIELD] || span; +export function getRootSpan(span: Span): Span { + return SPAN_TO_ROOT_SPAN_MAP.get(span) || span; } /** diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 81d1258517d3..61dd9f5cf9de 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -687,7 +687,7 @@ describe('startSpan', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { startSpan({ name: 'inner' }, innerSpan => { - const childSpans = Array.from(outerSpan._sentryChildSpans); + const childSpans = getSpanDescendants(outerSpan); expect(childSpans).toContain(innerSpan); }); }); @@ -1178,7 +1178,7 @@ describe('startSpanManual', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { startSpanManual({ name: 'inner' }, innerSpan => { - const childSpans = Array.from(outerSpan._sentryChildSpans); + const childSpans = getSpanDescendants(outerSpan); expect(childSpans).toContain(innerSpan); }); }); @@ -1591,7 +1591,7 @@ describe('startInactiveSpan', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { const innerSpan = startInactiveSpan({ name: 'inner' }); - const childSpans = Array.from(outerSpan._sentryChildSpans); + const childSpans = getSpanDescendants(outerSpan); expect(childSpans).toContain(innerSpan); }); }); diff --git a/packages/opentelemetry/src/utils/contextData.ts b/packages/opentelemetry/src/utils/contextData.ts index 468b377f9ccd..2ab9bf661f14 100644 --- a/packages/opentelemetry/src/utils/contextData.ts +++ b/packages/opentelemetry/src/utils/contextData.ts @@ -1,11 +1,8 @@ import type { Context } from '@opentelemetry/api'; import type { Scope } from '@sentry/core'; -import { addNonEnumerableProperty } from '@sentry/core'; import { SENTRY_SCOPES_CONTEXT_KEY } from '../constants'; import type { CurrentScopes } from '../types'; -const SCOPE_CONTEXT_FIELD = '_scopeContext'; - /** * Try to get the current scopes from the given OTEL context. * This requires a Context Manager that was wrapped with getWrappedContextManager. @@ -22,17 +19,19 @@ export function setScopesOnContext(context: Context, scopes: CurrentScopes): Con return context.setValue(SENTRY_SCOPES_CONTEXT_KEY, scopes); } +const SCOPE_TO_CONTEXT_MAP = new WeakMap(); + /** * Set the context on the scope so we can later look it up. * We need this to get the context from the scope in the `trace` functions. */ export function setContextOnScope(scope: Scope, context: Context): void { - addNonEnumerableProperty(scope, SCOPE_CONTEXT_FIELD, context); + SCOPE_TO_CONTEXT_MAP.set(scope, context); } /** * Get the context related to a scope. */ export function getContextFromScope(scope: Scope): Context | undefined { - return (scope as { [SCOPE_CONTEXT_FIELD]?: Context })[SCOPE_CONTEXT_FIELD]; + return SCOPE_TO_CONTEXT_MAP.get(scope); } From f31c4384c3ff32e7698b6603d88a831d545df504 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 21 Mar 2025 12:03:52 -0400 Subject: [PATCH 2/4] remove one more usage and ref rollup mangle --- .../rollup-utils/plugins/bundlePlugins.mjs | 6 ------ packages/core/src/utils/spanOnScope.ts | 16 +++++----------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs index 9d6edd3157c0..3b95b2c6d2fd 100644 --- a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs +++ b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs @@ -126,12 +126,6 @@ export function makeTerserPlugin() { '_sentryId', // Keeps the frozen DSC on a Sentry Span '_frozenDsc', - // These are used to keep span & scope relationships - '_sentryRootSpan', - '_sentryChildSpans', - '_sentrySpan', - '_sentryScope', - '_sentryIsolationScope', // require-in-the-middle calls `Module._resolveFilename`. We cannot mangle this (AWS lambda layer bundle). '_resolveFilename', // Set on e.g. the shim feedbackIntegration to be able to detect it diff --git a/packages/core/src/utils/spanOnScope.ts b/packages/core/src/utils/spanOnScope.ts index 33d0ff80dd12..8f1ab3cded5e 100644 --- a/packages/core/src/utils/spanOnScope.ts +++ b/packages/core/src/utils/spanOnScope.ts @@ -1,12 +1,7 @@ import type { Scope } from '../scope'; import type { Span } from '../types-hoist'; -import { addNonEnumerableProperty } from '../utils-hoist/object'; -const SCOPE_SPAN_FIELD = '_sentrySpan'; - -type ScopeWithMaybeSpan = Scope & { - [SCOPE_SPAN_FIELD]?: Span; -}; +const SCOPE_TO_SPAN_MAP = new WeakMap(); /** * Set the active span for a given scope. @@ -14,10 +9,9 @@ type ScopeWithMaybeSpan = Scope & { */ export function _setSpanForScope(scope: Scope, span: Span | undefined): void { if (span) { - addNonEnumerableProperty(scope as ScopeWithMaybeSpan, SCOPE_SPAN_FIELD, span); + SCOPE_TO_SPAN_MAP.set(scope, span); } else { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete (scope as ScopeWithMaybeSpan)[SCOPE_SPAN_FIELD]; + SCOPE_TO_SPAN_MAP.delete(scope); } } @@ -25,6 +19,6 @@ export function _setSpanForScope(scope: Scope, span: Span | undefined): void { * Get the active span for a given scope. * NOTE: This should NOT be used directly, but is only used internally by the trace methods. */ -export function _getSpanForScope(scope: ScopeWithMaybeSpan): Span | undefined { - return scope[SCOPE_SPAN_FIELD]; +export function _getSpanForScope(scope: Scope): Span | undefined { + return SCOPE_TO_SPAN_MAP.get(scope); } From a416137803beb56675a3be1aa88ab06b46353683 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 24 Mar 2025 13:15:08 -0400 Subject: [PATCH 3/4] revert packages/core/src/tracing/utils.ts and amend dsc logic --- .../rollup-utils/plugins/bundlePlugins.mjs | 3 +++ .../core/src/tracing/dynamicSamplingContext.ts | 17 ++++------------- packages/core/src/tracing/utils.ts | 18 ++++++++++++------ 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs index 3b95b2c6d2fd..e0d580723e79 100644 --- a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs +++ b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs @@ -126,6 +126,9 @@ export function makeTerserPlugin() { '_sentryId', // Keeps the frozen DSC on a Sentry Span '_frozenDsc', + // These are used to keep span & scope relationships + '_sentryScope', + '_sentryIsolationScope', // require-in-the-middle calls `Module._resolveFilename`. We cannot mangle this (AWS lambda layer bundle). '_resolveFilename', // Set on e.g. the shim feedbackIntegration to be able to detect it diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 706684ffecab..41b3b61fc4b1 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -8,27 +8,18 @@ import { baggageHeaderToDynamicSamplingContext, dynamicSamplingContextToSentryBaggageHeader, } from '../utils-hoist/baggage'; -import { addNonEnumerableProperty, dropUndefinedKeys } from '../utils-hoist/object'; +import { dropUndefinedKeys } from '../utils-hoist/object'; import { hasSpansEnabled } from '../utils/hasSpansEnabled'; import { getRootSpan, spanIsSampled, spanToJSON } from '../utils/spanUtils'; import { getCapturedScopesOnSpan } from './utils'; -/** - * If you change this value, also update the terser plugin config to - * avoid minification of the object property! - */ -const FROZEN_DSC_FIELD = '_frozenDsc'; - -type SpanWithMaybeDsc = Span & { - [FROZEN_DSC_FIELD]?: Partial | undefined; -}; +const SPAN_TO_DSC_MAP = new WeakMap>(); /** * Freeze the given DSC on the given span. */ export function freezeDscOnSpan(span: Span, dsc: Partial): void { - const spanWithMaybeDsc = span as SpanWithMaybeDsc; - addNonEnumerableProperty(spanWithMaybeDsc, FROZEN_DSC_FIELD, dsc); + SPAN_TO_DSC_MAP.set(span, dsc); } /** @@ -91,7 +82,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly(); -const SPAN_TO_ISOLATION_SCOPE_MAP = new WeakMap(); +const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; +const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope'; + +type SpanWithScopes = Span & { + [SCOPE_ON_START_SPAN_FIELD]?: Scope; + [ISOLATION_SCOPE_ON_START_SPAN_FIELD]?: Scope; +}; /** Store the scope & isolation scope for a span, which can the be used when it is finished. */ export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, isolationScope: Scope): void { if (span) { - SPAN_TO_SCOPE_MAP.set(span, scope); - SPAN_TO_ISOLATION_SCOPE_MAP.set(span, isolationScope); + addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, isolationScope); + addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope); } } @@ -17,7 +23,7 @@ export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, is */ export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationScope?: Scope } { return { - scope: SPAN_TO_SCOPE_MAP.get(span), - isolationScope: SPAN_TO_ISOLATION_SCOPE_MAP.get(span), + scope: (span as SpanWithScopes)[SCOPE_ON_START_SPAN_FIELD], + isolationScope: (span as SpanWithScopes)[ISOLATION_SCOPE_ON_START_SPAN_FIELD], }; } From f3c7cc3c4b740f2ba429139053b1ff74367b3afd Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 24 Mar 2025 13:16:03 -0400 Subject: [PATCH 4/4] update minification --- dev-packages/rollup-utils/plugins/bundlePlugins.mjs | 2 -- 1 file changed, 2 deletions(-) diff --git a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs index e0d580723e79..d9dd190f6eab 100644 --- a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs +++ b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs @@ -124,8 +124,6 @@ export function makeTerserPlugin() { // These are used by instrument.ts in utils for identifying HTML elements & events '_sentryCaptured', '_sentryId', - // Keeps the frozen DSC on a Sentry Span - '_frozenDsc', // These are used to keep span & scope relationships '_sentryScope', '_sentryIsolationScope',