From 1d5c610d632f0f3dbc6b2d06910bfc76707ce50a Mon Sep 17 00:00:00 2001 From: Outsider Date: Fri, 7 Oct 2022 18:07:48 +0900 Subject: [PATCH] feat(tracing): Make BrowserTracing heartbeat interval configurable (#5867) Make the heartbeat interval of an `IdleTransaction` configurable via `BrowserTracingOptions`. Add an optional parameter to `startIdleTransaction` to pass in a custom headtbeat interval. Signed-off-by: Outsider Co-authored-by: Lukas Stracke --- .../tracing/src/browser/browsertracing.ts | 15 +++++- packages/tracing/src/hubextensions.ts | 3 +- packages/tracing/src/idletransaction.ts | 5 +- .../test/browser/browsertracing.test.ts | 26 +++++++++- packages/tracing/test/idletransaction.test.ts | 49 ++++++++++++------- 5 files changed, 75 insertions(+), 23 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 8a06f029d53f..f0924b543450 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -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'; @@ -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. * @@ -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, @@ -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'; @@ -264,6 +274,7 @@ export class BrowserTracing implements Integration { finalTimeout, true, { location }, // for use in the tracesSampler + heartbeatInterval, ); idleTransaction.registerBeforeFinishCallback(transaction => { addPerformanceEntries(transaction); diff --git a/packages/tracing/src/hubextensions.ts b/packages/tracing/src/hubextensions.ts index dbb9254054e9..e3f953805697 100644 --- a/packages/tracing/src/hubextensions.ts +++ b/packages/tracing/src/hubextensions.ts @@ -188,11 +188,12 @@ export function startIdleTransaction( finalTimeout: number, onScope?: boolean, customSamplingContext?: CustomSamplingContext, + heartbeatInterval?: number, ): IdleTransaction { const client = hub.getClient(); const options: Partial = (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, diff --git a/packages/tracing/src/idletransaction.ts b/packages/tracing/src/idletransaction.ts index b12ba4b34601..c19f187fd00a 100644 --- a/packages/tracing/src/idletransaction.ts +++ b/packages/tracing/src/idletransaction.ts @@ -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 @@ -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, ) { @@ -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); } } diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 2fc24fa1d670..325078d05acf 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -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'; @@ -83,6 +88,7 @@ describe('BrowserTracing', () => { }, idleTimeout: DEFAULT_IDLE_TIMEOUT, finalTimeout: DEFAULT_FINAL_TIMEOUT, + heartbeatInterval: DEFAULT_HEARTBEAT_INTERVAL, markBackgroundTransactions: true, routingInstrumentation: instrumentRoutingWithDefaults, startTransactionOnLocationChange: true, @@ -268,6 +274,7 @@ describe('BrowserTracing', () => { expect.any(Number), expect.any(Boolean), expect.any(Object), + expect.any(Number), ); }); @@ -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 diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts index 956de96b5ea5..1e7af1d78c86 100644 --- a/packages/tracing/test/idletransaction.test.ts +++ b/packages/tracing/test/idletransaction.test.ts @@ -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'; @@ -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 => { @@ -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(); @@ -56,6 +70,7 @@ describe('IdleTransaction', () => { hub, DEFAULT_IDLE_TIMEOUT, DEFAULT_FINAL_TIMEOUT, + DEFAULT_HEARTBEAT_INTERVAL, true, ); @@ -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); }); @@ -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); }); @@ -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