From 7e87329f97f9e883dcf67e3852d486cc59a2155b Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 7 Dec 2022 13:35:05 +0000 Subject: [PATCH 1/2] Remove reportAllChanges --- .../tracing/src/browser/browsertracing.ts | 9 ++----- packages/tracing/src/browser/metrics/index.ts | 27 +++++++++---------- .../tracing/src/browser/web-vitals/getCLS.ts | 6 ++--- .../tracing/src/browser/web-vitals/getFID.ts | 6 ++--- .../tracing/src/browser/web-vitals/getLCP.ts | 11 +++----- 5 files changed, 23 insertions(+), 36 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 24cf9bc0459e..937ad1941378 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -74,11 +74,7 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { markBackgroundTransactions: boolean; /** - * _metricOptions allows the user to send options to change how metrics are collected. - * - * _metricOptions is currently experimental. - * - * Default: undefined + * @deprecated This property no longer has any effect and will be removed in v8. */ _metricOptions?: Partial<{ _reportAllChanges: boolean }>; @@ -162,8 +158,7 @@ export class BrowserTracing implements Integration { this.options.tracePropagationTargets = _options.tracingOrigins; } - const { _metricOptions } = this.options; - startTrackingWebVitals(_metricOptions && _metricOptions._reportAllChanges); + startTrackingWebVitals(); if (this.options._experiments?.enableLongTask) { startTrackingLongTasks(); } diff --git a/packages/tracing/src/browser/metrics/index.ts b/packages/tracing/src/browser/metrics/index.ts index 102406c79a95..481a6ea59d83 100644 --- a/packages/tracing/src/browser/metrics/index.ts +++ b/packages/tracing/src/browser/metrics/index.ts @@ -27,14 +27,14 @@ let _clsEntry: LayoutShift | undefined; /** * Start tracking web vitals */ -export function startTrackingWebVitals(reportAllChanges: boolean = false): void { +export function startTrackingWebVitals(): void { const performance = getBrowserPerformanceAPI(); if (performance && browserPerformanceTimeOrigin) { if (performance.mark) { WINDOW.performance.mark('sentry-tracing-init'); } _trackCLS(); - _trackLCP(reportAllChanges); + _trackLCP(); _trackFID(); } } @@ -82,20 +82,17 @@ function _trackCLS(): void { } /** Starts tracking the Largest Contentful Paint on the current page. */ -function _trackLCP(reportAllChanges: boolean): void { - onLCP( - metric => { - const entry = metric.entries.pop(); - if (!entry) { - return; - } +function _trackLCP(): void { + onLCP(metric => { + const entry = metric.entries.pop(); + if (!entry) { + return; + } - __DEBUG_BUILD__ && logger.log('[Measurements] Adding LCP'); - _measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; - _lcpEntry = entry as LargestContentfulPaint; - }, - { reportAllChanges }, - ); + __DEBUG_BUILD__ && logger.log('[Measurements] Adding LCP'); + _measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; + _lcpEntry = entry as LargestContentfulPaint; + }); } /** Starts tracking the First Input Delay on the current page. */ diff --git a/packages/tracing/src/browser/web-vitals/getCLS.ts b/packages/tracing/src/browser/web-vitals/getCLS.ts index 52fe7e519bd0..0980eb5cf06d 100644 --- a/packages/tracing/src/browser/web-vitals/getCLS.ts +++ b/packages/tracing/src/browser/web-vitals/getCLS.ts @@ -18,7 +18,7 @@ import { bindReporter } from './lib/bindReporter'; import { initMetric } from './lib/initMetric'; import { observe } from './lib/observe'; import { onHidden } from './lib/onHidden'; -import { CLSMetric, ReportCallback, ReportOpts } from './types'; +import { CLSMetric, ReportCallback } from './types'; /** * Calculates the [CLS](https://web.dev/cls/) value for the current page and @@ -41,7 +41,7 @@ import { CLSMetric, ReportCallback, ReportOpts } from './types'; * hidden. As a result, the `callback` function might be called multiple times * during the same page load._ */ -export const onCLS = (onReport: ReportCallback, opts: ReportOpts = {}): void => { +export const onCLS = (onReport: ReportCallback): void => { const metric = initMetric('CLS', 0); let report: ReturnType; @@ -87,7 +87,7 @@ export const onCLS = (onReport: ReportCallback, opts: ReportOpts = {}): void => const po = observe('layout-shift', handleEntries); if (po) { - report = bindReporter(onReport, metric, opts.reportAllChanges); + report = bindReporter(onReport, metric); onHidden(() => { handleEntries(po.takeRecords() as CLSMetric['entries']); diff --git a/packages/tracing/src/browser/web-vitals/getFID.ts b/packages/tracing/src/browser/web-vitals/getFID.ts index 982bbd4a3a01..fcf5d529669c 100644 --- a/packages/tracing/src/browser/web-vitals/getFID.ts +++ b/packages/tracing/src/browser/web-vitals/getFID.ts @@ -19,7 +19,7 @@ import { getVisibilityWatcher } from './lib/getVisibilityWatcher'; import { initMetric } from './lib/initMetric'; import { observe } from './lib/observe'; import { onHidden } from './lib/onHidden'; -import { FIDMetric, PerformanceEventTiming, ReportCallback, ReportOpts } from './types'; +import { FIDMetric, PerformanceEventTiming, ReportCallback } from './types'; /** * Calculates the [FID](https://web.dev/fid/) value for the current page and @@ -30,7 +30,7 @@ import { FIDMetric, PerformanceEventTiming, ReportCallback, ReportOpts } from '. * _**Important:** since FID is only reported after the user interacts with the * page, it's possible that it will not be reported for some page loads._ */ -export const onFID = (onReport: ReportCallback, opts: ReportOpts = {}): void => { +export const onFID = (onReport: ReportCallback): void => { const visibilityWatcher = getVisibilityWatcher(); const metric = initMetric('FID'); // eslint-disable-next-line prefer-const @@ -50,7 +50,7 @@ export const onFID = (onReport: ReportCallback, opts: ReportOpts = {}): void => }; const po = observe('first-input', handleEntries); - report = bindReporter(onReport, metric, opts.reportAllChanges); + report = bindReporter(onReport, metric); if (po) { onHidden(() => { diff --git a/packages/tracing/src/browser/web-vitals/getLCP.ts b/packages/tracing/src/browser/web-vitals/getLCP.ts index 286f0aa1b80b..01ddb2465ebc 100644 --- a/packages/tracing/src/browser/web-vitals/getLCP.ts +++ b/packages/tracing/src/browser/web-vitals/getLCP.ts @@ -20,7 +20,7 @@ import { getVisibilityWatcher } from './lib/getVisibilityWatcher'; import { initMetric } from './lib/initMetric'; import { observe } from './lib/observe'; import { onHidden } from './lib/onHidden'; -import { LCPMetric, ReportCallback, ReportOpts } from './types'; +import { LCPMetric, ReportCallback } from './types'; const reportedMetricIDs: Record = {}; @@ -29,13 +29,8 @@ const reportedMetricIDs: Record = {}; * calls the `callback` function once the value is ready (along with the * relevant `largest-contentful-paint` performance entry used to determine the * value). The reported value is a `DOMHighResTimeStamp`. - * - * If the `reportAllChanges` configuration option is set to `true`, the - * `callback` function will be called any time a new `largest-contentful-paint` - * performance entry is dispatched, or once the final value of the metric has - * been determined. */ -export const onLCP = (onReport: ReportCallback, opts: ReportOpts = {}): void => { +export const onLCP = (onReport: ReportCallback): void => { const visibilityWatcher = getVisibilityWatcher(); const metric = initMetric('LCP'); let report: ReturnType; @@ -61,7 +56,7 @@ export const onLCP = (onReport: ReportCallback, opts: ReportOpts = {}): void => const po = observe('largest-contentful-paint', handleEntries); if (po) { - report = bindReporter(onReport, metric, opts.reportAllChanges); + report = bindReporter(onReport, metric); const stopListening = (): void => { if (!reportedMetricIDs[metric.id]) { From 4da6327ec46b45924e059f411abd88df815108e6 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 7 Dec 2022 15:19:41 +0000 Subject: [PATCH 2/2] Deprecate _reportAllChanges rather than _metricOptions --- packages/tracing/src/browser/browsertracing.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 937ad1941378..ae135b213b9a 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -74,9 +74,18 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { markBackgroundTransactions: boolean; /** - * @deprecated This property no longer has any effect and will be removed in v8. + * _metricOptions allows the user to send options to change how metrics are collected. + * + * _metricOptions is currently experimental. + * + * Default: undefined */ - _metricOptions?: Partial<{ _reportAllChanges: boolean }>; + _metricOptions?: Partial<{ + /** + * @deprecated This property no longer has any effect and will be removed in v8. + */ + _reportAllChanges: boolean; + }>; /** * _experiments allows the user to send options to define how this integration works.