Skip to content

Commit

Permalink
feat(browser): make heartbeat interval configurable
Browse files Browse the repository at this point in the history
Signed-off-by: Outsider <outsideris@gmail.com>
  • Loading branch information
outsideris committed Oct 3, 2022
1 parent f5cbc4c commit 07ed1fc
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 23 deletions.
14 changes: 12 additions & 2 deletions packages/tracing/src/browser/browsertracing.ts
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,14 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
*/
finalTimeout: number;

/**
* The heartbeat interval. If activities don't change in 3 heartbeats, a 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 +113,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 +222,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 @@ -262,6 +271,7 @@ export class BrowserTracing implements Integration {
finalContext,
idleTimeout,
finalTimeout,
heartbeatInterval,
true,
{ location }, // for use in the tracesSampler
);
Expand Down
3 changes: 2 additions & 1 deletion packages/tracing/src/hubextensions.ts
Expand Up @@ -186,13 +186,14 @@ export function startIdleTransaction(
transactionContext: TransactionContext,
idleTimeout: number,
finalTimeout: number,
heartbeatInterval: number,
onScope?: boolean,
customSamplingContext?: CustomSamplingContext,
): 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
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
9 changes: 8 additions & 1 deletion packages/tracing/test/browser/browsertracing.test.ts
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 @@ -266,6 +272,7 @@ describe('BrowserTracing', () => {
}),
expect.any(Number),
expect.any(Number),
expect.any(Number),
expect.any(Boolean),
expect.any(Object),
);
Expand Down
49 changes: 32 additions & 17 deletions packages/tracing/test/idletransaction.test.ts
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

0 comments on commit 07ed1fc

Please sign in to comment.