Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(tracing): Make BrowserTracing heartbeat interval configurable #5867

Merged
merged 7 commits into from Oct 7, 2022
Merged

feat(tracing): Make BrowserTracing heartbeat interval configurable #5867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0d1b6cc
feat(browser): make heartbeat interval configurable
outsideris Oct 2, 2022
a7b0cfc
feat(browser): make heartbeatInterval optional
outsideris Oct 3, 2022
7294db1
feat(browser): test for heartbeatInterval
outsideris Oct 3, 2022
60ab075
Update packages/tracing/src/browser/browsertracing.ts
outsideris Oct 3, 2022
aa1b794
feat(browser): fix lint errors
outsideris Oct 4, 2022
8f7ca31
Update packages/tracing/src/hubextensions.ts
outsideris Oct 5, 2022
fafb533
fix tests as startIdleTransaction signature
outsideris Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions packages/tracing/src/browser/browsertracing.ts
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@ import { EventProcessor, Integration, Transaction, TransactionContext } from '@s
import { baggageHeaderToDynamicSamplingContext, getDomElement, getGlobalObject, logger } from '@sentry/utils';

import { startIdleTransaction } from '../hubextensions';
import { DEFAULT_FINAL_TIMEOUT, DEFAULT_IDLE_TIMEOUT } from '../idletransaction';
import { DEFAULT_FINAL_TIMEOUT, DEFAULT_HEARTBEAT_INTERVAL, DEFAULT_IDLE_TIMEOUT } from '../idletransaction';
import { extractTraceparentData } from '../utils';
import { registerBackgroundTabDetection } from './backgroundtab';
import { addPerformanceEntries, startTrackingLongTasks, startTrackingWebVitals } from './metrics';
Expand Down Expand Up @@ -40,6 +40,15 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
*/
finalTimeout: number;

/**
* The heartbeat interval. If no new spans are started or open spans are finished within 3 heartbeats,
* the transaction will be finished.
* Time is in ms.
*
* Default: 5000
*/
heartbeatInterval: number;

/**
* Flag to enable/disable creation of `navigation` transaction on history changes.
*
Expand Down Expand Up @@ -105,6 +114,7 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
const DEFAULT_BROWSER_TRACING_OPTIONS = {
idleTimeout: DEFAULT_IDLE_TIMEOUT,
finalTimeout: DEFAULT_FINAL_TIMEOUT,
heartbeatInterval: DEFAULT_HEARTBEAT_INTERVAL,
markBackgroundTransactions: true,
routingInstrumentation: instrumentRoutingWithDefaults,
startTransactionOnLocationChange: true,
Expand Down Expand Up @@ -213,7 +223,7 @@ export class BrowserTracing implements Integration {
}

// eslint-disable-next-line @typescript-eslint/unbound-method
const { beforeNavigate, idleTimeout, finalTimeout } = this.options;
const { beforeNavigate, idleTimeout, finalTimeout, heartbeatInterval } = this.options;

const isPageloadTransaction = context.op === 'pageload';

Expand Down Expand Up @@ -264,6 +274,7 @@ export class BrowserTracing implements Integration {
finalTimeout,
true,
{ location }, // for use in the tracesSampler
heartbeatInterval,
);
idleTransaction.registerBeforeFinishCallback(transaction => {
addPerformanceEntries(transaction);
Expand Down
3 changes: 2 additions & 1 deletion packages/tracing/src/hubextensions.ts
View file
Open in desktop
Expand Up @@ -188,11 +188,12 @@ export function startIdleTransaction(
finalTimeout: number,
onScope?: boolean,
customSamplingContext?: CustomSamplingContext,
heartbeatInterval?: number,
): IdleTransaction {
const client = hub.getClient();
const options: Partial<ClientOptions> = (client && client.getOptions()) || {};

let transaction = new IdleTransaction(transactionContext, hub, idleTimeout, finalTimeout, onScope);
let transaction = new IdleTransaction(transactionContext, hub, idleTimeout, finalTimeout, heartbeatInterval, onScope);
transaction = sample(transaction, options, {
parentSampled: transactionContext.parentSampled,
transactionContext,
Expand Down
5 changes: 3 additions & 2 deletions packages/tracing/src/idletransaction.ts
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ import { Transaction } from './transaction';

export const DEFAULT_IDLE_TIMEOUT = 1000;
export const DEFAULT_FINAL_TIMEOUT = 30000;
export const HEARTBEAT_INTERVAL = 5000;
export const DEFAULT_HEARTBEAT_INTERVAL = 5000;

/**
* @inheritDoc
Expand Down Expand Up @@ -85,6 +85,7 @@ export class IdleTransaction extends Transaction {
* The final value in ms that a transaction cannot exceed
*/
private readonly _finalTimeout: number = DEFAULT_FINAL_TIMEOUT,
private readonly _heartbeatInterval: number = DEFAULT_HEARTBEAT_INTERVAL,
// Whether or not the transaction should put itself on the scope when it starts and pop itself off when it ends
private readonly _onScope: boolean = false,
) {
Expand Down Expand Up @@ -287,7 +288,7 @@ export class IdleTransaction extends Transaction {
__DEBUG_BUILD__ && logger.log(`pinging Heartbeat -> current counter: ${this._heartbeatCounter}`);
setTimeout(() => {
this._beat();
}, HEARTBEAT_INTERVAL);
}, this._heartbeatInterval);
}
}

