From 330c80c5b70118c72916ac8aa4bbf77ce5a3fed1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 6 Mar 2024 11:28:19 +0000 Subject: [PATCH] ref: Make browser metrics accept spans --- packages/core/src/tracing/sentrySpan.ts | 12 + .../src/browser/metrics/index.ts | 157 ++++++----- .../src/browser/metrics/utils.ts | 38 ++- .../test/browser/metrics/index.test.ts | 247 ++++++++++++------ .../test/browser/metrics/utils.test.ts | 63 +++-- 5 files changed, 321 insertions(+), 196 deletions(-) diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 36a7acc403b0..40b7e1af500a 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -371,6 +371,18 @@ export class SentrySpan implements Span { Object.keys(attributes).forEach(key => this.setAttribute(key, attributes[key])); } + /** + * This should generally not be used, + * but we need it for browser tracing where we want to adjust the start time afterwards. + * USE THIS WITH CAUTION! + * + * @hidden + * @internal + */ + public updateStartTime(timeInput: SpanTimeInput): void { + this._startTime = spanTimeInputToSeconds(timeInput); + } + /** * @inheritDoc */ diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index 9bd3a19e770d..6a25bd00c92f 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -1,7 +1,8 @@ /* eslint-disable max-lines */ -import type { IdleTransaction, Transaction } from '@sentry/core'; -import { getActiveTransaction, setMeasurement } from '@sentry/core'; -import type { Measurements, SpanContext } from '@sentry/types'; +import type { SentrySpan } from '@sentry/core'; +import { getActiveSpan, startInactiveSpan } from '@sentry/core'; +import { setMeasurement } from '@sentry/core'; +import type { Measurements, Span, StartSpanOptions } from '@sentry/types'; import { browserPerformanceTimeOrigin, getComponentName, htmlTreeAsString, logger, parseUrl } from '@sentry/utils'; import { spanToJSON } from '@sentry/core'; @@ -15,7 +16,7 @@ import { import { WINDOW } from '../types'; import { getVisibilityWatcher } from '../web-vitals/lib/getVisibilityWatcher'; import type { NavigatorDeviceMemory, NavigatorNetworkInformation } from '../web-vitals/types'; -import { _startChild, isMeasurementValue } from './utils'; +import { isMeasurementValue, startAndEndSpan } from './utils'; const MAX_INT_AS_BYTES = 2147483647; @@ -71,22 +72,21 @@ export function startTrackingWebVitals(): () => void { export function startTrackingLongTasks(): void { addPerformanceInstrumentationHandler('longtask', ({ entries }) => { for (const entry of entries) { - // eslint-disable-next-line deprecation/deprecation - const transaction = getActiveTransaction() as IdleTransaction | undefined; - if (!transaction) { + if (!getActiveSpan()) { return; } const startTime = msToSec((browserPerformanceTimeOrigin as number) + entry.startTime); const duration = msToSec(entry.duration); - // eslint-disable-next-line deprecation/deprecation - transaction.startChild({ + const span = startInactiveSpan({ name: 'Main UI thread blocked', op: 'ui.long-task', origin: 'auto.ui.browser.metrics', - startTimestamp: startTime, - endTimestamp: startTime + duration, + startTime, }); + if (span) { + span.end(startTime + duration); + } } }); } @@ -97,9 +97,7 @@ export function startTrackingLongTasks(): void { export function startTrackingInteractions(): void { addPerformanceInstrumentationHandler('event', ({ entries }) => { for (const entry of entries) { - // eslint-disable-next-line deprecation/deprecation - const transaction = getActiveTransaction() as IdleTransaction | undefined; - if (!transaction) { + if (!getActiveSpan()) { return; } @@ -107,21 +105,22 @@ export function startTrackingInteractions(): void { const startTime = msToSec((browserPerformanceTimeOrigin as number) + entry.startTime); const duration = msToSec(entry.duration); - const span: SpanContext = { + const spanOptions: StartSpanOptions = { name: htmlTreeAsString(entry.target), op: `ui.interaction.${entry.name}`, origin: 'auto.ui.browser.metrics', - startTimestamp: startTime, - endTimestamp: startTime + duration, + startTime: startTime, }; const componentName = getComponentName(entry.target); if (componentName) { - span.attributes = { 'ui.component_name': componentName }; + spanOptions.attributes = { 'ui.component_name': componentName }; } - // eslint-disable-next-line deprecation/deprecation - transaction.startChild(span); + const span = startInactiveSpan(spanOptions); + if (span) { + span.end(startTime + duration); + } } } }); @@ -171,8 +170,8 @@ function _trackFID(): () => void { }); } -/** Add performance related spans to a transaction */ -export function addPerformanceEntries(transaction: Transaction): void { +/** Add performance related spans to a span */ +export function addPerformanceEntries(span: Span): void { const performance = getBrowserPerformanceAPI(); if (!performance || !WINDOW.performance.getEntries || !browserPerformanceTimeOrigin) { // Gatekeeper if performance API not available @@ -187,7 +186,7 @@ export function addPerformanceEntries(transaction: Transaction): void { let responseStartTimestamp: number | undefined; let requestStartTimestamp: number | undefined; - const { op, start_timestamp: transactionStartTime } = spanToJSON(transaction); + const { op, start_timestamp: transactionStartTime } = spanToJSON(span); // eslint-disable-next-line @typescript-eslint/no-explicit-any performanceEntries.slice(_performanceCursor).forEach((entry: Record) => { @@ -200,7 +199,7 @@ export function addPerformanceEntries(transaction: Transaction): void { switch (entry.entryType) { case 'navigation': { - _addNavigationSpans(transaction, entry, timeOrigin); + _addNavigationSpans(span, entry, timeOrigin); responseStartTimestamp = timeOrigin + msToSec(entry.responseStart); requestStartTimestamp = timeOrigin + msToSec(entry.requestStart); break; @@ -208,7 +207,7 @@ export function addPerformanceEntries(transaction: Transaction): void { case 'mark': case 'paint': case 'measure': { - _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); + _addMeasureSpans(span, entry, startTime, duration, timeOrigin); // capture web vitals const firstHidden = getVisibilityWatcher(); @@ -226,7 +225,7 @@ export function addPerformanceEntries(transaction: Transaction): void { break; } case 'resource': { - _addResourceSpans(transaction, entry, entry.name as string, startTime, duration, timeOrigin); + _addResourceSpans(span, entry, entry.name as string, startTime, duration, timeOrigin); break; } default: @@ -236,7 +235,7 @@ export function addPerformanceEntries(transaction: Transaction): void { _performanceCursor = Math.max(performanceEntries.length - 1, 0); - _trackNavigator(transaction); + _trackNavigator(span); // Measurements are only available for pageload transactions if (op === 'pageload') { @@ -247,8 +246,8 @@ export function addPerformanceEntries(transaction: Transaction): void { return; } // The web vitals, fcp, fp, lcp, and ttfb, all measure relative to timeOrigin. - // Unfortunately, timeOrigin is not captured within the transaction span data, so these web vitals will need - // to be adjusted to be relative to transaction.startTimestamp. + // Unfortunately, timeOrigin is not captured within the span span data, so these web vitals will need + // to be adjusted to be relative to span.startTimestamp. const oldValue = _measurements[name].value; const measurementTimestamp = timeOrigin + msToSec(oldValue); @@ -263,12 +262,10 @@ export function addPerformanceEntries(transaction: Transaction): void { const fidMark = _measurements['mark.fid']; if (fidMark && _measurements['fid']) { // create span for FID - _startChild(transaction, { + startAndEndSpan(span, fidMark.value, fidMark.value + msToSec(_measurements['fid'].value), { name: 'first input delay', - endTimestamp: fidMark.value + msToSec(_measurements['fid'].value), op: 'ui.action', origin: 'auto.ui.browser.metrics', - startTimestamp: fidMark.value, }); // Delete mark.fid as we don't want it to be part of final payload @@ -285,7 +282,7 @@ export function addPerformanceEntries(transaction: Transaction): void { setMeasurement(measurementName, _measurements[measurementName].value, _measurements[measurementName].unit); }); - _tagMetricInfo(transaction); + _tagMetricInfo(span); } _lcpEntry = undefined; @@ -295,7 +292,7 @@ export function addPerformanceEntries(transaction: Transaction): void { /** Create measure related spans */ export function _addMeasureSpans( - transaction: Transaction, + span: Span, // eslint-disable-next-line @typescript-eslint/no-explicit-any entry: Record, startTime: number, @@ -305,12 +302,10 @@ export function _addMeasureSpans( const measureStartTimestamp = timeOrigin + startTime; const measureEndTimestamp = measureStartTimestamp + duration; - _startChild(transaction, { + startAndEndSpan(span, measureStartTimestamp, measureEndTimestamp, { name: entry.name as string, - endTimestamp: measureEndTimestamp, op: entry.entryType as string, origin: 'auto.resource.browser.metrics', - startTimestamp: measureStartTimestamp, }); return measureStartTimestamp; @@ -318,19 +313,19 @@ export function _addMeasureSpans( /** Instrument navigation entries */ // eslint-disable-next-line @typescript-eslint/no-explicit-any -function _addNavigationSpans(transaction: Transaction, entry: Record, timeOrigin: number): void { +function _addNavigationSpans(span: Span, entry: Record, timeOrigin: number): void { ['unloadEvent', 'redirect', 'domContentLoadedEvent', 'loadEvent', 'connect'].forEach(event => { - _addPerformanceNavigationTiming(transaction, entry, event, timeOrigin); + _addPerformanceNavigationTiming(span, entry, event, timeOrigin); }); - _addPerformanceNavigationTiming(transaction, entry, 'secureConnection', timeOrigin, 'TLS/SSL', 'connectEnd'); - _addPerformanceNavigationTiming(transaction, entry, 'fetch', timeOrigin, 'cache', 'domainLookupStart'); - _addPerformanceNavigationTiming(transaction, entry, 'domainLookup', timeOrigin, 'DNS'); - _addRequest(transaction, entry, timeOrigin); + _addPerformanceNavigationTiming(span, entry, 'secureConnection', timeOrigin, 'TLS/SSL', 'connectEnd'); + _addPerformanceNavigationTiming(span, entry, 'fetch', timeOrigin, 'cache', 'domainLookupStart'); + _addPerformanceNavigationTiming(span, entry, 'domainLookup', timeOrigin, 'DNS'); + _addRequest(span, entry, timeOrigin); } /** Create performance navigation related spans */ function _addPerformanceNavigationTiming( - transaction: Transaction, + span: Span, // eslint-disable-next-line @typescript-eslint/no-explicit-any entry: Record, event: string, @@ -343,38 +338,42 @@ function _addPerformanceNavigationTiming( if (!start || !end) { return; } - _startChild(transaction, { + startAndEndSpan(span, timeOrigin + msToSec(start), timeOrigin + msToSec(end), { op: 'browser', origin: 'auto.browser.browser.metrics', name: name || event, - startTimestamp: timeOrigin + msToSec(start), - endTimestamp: timeOrigin + msToSec(end), }); } /** Create request and response related spans */ // eslint-disable-next-line @typescript-eslint/no-explicit-any -function _addRequest(transaction: Transaction, entry: Record, timeOrigin: number): void { +function _addRequest(span: Span, entry: Record, timeOrigin: number): void { if (entry.responseEnd) { // It is possible that we are collecting these metrics when the page hasn't finished loading yet, for example when the HTML slowly streams in. // In this case, ie. when the document request hasn't finished yet, `entry.responseEnd` will be 0. // In order not to produce faulty spans, where the end timestamp is before the start timestamp, we will only collect - // these spans when the responseEnd value is available. The backend (Relay) would drop the entire transaction if it contained faulty spans. - _startChild(transaction, { - op: 'browser', - origin: 'auto.browser.browser.metrics', - name: 'request', - startTimestamp: timeOrigin + msToSec(entry.requestStart as number), - endTimestamp: timeOrigin + msToSec(entry.responseEnd as number), - }); + // these spans when the responseEnd value is available. The backend (Relay) would drop the entire span if it contained faulty spans. + startAndEndSpan( + span, + timeOrigin + msToSec(entry.requestStart as number), + timeOrigin + msToSec(entry.responseEnd as number), + { + op: 'browser', + origin: 'auto.browser.browser.metrics', + name: 'request', + }, + ); - _startChild(transaction, { - op: 'browser', - origin: 'auto.browser.browser.metrics', - name: 'response', - startTimestamp: timeOrigin + msToSec(entry.responseStart as number), - endTimestamp: timeOrigin + msToSec(entry.responseEnd as number), - }); + startAndEndSpan( + span, + timeOrigin + msToSec(entry.responseStart as number), + timeOrigin + msToSec(entry.responseEnd as number), + { + op: 'browser', + origin: 'auto.browser.browser.metrics', + name: 'response', + }, + ); } } @@ -388,7 +387,7 @@ export interface ResourceEntry extends Record { /** Create resource-related spans */ export function _addResourceSpans( - transaction: Transaction, + span: Span, entry: ResourceEntry, resourceUrl: string, startTime: number, @@ -425,12 +424,10 @@ export function _addResourceSpans( const startTimestamp = timeOrigin + startTime; const endTimestamp = startTimestamp + duration; - _startChild(transaction, { + startAndEndSpan(span, startTimestamp, endTimestamp, { name: resourceUrl.replace(WINDOW.location.origin, ''), - endTimestamp, op: entry.initiatorType ? `resource.${entry.initiatorType}` : 'resource.other', origin: 'auto.resource.browser.metrics', - startTimestamp, data, }); } @@ -438,7 +435,7 @@ export function _addResourceSpans( /** * Capture the information of the user agent. */ -function _trackNavigator(transaction: Transaction): void { +function _trackNavigator(span: Span): void { const navigator = WINDOW.navigator as null | (Navigator & NavigatorNetworkInformation & NavigatorDeviceMemory); if (!navigator) { return; @@ -450,13 +447,13 @@ function _trackNavigator(transaction: Transaction): void { if (connection.effectiveType) { // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('effectiveConnectionType', connection.effectiveType); + (span as SentrySpan).setTag('effectiveConnectionType', connection.effectiveType); } if (connection.type) { // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('connectionType', connection.type); + (span as SentrySpan).setTag('connectionType', connection.type); } if (isMeasurementValue(connection.rtt)) { @@ -467,18 +464,18 @@ function _trackNavigator(transaction: Transaction): void { if (isMeasurementValue(navigator.deviceMemory)) { // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('deviceMemory', `${navigator.deviceMemory} GB`); + (span as SentrySpan).setTag('deviceMemory', `${navigator.deviceMemory} GB`); } if (isMeasurementValue(navigator.hardwareConcurrency)) { // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('hardwareConcurrency', String(navigator.hardwareConcurrency)); + (span as SentrySpan).setTag('hardwareConcurrency', String(navigator.hardwareConcurrency)); } } -/** Add LCP / CLS data to transaction to allow debugging */ -function _tagMetricInfo(transaction: Transaction): void { +/** Add LCP / CLS data to span to allow debugging */ +function _tagMetricInfo(span: Span): void { if (_lcpEntry) { DEBUG_BUILD && logger.log('[Measurements] Adding LCP Data'); @@ -487,25 +484,25 @@ function _tagMetricInfo(transaction: Transaction): void { if (_lcpEntry.element) { // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('lcp.element', htmlTreeAsString(_lcpEntry.element)); + (span as SentrySpan).setTag('lcp.element', htmlTreeAsString(_lcpEntry.element)); } if (_lcpEntry.id) { // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('lcp.id', _lcpEntry.id); + (span as SentrySpan).setTag('lcp.id', _lcpEntry.id); } if (_lcpEntry.url) { // Trim URL to the first 200 characters. // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('lcp.url', _lcpEntry.url.trim().slice(0, 200)); + (span as SentrySpan).setTag('lcp.url', _lcpEntry.url.trim().slice(0, 200)); } // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag('lcp.size', _lcpEntry.size); + (span as SentrySpan).setTag('lcp.size', _lcpEntry.size); } // See: https://developer.mozilla.org/en-US/docs/Web/API/LayoutShift @@ -514,7 +511,7 @@ function _tagMetricInfo(transaction: Transaction): void { _clsEntry.sources.forEach((source, index) => // TODO: Can we rewrite this to an attribute? // eslint-disable-next-line deprecation/deprecation - transaction.setTag(`cls.source.${index + 1}`, htmlTreeAsString(source.node)), + (span as SentrySpan).setTag(`cls.source.${index + 1}`, htmlTreeAsString(source.node)), ); } } @@ -542,7 +539,7 @@ export function _addTtfbToMeasurements( requestStartTimestamp: number | undefined, transactionStartTime: number | undefined, ): void { - // Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the + // Generate TTFB (Time to First Byte), which measured as the time between the beginning of the span and the // start of the response in milliseconds if (typeof responseStartTimestamp === 'number' && transactionStartTime) { DEBUG_BUILD && logger.log('[Measurements] Adding TTFB'); diff --git a/packages/tracing-internal/src/browser/metrics/utils.ts b/packages/tracing-internal/src/browser/metrics/utils.ts index 4669ff13b762..facb3f429e3a 100644 --- a/packages/tracing-internal/src/browser/metrics/utils.ts +++ b/packages/tracing-internal/src/browser/metrics/utils.ts @@ -1,5 +1,6 @@ -import type { Transaction } from '@sentry/core'; -import type { Span, SpanContext } from '@sentry/types'; +import type { SentrySpan } from '@sentry/core'; +import { spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core'; +import type { Span, SpanTimeInput, StartSpanOptions } from '@sentry/types'; /** * Checks if a given value is a valid measurement value. @@ -16,16 +17,31 @@ export function isMeasurementValue(value: unknown): value is number { * Note: this will not be possible anymore in v8, * unless we do some special handling for browser here... */ -export function _startChild(transaction: Transaction, { startTimestamp, ...ctx }: SpanContext): Span { - // eslint-disable-next-line deprecation/deprecation - if (startTimestamp && transaction.startTimestamp > startTimestamp) { - // eslint-disable-next-line deprecation/deprecation - transaction.startTimestamp = startTimestamp; +export function startAndEndSpan( + parentSpan: Span, + startTimeInSeconds: number, + endTime: SpanTimeInput, + { ...ctx }: StartSpanOptions, +): Span | undefined { + const parentStartTime = spanToJSON(parentSpan).start_timestamp; + if (parentStartTime && parentStartTime > startTimeInSeconds) { + // We can only do this for SentrySpans... + if (typeof (parentSpan as Partial).updateStartTime === 'function') { + (parentSpan as SentrySpan).updateStartTime(startTimeInSeconds); + } } - // eslint-disable-next-line deprecation/deprecation - return transaction.startChild({ - startTimestamp, - ...ctx, + // The return value only exists for tests + return withActiveSpan(parentSpan, () => { + const span = startInactiveSpan({ + startTime: startTimeInSeconds, + ...ctx, + }); + + if (span) { + span.end(endTime); + } + + return span; }); } diff --git a/packages/tracing-internal/test/browser/metrics/index.test.ts b/packages/tracing-internal/test/browser/metrics/index.test.ts index abeddad7c46c..84b3950aeeec 100644 --- a/packages/tracing-internal/test/browser/metrics/index.test.ts +++ b/packages/tracing-internal/test/browser/metrics/index.test.ts @@ -1,8 +1,19 @@ -import { Transaction } from '../../../src'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SentrySpan, + getClient, + getCurrentScope, + getIsolationScope, + setCurrentClient, + spanToJSON, +} from '@sentry/core'; +import type { Span } from '@sentry/types'; import type { ResourceEntry } from '../../../src/browser/metrics'; import { _addTtfbToMeasurements } from '../../../src/browser/metrics'; import { _addMeasureSpans, _addResourceSpans } from '../../../src/browser/metrics'; import { WINDOW } from '../../../src/browser/types'; +import { TestClient, getDefaultClientOptions } from '../../utils/TestClient'; const mockWindowLocation = { ancestorOrigins: {}, @@ -22,15 +33,28 @@ const originalLocation = WINDOW.location; const resourceEntryName = 'https://example.com/assets/to/css'; describe('_addMeasureSpans', () => { - // eslint-disable-next-line deprecation/deprecation - const transaction = new Transaction({ op: 'pageload', name: '/' }); + const span = new SentrySpan({ op: 'pageload', name: '/' }); beforeEach(() => { - // eslint-disable-next-line deprecation/deprecation - transaction.startChild = jest.fn(); + getCurrentScope().clear(); + getIsolationScope().clear(); + + const client = new TestClient( + getDefaultClientOptions({ + tracesSampleRate: 1, + }), + ); + setCurrentClient(client); + client.init(); }); - it('adds measure spans to a transaction', () => { + it('adds measure spans to a span', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry: Omit = { entryType: 'measure', name: 'measure-1', @@ -43,25 +67,27 @@ describe('_addMeasureSpans', () => { const startTime = 23; const duration = 356; - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(0); - _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(1); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenLastCalledWith({ - name: 'measure-1', - startTimestamp: timeOrigin + startTime, - endTimestamp: timeOrigin + startTime + duration, - op: 'measure', - origin: 'auto.resource.browser.metrics', - }); + _addMeasureSpans(span, entry, startTime, duration, timeOrigin); + + expect(spans).toHaveLength(1); + expect(spanToJSON(spans[0]!)).toEqual( + expect.objectContaining({ + description: 'measure-1', + start_timestamp: timeOrigin + startTime, + timestamp: timeOrigin + startTime + duration, + op: 'measure', + origin: 'auto.resource.browser.metrics', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'measure', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics', + }, + }), + ); }); }); describe('_addResourceSpans', () => { - // eslint-disable-next-line deprecation/deprecation - const transaction = new Transaction({ op: 'pageload', name: '/' }); + const span = new SentrySpan({ op: 'pageload', name: '/' }); beforeAll(() => { setGlobalLocation(mockWindowLocation); @@ -72,12 +98,25 @@ describe('_addResourceSpans', () => { }); beforeEach(() => { - // eslint-disable-next-line deprecation/deprecation - transaction.startChild = jest.fn(); + getCurrentScope().clear(); + getIsolationScope().clear(); + + const client = new TestClient( + getDefaultClientOptions({ + tracesSampleRate: 1, + }), + ); + setCurrentClient(client); + client.init(); }); - // We already track xhr, we don't need to use it('does not create spans for xmlhttprequest', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry: ResourceEntry = { initiatorType: 'xmlhttprequest', transferSize: 256, @@ -85,13 +124,18 @@ describe('_addResourceSpans', () => { decodedBodySize: 256, renderBlockingStatus: 'non-blocking', }; - _addResourceSpans(transaction, entry, resourceEntryName, 123, 456, 100); + _addResourceSpans(span, entry, resourceEntryName, 123, 456, 100); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(0); + expect(spans).toHaveLength(0); }); it('does not create spans for fetch', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry: ResourceEntry = { initiatorType: 'fetch', transferSize: 256, @@ -99,13 +143,18 @@ describe('_addResourceSpans', () => { decodedBodySize: 256, renderBlockingStatus: 'non-blocking', }; - _addResourceSpans(transaction, entry, 'https://example.com/assets/to/me', 123, 456, 100); + _addResourceSpans(span, entry, 'https://example.com/assets/to/me', 123, 456, 100); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(0); + expect(spans).toHaveLength(0); }); it('creates spans for resource spans', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry: ResourceEntry = { initiatorType: 'css', transferSize: 256, @@ -118,30 +167,38 @@ describe('_addResourceSpans', () => { const startTime = 23; const duration = 356; - _addResourceSpans(transaction, entry, resourceEntryName, startTime, duration, timeOrigin); - - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(1); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenLastCalledWith({ - data: { - ['http.decoded_response_content_length']: entry.decodedBodySize, - ['http.response_content_length']: entry.encodedBodySize, - ['http.response_transfer_size']: entry.transferSize, - ['resource.render_blocking_status']: entry.renderBlockingStatus, - ['url.scheme']: 'https', - ['server.address']: 'example.com', - ['url.same_origin']: true, - }, - name: '/assets/to/css', - endTimestamp: timeOrigin + startTime + duration, - op: 'resource.css', - origin: 'auto.resource.browser.metrics', - startTimestamp: timeOrigin + startTime, - }); + _addResourceSpans(span, entry, resourceEntryName, startTime, duration, timeOrigin); + + expect(spans).toHaveLength(1); + expect(spanToJSON(spans[0]!)).toEqual( + expect.objectContaining({ + description: '/assets/to/css', + start_timestamp: timeOrigin + startTime, + timestamp: timeOrigin + startTime + duration, + op: 'resource.css', + origin: 'auto.resource.browser.metrics', + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.css', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics', + ['http.decoded_response_content_length']: entry.decodedBodySize, + ['http.response_content_length']: entry.encodedBodySize, + ['http.response_transfer_size']: entry.transferSize, + ['resource.render_blocking_status']: entry.renderBlockingStatus, + ['url.scheme']: 'https', + ['server.address']: 'example.com', + ['url.same_origin']: true, + }, + }), + ); }); it('creates a variety of resource spans', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const table = [ { initiatorType: undefined, @@ -164,23 +221,25 @@ describe('_addResourceSpans', () => { op: 'resource.script', }, ]; - - for (const { initiatorType, op } of table) { + for (let i = 0; i < table.length; i++) { + const { initiatorType, op } = table[i]; const entry: ResourceEntry = { initiatorType, }; - _addResourceSpans(transaction, entry, 'https://example.com/assets/to/me', 123, 234, 465); - - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenLastCalledWith( - expect.objectContaining({ - op, - }), - ); + _addResourceSpans(span, entry, 'https://example.com/assets/to/me', 123, 234, 465); + + expect(spans).toHaveLength(i + 1); + expect(spanToJSON(spans[i]!)).toEqual(expect.objectContaining({ op })); } }); it('allows for enter size of 0', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry: ResourceEntry = { initiatorType: 'css', transferSize: 0, @@ -189,14 +248,14 @@ describe('_addResourceSpans', () => { renderBlockingStatus: 'non-blocking', }; - _addResourceSpans(transaction, entry, resourceEntryName, 100, 23, 345); + _addResourceSpans(span, entry, resourceEntryName, 100, 23, 345); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(1); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenLastCalledWith( + expect(spans).toHaveLength(1); + expect(spanToJSON(spans[0])).toEqual( expect.objectContaining({ data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.css', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics', ['http.decoded_response_content_length']: entry.decodedBodySize, ['http.response_content_length']: entry.encodedBodySize, ['http.response_transfer_size']: entry.transferSize, @@ -210,6 +269,12 @@ describe('_addResourceSpans', () => { }); it('does not attach resource sizes that exceed MAX_INT bytes', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry: ResourceEntry = { initiatorType: 'css', transferSize: 2147483647, @@ -217,19 +282,23 @@ describe('_addResourceSpans', () => { decodedBodySize: 2147483647, }; - _addResourceSpans(transaction, entry, resourceEntryName, 100, 23, 345); + _addResourceSpans(span, entry, resourceEntryName, 100, 23, 345); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(1); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenLastCalledWith( + expect(spans).toHaveLength(1); + expect(spanToJSON(spans[0])).toEqual( expect.objectContaining({ - data: { 'server.address': 'example.com', 'url.same_origin': true, 'url.scheme': 'https' }, - name: '/assets/to/css', - endTimestamp: 468, + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.css', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics', + 'server.address': 'example.com', + 'url.same_origin': true, + 'url.scheme': 'https', + }, + description: '/assets/to/css', + timestamp: 468, op: 'resource.css', origin: 'auto.resource.browser.metrics', - startTimestamp: 445, + start_timestamp: 445, }), ); }); @@ -237,6 +306,12 @@ describe('_addResourceSpans', () => { // resource sizes can be set as null on some browsers // https://github.com/getsentry/sentry/pull/60601 it('does not attach null resource sizes', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + const entry = { initiatorType: 'css', transferSize: null, @@ -244,19 +319,23 @@ describe('_addResourceSpans', () => { decodedBodySize: null, } as unknown as ResourceEntry; - _addResourceSpans(transaction, entry, resourceEntryName, 100, 23, 345); + _addResourceSpans(span, entry, resourceEntryName, 100, 23, 345); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenCalledTimes(1); - // eslint-disable-next-line @typescript-eslint/unbound-method, deprecation/deprecation - expect(transaction.startChild).toHaveBeenLastCalledWith( + expect(spans).toHaveLength(1); + expect(spanToJSON(spans[0])).toEqual( expect.objectContaining({ - data: { 'server.address': 'example.com', 'url.same_origin': true, 'url.scheme': 'https' }, - name: '/assets/to/css', - endTimestamp: 468, + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.css', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics', + 'server.address': 'example.com', + 'url.same_origin': true, + 'url.scheme': 'https', + }, + description: '/assets/to/css', + timestamp: 468, op: 'resource.css', origin: 'auto.resource.browser.metrics', - startTimestamp: 445, + start_timestamp: 445, }), ); }); diff --git a/packages/tracing-internal/test/browser/metrics/utils.test.ts b/packages/tracing-internal/test/browser/metrics/utils.test.ts index 9cbae997cd7e..ad9d0dc801f8 100644 --- a/packages/tracing-internal/test/browser/metrics/utils.test.ts +++ b/packages/tracing-internal/test/browser/metrics/utils.test.ts @@ -1,15 +1,38 @@ -import { SentrySpan, Transaction, spanToJSON } from '@sentry/core'; -import { _startChild } from '../../../src/browser/metrics/utils'; +import { + SentrySpan, + addTracingExtensions, + getCurrentScope, + getIsolationScope, + setCurrentClient, + spanToJSON, +} from '@sentry/core'; +import { startAndEndSpan } from '../../../src/browser/metrics/utils'; +import { TestClient, getDefaultClientOptions } from '../../utils/TestClient'; + +describe('startAndEndSpan()', () => { + beforeEach(() => { + addTracingExtensions(); + + getCurrentScope().clear(); + getIsolationScope().clear(); + + const client = new TestClient( + getDefaultClientOptions({ + tracesSampleRate: 1, + }), + ); + setCurrentClient(client); + client.init(); + }); -describe('_startChild()', () => { it('creates a span with given properties', () => { - // eslint-disable-next-line deprecation/deprecation - const transaction = new Transaction({ name: 'test' }); - const span = _startChild(transaction, { + const parentSpan = new SentrySpan({ name: 'test' }); + const span = startAndEndSpan(parentSpan, 100, 200, { name: 'evaluation', op: 'script', - }); + })!; + expect(span).toBeDefined(); expect(span).toBeInstanceOf(SentrySpan); expect(spanToJSON(span).description).toBe('evaluation'); expect(spanToJSON(span).op).toBe('script'); @@ -17,28 +40,26 @@ describe('_startChild()', () => { }); it('adjusts the start timestamp if child span starts before transaction', () => { - // eslint-disable-next-line deprecation/deprecation - const transaction = new Transaction({ name: 'test', startTimestamp: 123 }); - const span = _startChild(transaction, { + const parentSpan = new SentrySpan({ name: 'test', startTimestamp: 123 }); + const span = startAndEndSpan(parentSpan, 100, 200, { name: 'script.js', op: 'resource', - startTimestamp: 100, - }); + })!; - expect(spanToJSON(transaction).start_timestamp).toEqual(spanToJSON(span).start_timestamp); - expect(spanToJSON(transaction).start_timestamp).toEqual(100); + expect(span).toBeDefined(); + expect(spanToJSON(parentSpan).start_timestamp).toEqual(spanToJSON(span).start_timestamp); + expect(spanToJSON(parentSpan).start_timestamp).toEqual(100); }); it('does not adjust start timestamp if child span starts after transaction', () => { - // eslint-disable-next-line deprecation/deprecation - const transaction = new Transaction({ name: 'test', startTimestamp: 123 }); - const span = _startChild(transaction, { + const parentSpan = new SentrySpan({ name: 'test', startTimestamp: 123 }); + const span = startAndEndSpan(parentSpan, 150, 200, { name: 'script.js', op: 'resource', - startTimestamp: 150, - }); + })!; - expect(spanToJSON(transaction).start_timestamp).not.toEqual(spanToJSON(span).start_timestamp); - expect(spanToJSON(transaction).start_timestamp).toEqual(123); + expect(span).toBeDefined(); + expect(spanToJSON(parentSpan).start_timestamp).not.toEqual(spanToJSON(span).start_timestamp); + expect(spanToJSON(parentSpan).start_timestamp).toEqual(123); }); });