Skip to content

Commit 7736af6

Browse files
committed
perf(core): Remove usage of addNonEnumerableProperty from span utils
1 parent 38a499a commit 7736af6

File tree

4 files changed

+36
-44
lines changed

4 files changed

+36
-44
lines changed

packages/core/src/tracing/utils.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,14 @@
11
import type { Scope } from '../scope';
22
import type { Span } from '../types-hoist';
3-
import { addNonEnumerableProperty } from '../utils-hoist/object';
43

5-
const SCOPE_ON_START_SPAN_FIELD = '_sentryScope';
6-
const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope';
7-
8-
type SpanWithScopes = Span & {
9-
[SCOPE_ON_START_SPAN_FIELD]?: Scope;
10-
[ISOLATION_SCOPE_ON_START_SPAN_FIELD]?: Scope;
11-
};
4+
const SPAN_TO_SCOPE_MAP = new WeakMap<Span, Scope>();
5+
const SPAN_TO_ISOLATION_SCOPE_MAP = new WeakMap<Span, Scope>();
126

137
/** Store the scope & isolation scope for a span, which can the be used when it is finished. */
148
export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, isolationScope: Scope): void {
159
if (span) {
16-
addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, isolationScope);
17-
addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope);
10+
SPAN_TO_SCOPE_MAP.set(span, scope);
11+
SPAN_TO_ISOLATION_SCOPE_MAP.set(span, isolationScope);
1812
}
1913
}
2014

@@ -23,7 +17,7 @@ export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, is
2317
*/
2418
export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationScope?: Scope } {
2519
return {
26-
scope: (span as SpanWithScopes)[SCOPE_ON_START_SPAN_FIELD],
27-
isolationScope: (span as SpanWithScopes)[ISOLATION_SCOPE_ON_START_SPAN_FIELD],
20+
scope: SPAN_TO_SCOPE_MAP.get(span),
21+
isolationScope: SPAN_TO_ISOLATION_SCOPE_MAP.get(span),
2822
};
2923
}

packages/core/src/utils/spanUtils.ts

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import type {
2121
} from '../types-hoist';
2222
import type { SpanLink, SpanLinkJSON } from '../types-hoist/link';
2323
import { consoleSandbox } from '../utils-hoist/logger';
24-
import { addNonEnumerableProperty, dropUndefinedKeys } from '../utils-hoist/object';
24+
import { dropUndefinedKeys } from '../utils-hoist/object';
2525
import { generateSpanId } from '../utils-hoist/propagationContext';
2626
import { timestampInSeconds } from '../utils-hoist/time';
2727
import { generateSentryTraceHeader } from '../utils-hoist/tracing';
@@ -223,55 +223,54 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef
223223
return status.message || 'unknown_error';
224224
}
225225

226-
const CHILD_SPANS_FIELD = '_sentryChildSpans';
227-
const ROOT_SPAN_FIELD = '_sentryRootSpan';
228-
229-
type SpanWithPotentialChildren = Span & {
230-
[CHILD_SPANS_FIELD]?: Set<Span>;
231-
[ROOT_SPAN_FIELD]?: Span;
232-
};
226+
const SPAN_TO_ROOT_SPAN_MAP = new WeakMap<Span, Span>();
227+
const SPAN_TO_CHILD_SPANS_MAP = new WeakMap<Span, Set<Span>>();
233228

234229
/**
235230
* Adds an opaque child span reference to a span.
236231
*/
237-
export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
232+
export function addChildSpanToSpan(span: Span, childSpan: Span): void {
238233
// We store the root span reference on the child span
239234
// We need this for `getRootSpan()` to work
240-
const rootSpan = span[ROOT_SPAN_FIELD] || span;
241-
addNonEnumerableProperty(childSpan as SpanWithPotentialChildren, ROOT_SPAN_FIELD, rootSpan);
235+
const rootSpan = SPAN_TO_ROOT_SPAN_MAP.get(span) || span;
236+
SPAN_TO_ROOT_SPAN_MAP.set(childSpan, rootSpan);
242237

243238
// We store a list of child spans on the parent span
244239
// We need this for `getSpanDescendants()` to work
245-
if (span[CHILD_SPANS_FIELD]) {
246-
span[CHILD_SPANS_FIELD].add(childSpan);
240+
const childSpans = SPAN_TO_CHILD_SPANS_MAP.get(span);
241+
if (childSpans) {
242+
childSpans.add(childSpan);
247243
} else {
248-
addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([childSpan]));
244+
SPAN_TO_CHILD_SPANS_MAP.set(span, new Set([childSpan]));
249245
}
250246
}
251247

