Skip to content

Commit

Permalink
revert: "feat(browser): Add interactionsSampleRate to `browserTraci…
Browse files Browse the repository at this point in the history
…ngIntegration` options (#12023)" (#12048)

The reason for the revert is that we're concerned about 1. the usefulness and 2. the
name of this option and would like to revisit this without blocking the
8.1.0 release. This is not semver-breaking because in 8.0.0, this API
did not exist.
  • Loading branch information
Lms24 committed May 16, 2024
1 parent 32bf90c commit ef26423
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 56 deletions.
14 changes: 3 additions & 11 deletions packages/browser-utils/src/metrics/inp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import { getBrowserPerformanceAPI, msToSec } from './utils';
/**
* Start tracking INP webvital events.
*/
export function startTrackingINP(interactionsSampleRate: number): () => void {
export function startTrackingINP(): () => void {
const performance = getBrowserPerformanceAPI();
if (performance && browserPerformanceTimeOrigin) {
const inpCallback = _trackINP(interactionsSampleRate);
const inpCallback = _trackINP();

return (): void => {
inpCallback();
Expand Down Expand Up @@ -60,16 +60,8 @@ const INP_ENTRY_MAP: Record<string, 'click' | 'hover' | 'drag' | 'press'> = {
};

/** Starts tracking the Interaction to Next Paint on the current page. */
function _trackINP(interactionsSampleRate: number): () => void {
function _trackINP(): () => void {
return addInpInstrumentationHandler(({ metric }) => {
// As specified in the `interactionsSampleRate` option, the sampling decision shall be based on
// `tracesSampleRate` x `interactionsSampleRate`
// This is the same as sampling here first on `interactionsSampleRate` and later again on `tracesSampleRate`
// (which is done in `startInactiveSpan`). Doing it this way is easier and more bundle-size efficient.
if (Math.random() > interactionsSampleRate) {
return;
}

const client = getClient();
if (!client || metric.value == undefined) {
return;
Expand Down
22 changes: 1 addition & 21 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,6 @@ export interface BrowserTracingOptions {
*/
enableInp: boolean;

/**
* Sample rate to determine interaction (INP) span sampling.
*
* The `interactionsSampleRate` is applied on top of the global `tracesSampleRate`.
* For example, a tracesSampleRate of 0.1 and interactionsSampleRate of 0.5 will result in a 0.05 sample rate
* for interactions.
*
* Default: 1
*/
interactionsSampleRate: number;

/**
* Flag to disable patching all together for fetch requests.
*
Expand Down Expand Up @@ -166,7 +155,6 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
markBackgroundSpan: true,
enableLongTask: true,
enableInp: true,
interactionsSampleRate: 1,
_experiments: {},
...defaultRequestInstrumentationOptions,
};
Expand All @@ -185,7 +173,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio

const {
enableInp,
interactionsSampleRate,
enableLongTask,
_experiments: { enableInteractions },
beforeStartSpan,
Expand All @@ -207,14 +194,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
const _collectWebVitals = startTrackingWebVitals();

if (enableInp) {
const isValidInteractionsSampleRate = interactionsSampleRate >= 0 && interactionsSampleRate <= 1;
if (isValidInteractionsSampleRate) {
DEBUG_BUILD &&
logger.warn(
`[Tracing] \`interactionsSampleRate\` must be between 0 and 1. Got: ${interactionsSampleRate}. Setting to 100%`,
);
}
startTrackingINP(isValidInteractionsSampleRate ? interactionsSampleRate : 1);
startTrackingINP();
}

if (enableLongTask) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ import {
} from '../../../src/tracing/browserTracingIntegration';
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';

import * as browserUtils from '@sentry-internal/browser-utils';

// We're setting up JSDom here because the Next.js routing instrumentations requires a few things to be present on pageload:
// 1. Access to window.document API for `window.document.getElementById`
// 2. Access to window.location API for `window.location.pathname`
Expand Down Expand Up @@ -1000,28 +998,6 @@ describe('browserTracingIntegration', () => {
});
});

describe('INP - interactionsSampleRate', () => {
const startTrackingInpSpy = jest.spyOn(browserUtils, 'startTrackingINP');

it('sets interactionsSampleRate to 1 by default', () => {
browserTracingIntegration();
expect(startTrackingInpSpy).toHaveBeenCalledWith(1);
});

it.each([0, 0.5, 1])('passes on user-defined interactionsSampleRate', interactionsSampleRate => {
browserTracingIntegration({ interactionsSampleRate });
expect(startTrackingInpSpy).toHaveBeenCalledWith(interactionsSampleRate);
});

it.each([-1, 1.1, NaN, Infinity])(
'falls back to 100% when receiving an invalid interactionsSampleRate',
interactionsSampleRate => {
browserTracingIntegration({ interactionsSampleRate });
expect(startTrackingInpSpy).toHaveBeenCalledWith(1);
},
);
});

// TODO(lforst): I cannot manage to get this test to pass.
/*
it('heartbeatInterval can be a custom value', () => {
Expand Down

0 comments on commit ef26423

Please sign in to comment.