Skip to content

fix(tracing): Deprecate and remove reportAllChanges option #6456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
*
* 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.
Expand Down Expand Up @@ -162,8 +167,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();
}
Expand Down
27 changes: 12 additions & 15 deletions packages/tracing/src/browser/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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. */
Expand Down
6 changes: 3 additions & 3 deletions packages/tracing/src/browser/web-vitals/getCLS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<typeof bindReporter>;

Expand Down Expand Up @@ -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']);
Expand Down
6 changes: 3 additions & 3 deletions packages/tracing/src/browser/web-vitals/getFID.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(() => {
Expand Down
11 changes: 3 additions & 8 deletions packages/tracing/src/browser/web-vitals/getLCP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, boolean> = {};

Expand All @@ -29,13 +29,8 @@ const reportedMetricIDs: Record<string, boolean> = {};
* 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<typeof bindReporter>;
Expand All @@ -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]) {
Expand Down