diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 97f639cad216..cb1be728f26f 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -424,31 +424,6 @@ export const browserTracingIntegration = ((options: Partial { - addPerformanceEntries(span, { - ignoreResourceSpans, - spanStreamingEnabled: hasSpanStreamingEnabled(client), - }); - setActiveIdleSpan(client, undefined); - - // A trace should stay consistent over the entire timespan of one route - even after the pageload/navigation ended. - // Only when another navigation happens, we want to create a new trace. - // This way, e.g. errors that occur after the pageload span ended are still associated to the pageload trace. - const scope = getCurrentScope(); - const oldPropagationContext = scope.getPropagationContext(); - - scope.setPropagationContext({ - ...oldPropagationContext, - traceId: idleSpan.spanContext().traceId, - sampled: spanIsSampled(idleSpan), - dsc: getDynamicSamplingContextFromSpan(span), - }); - - if (isPageloadSpan) { - // clean up the stored pageload span on the intergration. - _pageloadSpan = undefined; - } - }, trimIdleSpanEndTimestamp: !enableReportPageLoaded, }); @@ -519,6 +494,39 @@ export const browserTracingIntegration = ((options: Partial { + // Interaction idle spans also flow through this hook, but the route bookkeeping below only + // applies to the pageload/navigation span. We identify it by reference rather than by op: + // only the route span is registered as the active idle span. + if (getActiveIdleSpan(client) !== span) { + return; + } + + addPerformanceEntries(span, { + ignoreResourceSpans, + spanStreamingEnabled: hasSpanStreamingEnabled(client), + }); + setActiveIdleSpan(client, undefined); + + // A trace should stay consistent over the entire timespan of one route - even after the pageload/navigation ended. + // Only when another navigation happens, we want to create a new trace. + // This way, e.g. errors that occur after the pageload span ended are still associated to the pageload trace. + const scope = getCurrentScope(); + const oldPropagationContext = scope.getPropagationContext(); + + scope.setPropagationContext({ + ...oldPropagationContext, + traceId: span.spanContext().traceId, + sampled: spanIsSampled(span), + dsc: getDynamicSamplingContextFromSpan(span), + }); + + if (_pageloadSpan === span) { + // clean up the stored pageload span on the integration. + _pageloadSpan = undefined; + } + }); + client.on('startNavigationSpan', (startSpanOptions, navigationOptions) => { if (getClient() !== client) { return; diff --git a/packages/core/src/tracing/idleSpan.ts b/packages/core/src/tracing/idleSpan.ts index d38576f15e47..4c26874ad7fe 100644 --- a/packages/core/src/tracing/idleSpan.ts +++ b/packages/core/src/tracing/idleSpan.ts @@ -75,9 +75,6 @@ interface IdleSpanOptions { */ disableAutoFinish?: boolean; - /** Allows to configure a hook that is called when the idle span is ended, before it is processed. */ - beforeSpanEnd?: (span: Span) => void; - /** * If set to `true`, the idle span will be trimmed to the latest span end timestamp of its children. * @@ -114,7 +111,6 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti idleTimeout = TRACING_DEFAULTS.idleTimeout, finalTimeout = TRACING_DEFAULTS.finalTimeout, childSpanTimeout = TRACING_DEFAULTS.childSpanTimeout, - beforeSpanEnd, trimIdleSpanEndTimestamp = true, } = options; @@ -142,10 +138,6 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti apply(target, thisArg, args: Parameters) { client.emit('beforeIdleSpanEnd', span); - if (beforeSpanEnd) { - beforeSpanEnd(span); - } - // If the span is non-recording, nothing more to do here... // This is the case if tracing is enabled but this specific span was not sampled if (spanIsNonRecordingSpan(thisArg)) { diff --git a/packages/core/test/lib/tracing/idleSpan.test.ts b/packages/core/test/lib/tracing/idleSpan.test.ts index c7cd1433a230..3d7d7f586ecf 100644 --- a/packages/core/test/lib/tracing/idleSpan.test.ts +++ b/packages/core/test/lib/tracing/idleSpan.test.ts @@ -214,21 +214,23 @@ describe('startIdleSpan', () => { }); }); - it('calls beforeSpanEnd callback before finishing', () => { - const beforeSpanEnd = vi.fn(); - const idleSpan = startIdleSpan({ name: 'foo' }, { beforeSpanEnd }); + it('emits beforeIdleSpanEnd hook before finishing', () => { + const beforeIdleSpanEnd = vi.fn(); + getClient()!.on('beforeIdleSpanEnd', beforeIdleSpanEnd); + + const idleSpan = startIdleSpan({ name: 'foo' }); expect(idleSpan).toBeDefined(); - expect(beforeSpanEnd).not.toHaveBeenCalled(); + expect(beforeIdleSpanEnd).not.toHaveBeenCalled(); startSpan({ name: 'inner' }, () => {}); vi.runOnlyPendingTimers(); - expect(beforeSpanEnd).toHaveBeenCalledTimes(1); - expect(beforeSpanEnd).toHaveBeenLastCalledWith(idleSpan); + expect(beforeIdleSpanEnd).toHaveBeenCalledTimes(1); + expect(beforeIdleSpanEnd).toHaveBeenLastCalledWith(idleSpan); }); - it('allows to mutate idle span in beforeSpanEnd before it is sent', () => { + it('allows to mutate idle span in beforeIdleSpanEnd before it is sent', () => { const transactions: Event[] = []; const beforeSendTransaction = vi.fn(event => { transactions.push(event); @@ -246,16 +248,18 @@ describe('startIdleSpan', () => { // We want to accommodate a bit of drift there, so we ensure this starts earlier... const baseTimeInSeconds = Math.floor(Date.now() / 1000) - 9999; - const beforeSpanEnd = vi.fn((span: Span) => { + const beforeIdleSpanEnd = vi.fn((span: Span) => { span.setAttribute('foo', 'bar'); // Try adding a child here - we do this in browser tracing... - const inner = startInactiveSpan({ name: 'from beforeSpanEnd', startTime: baseTimeInSeconds }); + const inner = startInactiveSpan({ name: 'from beforeIdleSpanEnd', startTime: baseTimeInSeconds }); inner.end(baseTimeInSeconds + 1); }); - const idleSpan = startIdleSpan({ name: 'idle span', startTime: baseTimeInSeconds }, { beforeSpanEnd }); + client.on('beforeIdleSpanEnd', beforeIdleSpanEnd); + + const idleSpan = startIdleSpan({ name: 'idle span', startTime: baseTimeInSeconds }); expect(idleSpan).toBeDefined(); - expect(beforeSpanEnd).not.toHaveBeenCalled(); + expect(beforeIdleSpanEnd).not.toHaveBeenCalled(); vi.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout + 1); vi.runOnlyPendingTimers(); @@ -270,7 +274,7 @@ describe('startIdleSpan', () => { const transaction = transactions[0]!; expect(transaction.start_timestamp).toBe(baseTimeInSeconds); - // It considers the end time of the span we added in beforeSpanEnd + // It considers the end time of the span we added in beforeIdleSpanEnd expect(transaction.timestamp).toBe(baseTimeInSeconds + 1); expect(transaction.contexts?.trace?.data).toEqual( @@ -279,7 +283,7 @@ describe('startIdleSpan', () => { }), ); expect(transaction.spans).toHaveLength(1); - expect(transaction.spans).toEqual([expect.objectContaining({ description: 'from beforeSpanEnd' })]); + expect(transaction.spans).toEqual([expect.objectContaining({ description: 'from beforeIdleSpanEnd' })]); }); it('runs beforeIdleSpanEnd before trimming the idle span', () => {