From 5bc8215e73623d5e24586baaf6ee5f96e6c8638e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 17 May 2022 19:11:24 -0400 Subject: [PATCH 1/3] ref(tracing): Reduce metrics bundle size --- .../tracing/src/browser/browsertracing.ts | 12 +- packages/tracing/src/browser/metrics.ts | 567 +++++++++--------- packages/tracing/test/browser/metrics.test.ts | 91 +-- 3 files changed, 335 insertions(+), 335 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 780edf9052d4..f18785783472 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -7,7 +7,7 @@ import { startIdleTransaction } from '../hubextensions'; import { DEFAULT_FINAL_TIMEOUT, DEFAULT_IDLE_TIMEOUT } from '../idletransaction'; import { extractTraceparentData } from '../utils'; import { registerBackgroundTabDetection } from './backgroundtab'; -import { MetricsInstrumentation } from './metrics'; +import { addPerformanceEntries, startTrackingWebVitals } from './metrics'; import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests, @@ -126,8 +126,6 @@ export class BrowserTracing implements Integration { private _getCurrentHub?: () => Hub; - private readonly _metrics: MetricsInstrumentation; - private readonly _emitOptionsWarning?: boolean; public constructor(_options?: Partial) { @@ -148,7 +146,7 @@ export class BrowserTracing implements Integration { }; const { _metricOptions } = this.options; - this._metrics = new MetricsInstrumentation(_metricOptions && _metricOptions._reportAllChanges); + startTrackingWebVitals(_metricOptions && _metricOptions._reportAllChanges); } /** @@ -235,7 +233,11 @@ export class BrowserTracing implements Integration { { location }, // for use in the tracesSampler ); idleTransaction.registerBeforeFinishCallback(transaction => { - this._metrics.addPerformanceEntries(transaction); + addPerformanceEntries(transaction); + transaction.setTag( + 'sentry_reportAllChanges', + Boolean(this.options._metricOptions && this.options._metricOptions._reportAllChanges), + ); }); return idleTransaction as Transaction; diff --git a/packages/tracing/src/browser/metrics.ts b/packages/tracing/src/browser/metrics.ts index 97d28806096d..20a464edbdc6 100644 --- a/packages/tracing/src/browser/metrics.ts +++ b/packages/tracing/src/browser/metrics.ts @@ -1,10 +1,8 @@ /* eslint-disable max-lines */ -/* eslint-disable @typescript-eslint/no-explicit-any */ -import { Measurements, SpanContext } from '@sentry/types'; +import { Measurements, Span, SpanContext } from '@sentry/types'; import { browserPerformanceTimeOrigin, getGlobalObject, htmlTreeAsString, isNodeEnv, logger } from '@sentry/utils'; import { IS_DEBUG_BUILD } from '../flags'; -import { Span } from '../span'; import { Transaction } from '../transaction'; import { msToSec } from '../utils'; import { getCLS, LayoutShift } from './web-vitals/getCLS'; @@ -15,275 +13,225 @@ import { NavigatorDeviceMemory, NavigatorNetworkInformation } from './web-vitals const global = getGlobalObject(); -/** Class tracking metrics */ -export class MetricsInstrumentation { - private _measurements: Measurements = {}; +function getBrowserPerformanceAPI(): false | Performance { + return !isNodeEnv() && global && global.document && global.performance; +} - private _performanceCursor: number = 0; - private _lcpEntry: LargestContentfulPaint | undefined; - private _clsEntry: LayoutShift | undefined; +let _performanceCursor: number = 0; - public constructor(private _reportAllChanges: boolean = false) { - if (!isNodeEnv() && global && global.performance && global.document) { - if (global.performance.mark) { - global.performance.mark('sentry-tracing-init'); - } +let _measurements: Measurements = {}; +let _lcpEntry: LargestContentfulPaint | undefined; +let _clsEntry: LayoutShift | undefined; - this._trackCLS(); - this._trackLCP(); - this._trackFID(); +/** + * Start tracking web vitals + */ +export function startTrackingWebVitals(reportAllChanges: boolean = false): void { + const performance = getBrowserPerformanceAPI(); + if (performance && browserPerformanceTimeOrigin) { + if (performance.mark) { + global.performance.mark('sentry-tracing-init'); } + _trackCLS(); + _trackLCP(reportAllChanges); + _trackFID(); } +} - /** Add performance related spans to a transaction */ - public addPerformanceEntries(transaction: Transaction): void { - if (!global || !global.performance || !global.performance.getEntries || !browserPerformanceTimeOrigin) { - // Gatekeeper if performance API not available +/** Starts tracking the Cumulative Layout Shift on the current page. */ +function _trackCLS(): void { + // See: + // https://web.dev/evolving-cls/ + // https://web.dev/cls-web-tooling/ + getCLS(metric => { + const entry = metric.entries.pop(); + if (!entry) { return; } - IS_DEBUG_BUILD && logger.log('[Tracing] Adding & adjusting spans using Performance API'); - - const timeOrigin = msToSec(browserPerformanceTimeOrigin); - - let responseStartTimestamp: number | undefined; - let requestStartTimestamp: number | undefined; - - global.performance - .getEntries() - .slice(this._performanceCursor) - .forEach((entry: Record) => { - const startTime = msToSec(entry.startTime as number); - const duration = msToSec(entry.duration as number); - - if (transaction.op === 'navigation' && timeOrigin + startTime < transaction.startTimestamp) { - return; - } - - switch (entry.entryType) { - case 'navigation': { - addNavigationSpans(transaction, entry, timeOrigin); - responseStartTimestamp = timeOrigin + msToSec(entry.responseStart as number); - requestStartTimestamp = timeOrigin + msToSec(entry.requestStart as number); - break; - } - case 'mark': - case 'paint': - case 'measure': { - const startTimestamp = addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); - // capture web vitals - - const firstHidden = getVisibilityWatcher(); - // Only report if the page wasn't hidden prior to the web vital. - const shouldRecord = entry.startTime < firstHidden.firstHiddenTime; - - if (entry.name === 'first-paint' && shouldRecord) { - IS_DEBUG_BUILD && logger.log('[Measurements] Adding FP'); - this._measurements['fp'] = { value: entry.startTime, unit: 'millisecond' }; - this._measurements['mark.fp'] = { value: startTimestamp, unit: 'second' }; - } - - if (entry.name === 'first-contentful-paint' && shouldRecord) { - IS_DEBUG_BUILD && logger.log('[Measurements] Adding FCP'); - this._measurements['fcp'] = { value: entry.startTime, unit: 'millisecond' }; - this._measurements['mark.fcp'] = { value: startTimestamp, unit: 'second' }; - } - - break; - } - case 'resource': { - const resourceName = (entry.name as string).replace(global.location.origin, ''); - addResourceSpans(transaction, entry, resourceName, startTime, duration, timeOrigin); - break; - } - default: - // Ignore other entry types. - } - }); - - this._performanceCursor = Math.max(performance.getEntries().length - 1, 0); - - this._trackNavigator(transaction); - - // Measurements are only available for pageload transactions - if (transaction.op === 'pageload') { - // normalize applicable web vital values to be relative to transaction.startTimestamp - - const timeOrigin = msToSec(browserPerformanceTimeOrigin); - - // Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the - // start of the response in milliseconds - if (typeof responseStartTimestamp === 'number') { - IS_DEBUG_BUILD && logger.log('[Measurements] Adding TTFB'); - this._measurements['ttfb'] = { - value: (responseStartTimestamp - transaction.startTimestamp) * 1000, - unit: 'millisecond', - }; - - if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) { - // Capture the time spent making the request and receiving the first byte of the response. - // This is the time between the start of the request and the start of the response in milliseconds. - this._measurements['ttfb.requestTime'] = { - value: (responseStartTimestamp - requestStartTimestamp) * 1000, - unit: 'second', - }; - } - } - - ['fcp', 'fp', 'lcp'].forEach(name => { - if (!this._measurements[name] || timeOrigin >= transaction.startTimestamp) { - return; - } + IS_DEBUG_BUILD && logger.log('[Measurements] Adding CLS'); + _measurements['cls'] = { value: metric.value, unit: 'millisecond' }; + _clsEntry = entry as LayoutShift; + }); +} - // 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. +/** Starts tracking the Largest Contentful Paint on the current page. */ +function _trackLCP(reportAllChanges: boolean): void { + getLCP(metric => { + const entry = metric.entries.pop(); + if (!entry) { + return; + } - const oldValue = this._measurements[name].value; - const measurementTimestamp = timeOrigin + msToSec(oldValue); - // normalizedValue should be in milliseconds - const normalizedValue = Math.abs((measurementTimestamp - transaction.startTimestamp) * 1000); + const timeOrigin = msToSec(browserPerformanceTimeOrigin as number); + const startTime = msToSec(entry.startTime); + IS_DEBUG_BUILD && logger.log('[Measurements] Adding LCP'); + _measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; + _measurements['mark.lcp'] = { value: timeOrigin + startTime, unit: 'second' }; + _lcpEntry = entry as LargestContentfulPaint; + }, reportAllChanges); +} - const delta = normalizedValue - oldValue; - IS_DEBUG_BUILD && - logger.log(`[Measurements] Normalized ${name} from ${oldValue} to ${normalizedValue} (${delta})`); +/** Starts tracking the First Input Delay on the current page. */ +function _trackFID(): void { + getFID(metric => { + const entry = metric.entries.pop(); + if (!entry) { + return; + } - this._measurements[name].value = normalizedValue; - }); + const timeOrigin = msToSec(browserPerformanceTimeOrigin as number); + const startTime = msToSec(entry.startTime); + IS_DEBUG_BUILD && logger.log('[Measurements] Adding FID'); + _measurements['fid'] = { value: metric.value, unit: 'millisecond' }; + _measurements['mark.fid'] = { value: timeOrigin + startTime, unit: 'second' }; + }); +} - if (this._measurements['mark.fid'] && this._measurements['fid']) { - // create span for FID +/** Add performance related spans to a transaction */ +export function addPerformanceEntries(transaction: Transaction): void { + const performance = getBrowserPerformanceAPI(); + if (!performance || !global.performance.getEntries || !browserPerformanceTimeOrigin) { + // Gatekeeper if performance API not available + return; + } - _startChild(transaction, { - description: 'first input delay', - endTimestamp: this._measurements['mark.fid'].value + msToSec(this._measurements['fid'].value), - op: 'web.vitals', - startTimestamp: this._measurements['mark.fid'].value, - }); - } + IS_DEBUG_BUILD && logger.log('[Tracing] Adding & adjusting spans using Performance API'); + const timeOrigin = msToSec(browserPerformanceTimeOrigin); - // If FCP is not recorded we should not record the cls value - // according to the new definition of CLS. - if (!('fcp' in this._measurements)) { - delete this._measurements.cls; - } + const performanceEntries = performance.getEntries(); - Object.keys(this._measurements).forEach(measurementName => { - transaction.setMeasurement( - measurementName, - this._measurements[measurementName].value, - this._measurements[measurementName].unit, - ); - }); + let responseStartTimestamp: number | undefined; + let requestStartTimestamp: number | undefined; - tagMetricInfo(transaction, this._lcpEntry, this._clsEntry); - transaction.setTag('sentry_reportAllChanges', this._reportAllChanges); - } - } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + performanceEntries.slice(_performanceCursor).forEach((entry: Record) => { + const startTime = msToSec(entry.startTime); + const duration = msToSec(entry.duration); - /** - * Capture the information of the user agent. - */ - private _trackNavigator(transaction: Transaction): void { - const navigator = global.navigator as null | (Navigator & NavigatorNetworkInformation & NavigatorDeviceMemory); - if (!navigator) { + if (transaction.op === 'navigation' && timeOrigin + startTime < transaction.startTimestamp) { return; } - // track network connectivity - const connection = navigator.connection; - if (connection) { - if (connection.effectiveType) { - transaction.setTag('effectiveConnectionType', connection.effectiveType); - } - - if (connection.type) { - transaction.setTag('connectionType', connection.type); + switch (entry.entryType) { + case 'navigation': { + _addNavigationSpans(transaction, entry, timeOrigin); + responseStartTimestamp = timeOrigin + msToSec(entry.responseStart); + requestStartTimestamp = timeOrigin + msToSec(entry.requestStart); + break; } - - if (isMeasurementValue(connection.rtt)) { - this._measurements['connection.rtt'] = { value: connection.rtt, unit: 'millisecond' }; + case 'mark': + case 'paint': + case 'measure': { + const startTimestamp = addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); + + // capture web vitals + const firstHidden = getVisibilityWatcher(); + // Only report if the page wasn't hidden prior to the web vital. + const shouldRecord = entry.startTime < firstHidden.firstHiddenTime; + + if (entry.name === 'first-paint' && shouldRecord) { + IS_DEBUG_BUILD && logger.log('[Measurements] Adding FP'); + _measurements['fp'] = { value: entry.startTime, unit: 'millisecond' }; + _measurements['mark.fp'] = { value: startTimestamp, unit: 'second' }; + } + if (entry.name === 'first-contentful-paint' && shouldRecord) { + IS_DEBUG_BUILD && logger.log('[Measurements] Adding FCP'); + _measurements['fcp'] = { value: entry.startTime, unit: 'millisecond' }; + _measurements['mark.fcp'] = { value: startTimestamp, unit: 'second' }; + } + break; } - - if (isMeasurementValue(connection.downlink)) { - this._measurements['connection.downlink'] = { value: connection.downlink, unit: '' }; // unit is empty string for now, while relay doesn't support download speed units + case 'resource': { + const resourceName = (entry.name as string).replace(global.location.origin, ''); + _addResourceSpans(transaction, entry, resourceName, startTime, duration, timeOrigin); + break; } + default: + // Ignore other entry types. } + }); - if (isMeasurementValue(navigator.deviceMemory)) { - transaction.setTag('deviceMemory', `${navigator.deviceMemory} GB`); - } - - if (isMeasurementValue(navigator.hardwareConcurrency)) { - transaction.setTag('hardwareConcurrency', String(navigator.hardwareConcurrency)); + _performanceCursor = Math.max(performanceEntries.length - 1, 0); + + _trackNavigator(transaction); + + // Measurements are only available for pageload transactions + if (transaction.op === 'pageload') { + // Generate TTFB (Time to First Byte), which measured as the time between the beginning of the transaction and the + // start of the response in milliseconds + if (typeof responseStartTimestamp === 'number') { + IS_DEBUG_BUILD && logger.log('[Measurements] Adding TTFB'); + _measurements['ttfb'] = { + value: (responseStartTimestamp - transaction.startTimestamp) * 1000, + unit: 'millisecond', + }; + + if (typeof requestStartTimestamp === 'number' && requestStartTimestamp <= responseStartTimestamp) { + // Capture the time spent making the request and receiving the first byte of the response. + // This is the time between the start of the request and the start of the response in milliseconds. + _measurements['ttfb.requestTime'] = { + value: (responseStartTimestamp - requestStartTimestamp) * 1000, + unit: 'second', + }; + } } - } - /** Starts tracking the Cumulative Layout Shift on the current page. */ - private _trackCLS(): void { - // See: - // https://web.dev/evolving-cls/ - // https://web.dev/cls-web-tooling/ - getCLS(metric => { - const entry = metric.entries.pop(); - if (!entry) { + ['fcp', 'fp', 'lcp'].forEach(name => { + if (!_measurements[name] || timeOrigin >= transaction.startTimestamp) { return; } - - IS_DEBUG_BUILD && logger.log('[Measurements] Adding CLS'); - this._measurements['cls'] = { value: metric.value, unit: 'millisecond' }; - this._clsEntry = entry as LayoutShift; + // 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. + const oldValue = _measurements[name].value; + const measurementTimestamp = timeOrigin + msToSec(oldValue); + + // normalizedValue should be in milliseconds + const normalizedValue = Math.abs((measurementTimestamp - transaction.startTimestamp) * 1000); + const delta = normalizedValue - oldValue; + + IS_DEBUG_BUILD && + logger.log(`[Measurements] Normalized ${name} from ${oldValue} to ${normalizedValue} (${delta})`); + _measurements[name].value = normalizedValue; }); - } - - /** Starts tracking the Largest Contentful Paint on the current page. */ - private _trackLCP(): void { - getLCP(metric => { - const entry = metric.entries.pop(); - if (!entry) { - return; - } - const timeOrigin = msToSec(browserPerformanceTimeOrigin as number); - const startTime = msToSec(entry.startTime); - IS_DEBUG_BUILD && logger.log('[Measurements] Adding LCP'); - this._measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; - this._measurements['mark.lcp'] = { value: timeOrigin + startTime, unit: 'second' }; - this._lcpEntry = entry as LargestContentfulPaint; - }, this._reportAllChanges); - } + if (_measurements['mark.fid'] && _measurements['fid']) { + // create span for FID + _startChild(transaction, { + description: 'first input delay', + endTimestamp: _measurements['mark.fid'].value + msToSec(_measurements['fid'].value), + op: 'web.vitals', + startTimestamp: _measurements['mark.fid'].value, + }); + } - /** Starts tracking the First Input Delay on the current page. */ - private _trackFID(): void { - getFID(metric => { - const entry = metric.entries.pop(); - if (!entry) { - return; - } + // If FCP is not recorded we should not record the cls value + // according to the new definition of CLS. + if (!('fcp' in _measurements)) { + delete _measurements.cls; + } - const timeOrigin = msToSec(browserPerformanceTimeOrigin as number); - const startTime = msToSec(entry.startTime); - IS_DEBUG_BUILD && logger.log('[Measurements] Adding FID'); - this._measurements['fid'] = { value: metric.value, unit: 'millisecond' }; - this._measurements['mark.fid'] = { value: timeOrigin + startTime, unit: 'second' }; + Object.keys(_measurements).forEach(measurementName => { + transaction.setMeasurement( + measurementName, + _measurements[measurementName].value, + _measurements[measurementName].unit, + ); }); + + _tagMetricInfo(transaction); } -} -/** Instrument navigation entries */ -function addNavigationSpans(transaction: Transaction, entry: Record, timeOrigin: number): void { - ['unloadEvent', 'redirect', 'domContentLoadedEvent', 'loadEvent', 'connect'].forEach(event => { - addPerformanceNavigationTiming(transaction, 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); + _lcpEntry = undefined; + _clsEntry = undefined; + _measurements = {}; } /** Create measure related spans */ function addMeasureSpans( transaction: Transaction, + // eslint-disable-next-line @typescript-eslint/no-explicit-any entry: Record, startTime: number, duration: number, @@ -302,6 +250,59 @@ function addMeasureSpans( return measureStartTimestamp; } +/** Instrument navigation entries */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function _addNavigationSpans(transaction: Transaction, entry: Record, timeOrigin: number): void { + ['unloadEvent', 'redirect', 'domContentLoadedEvent', 'loadEvent', 'connect'].forEach(event => { + _addPerformanceNavigationTiming(transaction, 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); +} + +/** Create performance navigation related spans */ +function _addPerformanceNavigationTiming( + transaction: Transaction, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + entry: Record, + event: string, + timeOrigin: number, + description?: string, + eventEnd?: string, +): void { + const end = eventEnd ? (entry[eventEnd] as number | undefined) : (entry[`${event}End`] as number | undefined); + const start = entry[`${event}Start`] as number | undefined; + if (!start || !end) { + return; + } + _startChild(transaction, { + op: 'browser', + description: description ?? 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 { + _startChild(transaction, { + op: 'browser', + description: 'request', + startTimestamp: timeOrigin + msToSec(entry.requestStart as number), + endTimestamp: timeOrigin + msToSec(entry.responseEnd as number), + }); + + _startChild(transaction, { + op: 'browser', + description: 'response', + startTimestamp: timeOrigin + msToSec(entry.responseStart as number), + endTimestamp: timeOrigin + msToSec(entry.responseEnd as number), + }); +} + export interface ResourceEntry extends Record { initiatorType?: string; transferSize?: number; @@ -310,7 +311,7 @@ export interface ResourceEntry extends Record { } /** Create resource-related spans */ -export function addResourceSpans( +export function _addResourceSpans( transaction: Transaction, entry: ResourceEntry, resourceName: string, @@ -324,6 +325,7 @@ export function addResourceSpans( return; } + // eslint-disable-next-line @typescript-eslint/no-explicit-any const data: Record = {}; if ('transferSize' in entry) { data['Transfer Size'] = entry.transferSize; @@ -347,43 +349,49 @@ export function addResourceSpans( }); } -/** Create performance navigation related spans */ -function addPerformanceNavigationTiming( - transaction: Transaction, - entry: Record, - event: string, - timeOrigin: number, - description?: string, - eventEnd?: string, -): void { - const end = eventEnd ? (entry[eventEnd] as number | undefined) : (entry[`${event}End`] as number | undefined); - const start = entry[`${event}Start`] as number | undefined; - if (!start || !end) { +/** + * Capture the information of the user agent. + */ +function _trackNavigator(transaction: Transaction): void { + const navigator = global.navigator as null | (Navigator & NavigatorNetworkInformation & NavigatorDeviceMemory); + if (!navigator) { return; } - _startChild(transaction, { - op: 'browser', - description: description ?? event, - startTimestamp: timeOrigin + msToSec(start), - endTimestamp: timeOrigin + msToSec(end), - }); -} -/** Create request and response related spans */ -function addRequest(transaction: Transaction, entry: Record, timeOrigin: number): void { - _startChild(transaction, { - op: 'browser', - description: 'request', - startTimestamp: timeOrigin + msToSec(entry.requestStart as number), - endTimestamp: timeOrigin + msToSec(entry.responseEnd as number), - }); + // track network connectivity + const connection = navigator.connection; + if (connection) { + if (connection.effectiveType) { + transaction.setTag('effectiveConnectionType', connection.effectiveType); + } - _startChild(transaction, { - op: 'browser', - description: 'response', - startTimestamp: timeOrigin + msToSec(entry.responseStart as number), - endTimestamp: timeOrigin + msToSec(entry.responseEnd as number), - }); + if (connection.type) { + transaction.setTag('connectionType', connection.type); + } + + if (isMeasurementValue(connection.rtt)) { + _measurements['connection.rtt'] = { value: connection.rtt, unit: 'millisecond' }; + } + + if (isMeasurementValue(connection.downlink)) { + _measurements['connection.downlink'] = { value: connection.downlink, unit: '' }; // unit is empty string for now, while relay doesn't support download speed units + } + } + + if (isMeasurementValue(navigator.deviceMemory)) { + transaction.setTag('deviceMemory', `${navigator.deviceMemory} GB`); + } + + if (isMeasurementValue(navigator.hardwareConcurrency)) { + transaction.setTag('hardwareConcurrency', String(navigator.hardwareConcurrency)); + } +} + +/** + * Checks if a given value is a valid measurement value. + */ +function isMeasurementValue(value: unknown): value is number { + return typeof value === 'number' && isFinite(value); } /** @@ -402,44 +410,33 @@ export function _startChild(transaction: Transaction, { startTimestamp, ...ctx } }); } -/** - * Checks if a given value is a valid measurement value. - */ -function isMeasurementValue(value: unknown): value is number { - return typeof value === 'number' && isFinite(value); -} - /** Add LCP / CLS data to transaction to allow debugging */ -function tagMetricInfo( - transaction: Transaction, - lcpEntry: MetricsInstrumentation['_lcpEntry'], - clsEntry: MetricsInstrumentation['_clsEntry'], -): void { - if (lcpEntry) { +function _tagMetricInfo(transaction: Transaction): void { + if (_lcpEntry) { IS_DEBUG_BUILD && logger.log('[Measurements] Adding LCP Data'); // Capture Properties of the LCP element that contributes to the LCP. - if (lcpEntry.element) { - transaction.setTag('lcp.element', htmlTreeAsString(lcpEntry.element)); + if (_lcpEntry.element) { + transaction.setTag('lcp.element', htmlTreeAsString(_lcpEntry.element)); } - if (lcpEntry.id) { - transaction.setTag('lcp.id', lcpEntry.id); + if (_lcpEntry.id) { + transaction.setTag('lcp.id', _lcpEntry.id); } - if (lcpEntry.url) { + if (_lcpEntry.url) { // Trim URL to the first 200 characters. - transaction.setTag('lcp.url', lcpEntry.url.trim().slice(0, 200)); + transaction.setTag('lcp.url', _lcpEntry.url.trim().slice(0, 200)); } - transaction.setTag('lcp.size', lcpEntry.size); + transaction.setTag('lcp.size', _lcpEntry.size); } // See: https://developer.mozilla.org/en-US/docs/Web/API/LayoutShift - if (clsEntry && clsEntry.sources) { + if (_clsEntry && _clsEntry.sources) { IS_DEBUG_BUILD && logger.log('[Measurements] Adding CLS Data'); - clsEntry.sources.forEach((source, index) => + _clsEntry.sources.forEach((source, index) => transaction.setTag(`cls.source.${index + 1}`, htmlTreeAsString(source.node)), ); } diff --git a/packages/tracing/test/browser/metrics.test.ts b/packages/tracing/test/browser/metrics.test.ts index eb79ff30df93..4ed2018e8b04 100644 --- a/packages/tracing/test/browser/metrics.test.ts +++ b/packages/tracing/test/browser/metrics.test.ts @@ -1,5 +1,5 @@ import { Span, Transaction } from '../../src'; -import { _startChild, addResourceSpans, MetricsInstrumentation, ResourceEntry } from '../../src/browser/metrics'; +import { _startChild, _addResourceSpans, MetricsInstrumentation, ResourceEntry } from '../../src/browser/metrics'; import { addDOMPropertiesToGlobal } from '../testutils'; // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-var @@ -43,7 +43,7 @@ describe('_startChild()', () => { }); }); -describe('addResourceSpans', () => { +describe('_addResourceSpans', () => { const transaction = new Transaction({ name: 'hello' }); beforeEach(() => { transaction.startChild = jest.fn(); @@ -57,7 +57,7 @@ describe('addResourceSpans', () => { encodedBodySize: 256, decodedBodySize: 256, }; - addResourceSpans(transaction, entry, '/assets/to/me', 123, 456, 100); + _addResourceSpans(transaction, entry, '/assets/to/me', 123, 456, 100); // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenCalledTimes(0); @@ -70,7 +70,7 @@ describe('addResourceSpans', () => { encodedBodySize: 256, decodedBodySize: 256, }; - addResourceSpans(transaction, entry, '/assets/to/me', 123, 456, 100); + _addResourceSpans(transaction, entry, '/assets/to/me', 123, 456, 100); // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenCalledTimes(0); @@ -88,7 +88,7 @@ describe('addResourceSpans', () => { const startTime = 23; const duration = 356; - addResourceSpans(transaction, entry, '/assets/to/css', startTime, duration, timeOrigin); + _addResourceSpans(transaction, entry, '/assets/to/css', startTime, duration, timeOrigin); // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenCalledTimes(1); @@ -134,7 +134,7 @@ describe('addResourceSpans', () => { const entry: ResourceEntry = { initiatorType, }; - addResourceSpans(transaction, entry, '/assets/to/me', 123, 234, 465); + _addResourceSpans(transaction, entry, '/assets/to/me', 123, 234, 465); // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenLastCalledWith( @@ -153,7 +153,7 @@ describe('addResourceSpans', () => { decodedBodySize: 0, }; - addResourceSpans(transaction, entry, '/assets/to/css', 100, 23, 345); + _addResourceSpans(transaction, entry, '/assets/to/css', 100, 23, 345); // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenCalledTimes(1); @@ -170,52 +170,53 @@ describe('addResourceSpans', () => { }); }); -describe('MetricsInstrumentation', () => { - afterEach(() => { - jest.clearAllMocks(); - }); +// TODO: Add these tests back +// describe('MetricsInstrumentation', () => { +// afterEach(() => { +// jest.clearAllMocks(); +// }); - it('does not initialize trackers when on node', () => { - const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => - jest.spyOn(MetricsInstrumentation.prototype as any, tracker), - ); +// it('does not initialize trackers when on node', () => { +// const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => +// jest.spyOn(MetricsInstrumentation.prototype as any, tracker), +// ); - new MetricsInstrumentation(); +// new MetricsInstrumentation(); - trackers.forEach(tracker => expect(tracker).not.toBeCalled()); - }); +// trackers.forEach(tracker => expect(tracker).not.toBeCalled()); +// }); - it('initializes trackers when not on node and `global.performance` and `global.document` are available.', () => { - addDOMPropertiesToGlobal(['performance', 'document', 'addEventListener', 'window']); +// it('initializes trackers when not on node and `global.performance` and `global.document` are available.', () => { +// addDOMPropertiesToGlobal(['performance', 'document', 'addEventListener', 'window']); - const backup = global.process; - global.process = undefined; +// const backup = global.process; +// global.process = undefined; - const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => - jest.spyOn(MetricsInstrumentation.prototype as any, tracker), - ); - new MetricsInstrumentation(); - global.process = backup; +// const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => +// jest.spyOn(MetricsInstrumentation.prototype as any, tracker), +// ); +// new MetricsInstrumentation(); +// global.process = backup; - trackers.forEach(tracker => expect(tracker).toBeCalled()); - }); +// trackers.forEach(tracker => expect(tracker).toBeCalled()); +// }); - it('does not initialize trackers when not on node but `global.document` is not available (in worker)', () => { - // window not necessary for this test, but it is here to exercise that it is absence of document that is checked - addDOMPropertiesToGlobal(['performance', 'addEventListener', 'window']); +// it('does not initialize trackers when not on node but `global.document` is not available (in worker)', () => { +// // window not necessary for this test, but it is here to exercise that it is absence of document that is checked +// addDOMPropertiesToGlobal(['performance', 'addEventListener', 'window']); - const processBackup = global.process; - global.process = undefined; - const documentBackup = global.document; - global.document = undefined; +// const processBackup = global.process; +// global.process = undefined; +// const documentBackup = global.document; +// global.document = undefined; - const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => - jest.spyOn(MetricsInstrumentation.prototype as any, tracker), - ); - new MetricsInstrumentation(); - global.process = processBackup; - global.document = documentBackup; +// const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => +// jest.spyOn(MetricsInstrumentation.prototype as any, tracker), +// ); +// new MetricsInstrumentation(); +// global.process = processBackup; +// global.document = documentBackup; - trackers.forEach(tracker => expect(tracker).not.toBeCalled()); - }); -}); +// trackers.forEach(tracker => expect(tracker).not.toBeCalled()); +// }); +// }); From 8b6ecb044f87ffae47fce2e8100e36dcc097fd8e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 19 May 2022 15:02:48 -0400 Subject: [PATCH 2/3] reorg structure --- .../browser/{metrics.ts => metrics/index.ts} | 46 ++----- packages/tracing/src/browser/metrics/utils.ts | 26 ++++ .../test/browser/browsertracing.test.ts | 26 ---- .../index.test.ts} | 113 ++++-------------- .../test/browser/metrics/utils.test.ts | 40 +++++++ 5 files changed, 103 insertions(+), 148 deletions(-) rename packages/tracing/src/browser/{metrics.ts => metrics/index.ts} (91%) create mode 100644 packages/tracing/src/browser/metrics/utils.ts rename packages/tracing/test/browser/{metrics.test.ts => metrics/index.test.ts} (50%) create mode 100644 packages/tracing/test/browser/metrics/utils.test.ts diff --git a/packages/tracing/src/browser/metrics.ts b/packages/tracing/src/browser/metrics/index.ts similarity index 91% rename from packages/tracing/src/browser/metrics.ts rename to packages/tracing/src/browser/metrics/index.ts index 20a464edbdc6..48438ab7b719 100644 --- a/packages/tracing/src/browser/metrics.ts +++ b/packages/tracing/src/browser/metrics/index.ts @@ -1,15 +1,16 @@ /* eslint-disable max-lines */ -import { Measurements, Span, SpanContext } from '@sentry/types'; +import { Measurements } from '@sentry/types'; import { browserPerformanceTimeOrigin, getGlobalObject, htmlTreeAsString, isNodeEnv, logger } from '@sentry/utils'; -import { IS_DEBUG_BUILD } from '../flags'; -import { Transaction } from '../transaction'; -import { msToSec } from '../utils'; -import { getCLS, LayoutShift } from './web-vitals/getCLS'; -import { getFID } from './web-vitals/getFID'; -import { getLCP, LargestContentfulPaint } from './web-vitals/getLCP'; -import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher'; -import { NavigatorDeviceMemory, NavigatorNetworkInformation } from './web-vitals/types'; +import { IS_DEBUG_BUILD } from '../../flags'; +import { Transaction } from '../../transaction'; +import { msToSec } from '../../utils'; +import { getCLS, LayoutShift } from '../web-vitals/getCLS'; +import { getFID } from '../web-vitals/getFID'; +import { getLCP, LargestContentfulPaint } from '../web-vitals/getLCP'; +import { getVisibilityWatcher } from '../web-vitals/lib/getVisibilityWatcher'; +import { NavigatorDeviceMemory, NavigatorNetworkInformation } from '../web-vitals/types'; +import { _startChild, isMeasurementValue } from './utils'; const global = getGlobalObject(); @@ -123,7 +124,7 @@ export function addPerformanceEntries(transaction: Transaction): void { case 'mark': case 'paint': case 'measure': { - const startTimestamp = addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); + const startTimestamp = _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); // capture web vitals const firstHidden = getVisibilityWatcher(); @@ -229,7 +230,7 @@ export function addPerformanceEntries(transaction: Transaction): void { } /** Create measure related spans */ -function addMeasureSpans( +export function _addMeasureSpans( transaction: Transaction, // eslint-disable-next-line @typescript-eslint/no-explicit-any entry: Record, @@ -387,29 +388,6 @@ function _trackNavigator(transaction: Transaction): void { } } -/** - * Checks if a given value is a valid measurement value. - */ -function isMeasurementValue(value: unknown): value is number { - return typeof value === 'number' && isFinite(value); -} - -/** - * Helper function to start child on transactions. This function will make sure that the transaction will - * use the start timestamp of the created child span if it is earlier than the transactions actual - * start timestamp. - */ -export function _startChild(transaction: Transaction, { startTimestamp, ...ctx }: SpanContext): Span { - if (startTimestamp && transaction.startTimestamp > startTimestamp) { - transaction.startTimestamp = startTimestamp; - } - - return transaction.startChild({ - startTimestamp, - ...ctx, - }); -} - /** Add LCP / CLS data to transaction to allow debugging */ function _tagMetricInfo(transaction: Transaction): void { if (_lcpEntry) { diff --git a/packages/tracing/src/browser/metrics/utils.ts b/packages/tracing/src/browser/metrics/utils.ts new file mode 100644 index 000000000000..4894dbd3ec8e --- /dev/null +++ b/packages/tracing/src/browser/metrics/utils.ts @@ -0,0 +1,26 @@ +import { Span, SpanContext } from '@sentry/types'; + +import { Transaction } from '../../transaction'; + +/** + * Checks if a given value is a valid measurement value. + */ +export function isMeasurementValue(value: unknown): value is number { + return typeof value === 'number' && isFinite(value); +} + +/** + * Helper function to start child on transactions. This function will make sure that the transaction will + * use the start timestamp of the created child span if it is earlier than the transactions actual + * start timestamp. + */ +export function _startChild(transaction: Transaction, { startTimestamp, ...ctx }: SpanContext): Span { + if (startTimestamp && transaction.startTimestamp > startTimestamp) { + transaction.startTimestamp = startTimestamp; + } + + return transaction.startChild({ + startTimestamp, + ...ctx, + }); +} diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 4bfc093fbe2b..be18cc389650 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -9,7 +9,6 @@ import { getHeaderContext, getMetaContent, } from '../../src/browser/browsertracing'; -import { MetricsInstrumentation } from '../../src/browser/metrics'; import { defaultRequestInstrumentationOptions } from '../../src/browser/request'; import { instrumentRoutingWithDefaults } from '../../src/browser/router'; import * as hubExtensions from '../../src/hubextensions'; @@ -466,29 +465,4 @@ describe('BrowserTracing', () => { ); }); }); - - describe('metrics', () => { - beforeEach(() => { - // @ts-ignore mock clear - MetricsInstrumentation.mockClear(); - }); - - it('creates metrics instrumentation', () => { - createBrowserTracing(true, {}); - - expect(MetricsInstrumentation).toHaveBeenCalledTimes(1); - expect(MetricsInstrumentation).toHaveBeenLastCalledWith(undefined); - }); - - it('creates metrics instrumentation with custom options', () => { - createBrowserTracing(true, { - _metricOptions: { - _reportAllChanges: true, - }, - }); - - expect(MetricsInstrumentation).toHaveBeenCalledTimes(1); - expect(MetricsInstrumentation).toHaveBeenLastCalledWith(true); - }); - }); }); diff --git a/packages/tracing/test/browser/metrics.test.ts b/packages/tracing/test/browser/metrics/index.test.ts similarity index 50% rename from packages/tracing/test/browser/metrics.test.ts rename to packages/tracing/test/browser/metrics/index.test.ts index 4ed2018e8b04..7296f9f256a5 100644 --- a/packages/tracing/test/browser/metrics.test.ts +++ b/packages/tracing/test/browser/metrics/index.test.ts @@ -1,50 +1,38 @@ -import { Span, Transaction } from '../../src'; -import { _startChild, _addResourceSpans, MetricsInstrumentation, ResourceEntry } from '../../src/browser/metrics'; -import { addDOMPropertiesToGlobal } from '../testutils'; - -// eslint-disable-next-line @typescript-eslint/no-explicit-any, no-var -declare var global: any; - -describe('_startChild()', () => { - it('creates a span with given properties', () => { - const transaction = new Transaction({ name: 'test' }); - const span = _startChild(transaction, { - description: 'evaluation', - op: 'script', - }); +import { Transaction } from '../../../src'; +import { _addResourceSpans, _addMeasureSpans, ResourceEntry } from '../../../src/browser/metrics'; - expect(span).toBeInstanceOf(Span); - expect(span.description).toBe('evaluation'); - expect(span.op).toBe('script'); +describe('_addMeasureSpans', () => { + const transaction = new Transaction({ op: 'pageload', name: '/' }); + beforeEach(() => { + transaction.startChild = jest.fn(); }); - it('adjusts the start timestamp if child span starts before transaction', () => { - const transaction = new Transaction({ name: 'test', startTimestamp: 123 }); - const span = _startChild(transaction, { - description: 'script.js', - op: 'resource', - startTimestamp: 100, - }); + it('adds measure spans to a transaction', () => { + const entry: Omit = { + entryType: 'measure', + name: 'measure-1', + duration: 10, + startTime: 12, + }; - expect(transaction.startTimestamp).toEqual(span.startTimestamp); - expect(transaction.startTimestamp).toEqual(100); - }); + const timeOrigin = 100; + const startTime = 23; + const duration = 356; - it('does not adjust start timestamp if child span starts after transaction', () => { - const transaction = new Transaction({ name: 'test', startTimestamp: 123 }); - const span = _startChild(transaction, { - description: 'script.js', - op: 'resource', - startTimestamp: 150, + expect(transaction.startChild).toHaveBeenCalledTimes(0); + _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); + expect(transaction.startChild).toHaveBeenCalledTimes(1); + expect(transaction.startChild).toHaveBeenLastCalledWith({ + description: 'measure-1', + startTimestamp: timeOrigin + startTime, + endTimestamp: timeOrigin + startTime + duration, + op: 'measure', }); - - expect(transaction.startTimestamp).not.toEqual(span.startTimestamp); - expect(transaction.startTimestamp).toEqual(123); }); }); describe('_addResourceSpans', () => { - const transaction = new Transaction({ name: 'hello' }); + const transaction = new Transaction({ op: 'pageload', name: '/' }); beforeEach(() => { transaction.startChild = jest.fn(); }); @@ -169,54 +157,3 @@ describe('_addResourceSpans', () => { ); }); }); - -// TODO: Add these tests back -// describe('MetricsInstrumentation', () => { -// afterEach(() => { -// jest.clearAllMocks(); -// }); - -// it('does not initialize trackers when on node', () => { -// const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => -// jest.spyOn(MetricsInstrumentation.prototype as any, tracker), -// ); - -// new MetricsInstrumentation(); - -// trackers.forEach(tracker => expect(tracker).not.toBeCalled()); -// }); - -// it('initializes trackers when not on node and `global.performance` and `global.document` are available.', () => { -// addDOMPropertiesToGlobal(['performance', 'document', 'addEventListener', 'window']); - -// const backup = global.process; -// global.process = undefined; - -// const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => -// jest.spyOn(MetricsInstrumentation.prototype as any, tracker), -// ); -// new MetricsInstrumentation(); -// global.process = backup; - -// trackers.forEach(tracker => expect(tracker).toBeCalled()); -// }); - -// it('does not initialize trackers when not on node but `global.document` is not available (in worker)', () => { -// // window not necessary for this test, but it is here to exercise that it is absence of document that is checked -// addDOMPropertiesToGlobal(['performance', 'addEventListener', 'window']); - -// const processBackup = global.process; -// global.process = undefined; -// const documentBackup = global.document; -// global.document = undefined; - -// const trackers = ['_trackCLS', '_trackLCP', '_trackFID'].map(tracker => -// jest.spyOn(MetricsInstrumentation.prototype as any, tracker), -// ); -// new MetricsInstrumentation(); -// global.process = processBackup; -// global.document = documentBackup; - -// trackers.forEach(tracker => expect(tracker).not.toBeCalled()); -// }); -// }); diff --git a/packages/tracing/test/browser/metrics/utils.test.ts b/packages/tracing/test/browser/metrics/utils.test.ts new file mode 100644 index 000000000000..25f03c11af0b --- /dev/null +++ b/packages/tracing/test/browser/metrics/utils.test.ts @@ -0,0 +1,40 @@ +import { Span, Transaction } from '../../../src'; +import { _startChild } from '../../../src/browser/metrics/utils'; + +describe('_startChild()', () => { + it('creates a span with given properties', () => { + const transaction = new Transaction({ name: 'test' }); + const span = _startChild(transaction, { + description: 'evaluation', + op: 'script', + }); + + expect(span).toBeInstanceOf(Span); + expect(span.description).toBe('evaluation'); + expect(span.op).toBe('script'); + }); + + it('adjusts the start timestamp if child span starts before transaction', () => { + const transaction = new Transaction({ name: 'test', startTimestamp: 123 }); + const span = _startChild(transaction, { + description: 'script.js', + op: 'resource', + startTimestamp: 100, + }); + + expect(transaction.startTimestamp).toEqual(span.startTimestamp); + expect(transaction.startTimestamp).toEqual(100); + }); + + it('does not adjust start timestamp if child span starts after transaction', () => { + const transaction = new Transaction({ name: 'test', startTimestamp: 123 }); + const span = _startChild(transaction, { + description: 'script.js', + op: 'resource', + startTimestamp: 150, + }); + + expect(transaction.startTimestamp).not.toEqual(span.startTimestamp); + expect(transaction.startTimestamp).toEqual(123); + }); +}); From a1a53239fffb5dcee2fc0817ec5533b22aa23d6d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 23 May 2022 08:19:28 +0000 Subject: [PATCH 3/3] Ignore some eslint errors --- packages/tracing/test/browser/metrics/index.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/tracing/test/browser/metrics/index.test.ts b/packages/tracing/test/browser/metrics/index.test.ts index 7296f9f256a5..b47ebd92a8f3 100644 --- a/packages/tracing/test/browser/metrics/index.test.ts +++ b/packages/tracing/test/browser/metrics/index.test.ts @@ -1,5 +1,5 @@ import { Transaction } from '../../../src'; -import { _addResourceSpans, _addMeasureSpans, ResourceEntry } from '../../../src/browser/metrics'; +import { _addMeasureSpans, _addResourceSpans, ResourceEntry } from '../../../src/browser/metrics'; describe('_addMeasureSpans', () => { const transaction = new Transaction({ op: 'pageload', name: '/' }); @@ -19,9 +19,12 @@ describe('_addMeasureSpans', () => { const startTime = 23; const duration = 356; + // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenCalledTimes(0); _addMeasureSpans(transaction, entry, startTime, duration, timeOrigin); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenCalledTimes(1); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(transaction.startChild).toHaveBeenLastCalledWith({ description: 'measure-1', startTimestamp: timeOrigin + startTime,