252248
/** This is only used internally by Idle Spans. */
253-
export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
254-
if (span[CHILD_SPANS_FIELD]) {
255-
span[CHILD_SPANS_FIELD].delete(childSpan);
249+
export function removeChildSpanFromSpan(span: Span, childSpan: Span): void {
250+
const childSpans = SPAN_TO_CHILD_SPANS_MAP.get(span);
251+
if (childSpans) {
252+
childSpans.delete(childSpan);
256253
}
257254
}
258255

259256
/**
260257
* Returns an array of the given span and all of its descendants.
261258
*/
262-
export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] {
259+
export function getSpanDescendants(span: Span): Span[] {
263260
const resultSet = new Set<Span>();
264261

265-
function addSpanChildren(span: SpanWithPotentialChildren): void {
262+
function addSpanChildren(span: Span): void {
266263
// This exit condition is required to not infinitely loop in case of a circular dependency.
267264
if (resultSet.has(span)) {
268265
return;
269266
// We want to ignore unsampled spans (e.g. non recording spans)
270267
} else if (spanIsSampled(span)) {
271268
resultSet.add(span);
272-
const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : [];
273-
for (const childSpan of childSpans) {
274-
addSpanChildren(childSpan);
269+
const childSpans = SPAN_TO_CHILD_SPANS_MAP.get(span);
270+
if (childSpans) {
271+
for (const childSpan of childSpans) {
272+
addSpanChildren(childSpan);
273+
}
275274
}
276275
}
277276
}
@@ -284,8 +283,8 @@ export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] {
284283
/**
285284
* Returns the root span of a given span.
286285
*/
287-
export function getRootSpan(span: SpanWithPotentialChildren): Span {
288-
return span[ROOT_SPAN_FIELD] || span;
286+
export function getRootSpan(span: Span): Span {
287+
return SPAN_TO_ROOT_SPAN_MAP.get(span) || span;
289288
}
290289

291290
/**

packages/core/test/lib/tracing/trace.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ describe('startSpan', () => {
687687
expect.assertions(1);
688688
startSpan({ name: 'outer' }, (outerSpan: any) => {
689689
startSpan({ name: 'inner' }, innerSpan => {
690-
const childSpans = Array.from(outerSpan._sentryChildSpans);
690+
const childSpans = getSpanDescendants(outerSpan);
691691
expect(childSpans).toContain(innerSpan);
692692
});
693693
});
@@ -1178,7 +1178,7 @@ describe('startSpanManual', () => {
11781178
expect.assertions(1);
11791179
startSpan({ name: 'outer' }, (outerSpan: any) => {
11801180
startSpanManual({ name: 'inner' }, innerSpan => {
1181-
const childSpans = Array.from(outerSpan._sentryChildSpans);
1181+
const childSpans = getSpanDescendants(outerSpan);
11821182
expect(childSpans).toContain(innerSpan);
11831183
});
11841184
});
@@ -1591,7 +1591,7 @@ describe('startInactiveSpan', () => {
15911591
expect.assertions(1);
15921592
startSpan({ name: 'outer' }, (outerSpan: any) => {
15931593
const innerSpan = startInactiveSpan({ name: 'inner' });
1594-
const childSpans = Array.from(outerSpan._sentryChildSpans);
1594+
const childSpans = getSpanDescendants(outerSpan);
15951595
expect(childSpans).toContain(innerSpan);
15961596
});
15971597
});

packages/opentelemetry/src/utils/contextData.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
import type { Context } from '@opentelemetry/api';
22
import type { Scope } from '@sentry/core';
3-
import { addNonEnumerableProperty } from '@sentry/core';
43
import { SENTRY_SCOPES_CONTEXT_KEY } from '../constants';
54
import type { CurrentScopes } from '../types';
65

7-
const SCOPE_CONTEXT_FIELD = '_scopeContext';
8-
96
/**
107
* Try to get the current scopes from the given OTEL context.
118
* This requires a Context Manager that was wrapped with getWrappedContextManager.
@@ -22,17 +19,19 @@ export function setScopesOnContext(context: Context, scopes: CurrentScopes): Con
2219
return context.setValue(SENTRY_SCOPES_CONTEXT_KEY, scopes);
2320
}
2421

22+
const SCOPE_TO_CONTEXT_MAP = new WeakMap<Scope, Context>();
23+
2524
/**
2625
* Set the context on the scope so we can later look it up.
2726
* We need this to get the context from the scope in the `trace` functions.
2827
*/
2928
export function setContextOnScope(scope: Scope, context: Context): void {
30-
addNonEnumerableProperty(scope, SCOPE_CONTEXT_FIELD, context);
29+
SCOPE_TO_CONTEXT_MAP.set(scope, context);
3130
}
3231

3332
/**
3433
* Get the context related to a scope.
3534
*/
3635
export function getContextFromScope(scope: Scope): Context | undefined {
37-
return (scope as { [SCOPE_CONTEXT_FIELD]?: Context })[SCOPE_CONTEXT_FIELD];
36+
return SCOPE_TO_CONTEXT_MAP.get(scope);
3837
}

0 commit comments

Comments
 (0)