Expand Down
26 changes: 25 additions & 1 deletion packages/tracing/test/browser/browsertracing.test.ts
View file
Open in desktop
Expand Up @@ -8,7 +8,12 @@ import { BrowserTracing, BrowserTracingOptions, getMetaContent } from '../../src
import { defaultRequestInstrumentationOptions } from '../../src/browser/request';
import { instrumentRoutingWithDefaults } from '../../src/browser/router';
import * as hubExtensions from '../../src/hubextensions';
import { DEFAULT_FINAL_TIMEOUT, DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../../src/idletransaction';
import {
DEFAULT_FINAL_TIMEOUT,
DEFAULT_HEARTBEAT_INTERVAL,
DEFAULT_IDLE_TIMEOUT,
IdleTransaction,
} from '../../src/idletransaction';
import { getActiveTransaction } from '../../src/utils';
import { getDefaultBrowserClientOptions } from '../testutils';

Expand Down Expand Up @@ -83,6 +88,7 @@ describe('BrowserTracing', () => {
},
idleTimeout: DEFAULT_IDLE_TIMEOUT,
finalTimeout: DEFAULT_FINAL_TIMEOUT,
heartbeatInterval: DEFAULT_HEARTBEAT_INTERVAL,
markBackgroundTransactions: true,
routingInstrumentation: instrumentRoutingWithDefaults,
startTransactionOnLocationChange: true,
Expand Down Expand Up @@ -268,6 +274,7 @@ describe('BrowserTracing', () => {
expect.any(Number),
expect.any(Boolean),
expect.any(Object),
expect.any(Number),
);
});

Expand Down Expand Up @@ -300,6 +307,23 @@ describe('BrowserTracing', () => {
expect(mockFinish).toHaveBeenCalledTimes(1);
});
});

describe('heartbeatInterval', () => {
it('can be a custom value', () => {
const interval = 200;
createBrowserTracing(true, { heartbeatInterval: interval, routingInstrumentation: customInstrumentRouting });
const mockFinish = jest.fn();
const transaction = getActiveTransaction(hub) as IdleTransaction;
transaction.finish = mockFinish;

const span = transaction.startChild(); // activities = 1
span.finish(); // activities = 0

expect(mockFinish).toHaveBeenCalledTimes(0);
jest.advanceTimersByTime(interval * 3);
expect(mockFinish).toHaveBeenCalledTimes(1);
});
});
});

// Integration tests for the default routing instrumentation
Expand Down
49 changes: 32 additions & 17 deletions packages/tracing/test/idletransaction.test.ts
View file
Open in desktop
Expand Up @@ -3,8 +3,8 @@ import { Hub } from '@sentry/hub';

import {
DEFAULT_FINAL_TIMEOUT,
DEFAULT_HEARTBEAT_INTERVAL,
DEFAULT_IDLE_TIMEOUT,
HEARTBEAT_INTERVAL,
IdleTransaction,
IdleTransactionSpanRecorder,
} from '../src/idletransaction';
Expand All @@ -21,7 +21,14 @@ beforeEach(() => {
describe('IdleTransaction', () => {
describe('onScope', () => {
it('sets the transaction on the scope on creation if onScope is true', () => {
const transaction = new IdleTransaction({ name: 'foo' }, hub, DEFAULT_IDLE_TIMEOUT, DEFAULT_FINAL_TIMEOUT, true);
const transaction = new IdleTransaction(
{ name: 'foo' },
hub,
DEFAULT_IDLE_TIMEOUT,
DEFAULT_FINAL_TIMEOUT,
DEFAULT_HEARTBEAT_INTERVAL,
true,
);
transaction.initSpanRecorder(10);

hub.configureScope(s => {
Expand All @@ -39,7 +46,14 @@ describe('IdleTransaction', () => {
});

it('removes sampled transaction from scope on finish if onScope is true', () => {
const transaction = new IdleTransaction({ name: 'foo' }, hub, DEFAULT_IDLE_TIMEOUT, DEFAULT_FINAL_TIMEOUT, true);
const transaction = new IdleTransaction(
{ name: 'foo' },
hub,
DEFAULT_IDLE_TIMEOUT,
DEFAULT_FINAL_TIMEOUT,
DEFAULT_HEARTBEAT_INTERVAL,
true,
);
transaction.initSpanRecorder(10);

transaction.finish();
Expand All @@ -56,6 +70,7 @@ describe('IdleTransaction', () => {
hub,
DEFAULT_IDLE_TIMEOUT,
DEFAULT_FINAL_TIMEOUT,
DEFAULT_HEARTBEAT_INTERVAL,
true,
);

Expand Down Expand Up @@ -248,17 +263,17 @@ describe('IdleTransaction', () => {
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 1
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(transaction.status).not.toEqual('deadline_exceeded');
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 2
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(transaction.status).not.toEqual('deadline_exceeded');
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 3
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(transaction.status).not.toEqual('deadline_exceeded');
expect(mockFinish).toHaveBeenCalledTimes(0);
});
Expand All @@ -272,15 +287,15 @@ describe('IdleTransaction', () => {
transaction.startChild({});

// Beat 1
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 2
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 3
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(1);
});

Expand All @@ -293,42 +308,42 @@ describe('IdleTransaction', () => {
transaction.startChild({});

// Beat 1
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

const span = transaction.startChild(); // push activity

// Beat 1
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 2
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

transaction.startChild(); // push activity
transaction.startChild(); // push activity

// Beat 1
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 2
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

span.finish(); // pop activity

// Beat 1
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 2
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(0);

// Beat 3
jest.advanceTimersByTime(HEARTBEAT_INTERVAL);
jest.advanceTimersByTime(DEFAULT_HEARTBEAT_INTERVAL);
expect(mockFinish).toHaveBeenCalledTimes(1);

// Heartbeat does not keep going after finish has been called
Expand Down