feat[apm]: Add discardBackgroundSpans option (#2471)

* feat: Add discardBackgroundSpans option

* ref: CodeReview + Discard all activities

Author: HazAT

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+- [apm] feat: Add `discardBackgroundSpans` to discard background spans by default
+
 ## 5.13.1
 
 - [node] fix: Restore engines back to `>= 6`

packages/apm/src/integrations/tracing.ts

@@ -74,6 +74,16 @@ interface TracingOptions {
    * Default: 600
    */
   maxTransactionDuration: number;
+
+  /**
+   * Flag to discard all spans that occur in background. This includes transactions. Browser background tab timing is
+   * not suited towards doing precise measurements of operations. That's why this option discards any active transaction
+   * and also doesn't add any spans that happen in the background. Background spans/transaction can mess up your
+   * statistics in non deterministic ways that's why we by default recommend leaving this opition enabled.
+   *
+   * Default: true
+   */
+  discardBackgroundSpans: boolean;
 }
 
 /** JSDoc */
@@ -114,9 +124,9 @@ export class Tracing implements Integration {
 
   private static _activeTransaction?: Span;
 
-  private static _currentIndex: number = 0;
+  private static _currentIndex: number = 1;
 
-  public static readonly _activities: { [key: number]: Activity } = {};
+  public static _activities: { [key: number]: Activity } = {};
 
   private static _debounce: number = 0;
 
@@ -127,8 +137,9 @@ export class Tracing implements Integration {
    *
    * @param _options TracingOptions
    */
-  public constructor(private readonly _options?: Partial<TracingOptions>) {
+  public constructor(_options?: Partial<TracingOptions>) {
     const defaults = {
+      discardBackgroundSpans: true,
       idleTimeout: 500,
       maxTransactionDuration: 600,
       shouldCreateSpanForRequest(url: string): boolean {
@@ -148,7 +159,7 @@ export class Tracing implements Integration {
     if (!_options || !Array.isArray(_options.tracingOrigins) || _options.tracingOrigins.length === 0) {
       this._emitOptionsWarning = true;
     }
-    Tracing.options = this._options = {
+    Tracing.options = {
       ...defaults,
       ..._options,
     };
@@ -171,23 +182,21 @@ export class Tracing implements Integration {
       return;
     }
 
-    // tslint:disable-next-line: no-non-null-assertion
-    if (this._options!.traceXHR !== false) {
+    if (Tracing.options.traceXHR) {
       addInstrumentationHandler({
         callback: xhrCallback,
         type: 'xhr',
       });
     }
-    // tslint:disable-next-line: no-non-null-assertion
-    if (this._options!.traceFetch !== false && supportsNativeFetch()) {
+
+    if (Tracing.options.traceFetch && supportsNativeFetch()) {
       addInstrumentationHandler({
         callback: fetchCallback,
         type: 'fetch',
       });
     }
 
-    // tslint:disable-next-line: no-non-null-assertion
-    if (this._options!.startTransactionOnLocationChange) {
+    if (Tracing.options.startTransactionOnLocationChange) {
       addInstrumentationHandler({
         callback: historyCallback,
         type: 'history',
@@ -202,6 +211,16 @@ export class Tracing implements Integration {
       });
     }
 
+    if (Tracing.options.discardBackgroundSpans && global.document) {
+      document.addEventListener('visibilitychange', () => {
+        if (document.hidden && Tracing._activeTransaction) {
+          logger.log('[Tracing] Discarded active transaction incl. activities since tab moved to the background');
+          Tracing._activeTransaction = undefined;
+          Tracing._activities = {};
+        }
+      });
+    }
+
     // This EventProcessor makes sure that the transaction is not longer than maxTransactionDuration
     addGlobalEventProcessor((event: Event) => {
       const self = getCurrentHub().getIntegration(Tracing);
@@ -351,6 +370,10 @@ export class Tracing implements Integration {
       // Tracing is not enabled
       return 0;
     }
+    if (!Tracing._activeTransaction) {
+      logger.log(`[Tracing] Not pushing activity ${name} since there is no active transaction`);
+      return 0;
+    }
 
     // We want to clear the timeout also here since we push a new activity
     clearTimeout(Tracing._debounce);
@@ -389,7 +412,10 @@ export class Tracing implements Integration {
    * Removes activity and finishes the span in case there is one
    */
   public static popActivity(id: number, spanData?: { [key: string]: any }): void {
-    if (!Tracing._isEnabled()) {
+    // The !id is on purpose to also fail with 0
+    // Since 0 is returned by push activity in case tracing is not enabled
+    // or there is no active transaction
+    if (!Tracing._isEnabled() || !id) {
       // Tracing is not enabled
       return;
     }
@@ -422,7 +448,7 @@ export class Tracing implements Integration {
 
     logger.log('[Tracing] activies count', count);
 
-    if (count === 0) {
+    if (count === 0 && Tracing._activeTransaction) {
       const timeout = Tracing.options && Tracing.options.idleTimeout;
       logger.log(`[Tracing] Flushing Transaction in ${timeout}ms`);
       Tracing._debounce = (setTimeout(() => {