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

outsideris · Lms24 · web-flow · commit 1d5c610d632f · 2022-10-07T11:07:48.000+02:00
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 &lt;outsideris@gmail.com&gt;
Co-authored-by: Lukas Stracke &lt;lukas.stracke@sentry.io&gt;
diff --git 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
@@ -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,
diff --git 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
@@ -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
@@ -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
