Ref/apm transaction timeout (#2412)

* ref: Correct check for transaction length

* meta: Travis bumps, Readme and changelog

* ref: Rename option, Make time in seconds

* fix: Wording

* Update packages/apm/src/integrations/tracing.ts

Co-Authored-By: Kamil Ogórek <kamil.ogorek@gmail.com>

Author: HazAT

.travis.yml

@@ -6,9 +6,8 @@ branches:
     - master
     - /^release\/.+$/
     - /^major\/.+$/
-    - 4.x
 
-install: yarn
+install: yarn --ignore-engines
 os: linux
 
 language: node_js
@@ -39,32 +38,9 @@ jobs:
     - name: '@sentry/browser - browserstack integration tests'
       node_js: '10'
       script: scripts/browser-integration.sh
-    - name: 'raven-node [node v4]'
-      if: branch = 4.x
-      node_js: '4'
-      script: scripts/raven-node.sh
-    - name: 'raven-node [node v6]'
-      if: branch = 4.x
-      node_js: '6'
-      script: scripts/raven-node.sh
-    - name: 'raven-node [node v8]'
-      if: branch = 4.x
-      node_js: '8'
-      script: scripts/raven-node.sh
-    - name: 'raven-node [node v10]'
-      if: branch = 4.x
-      node_js: '10'
-      script: scripts/raven-node.sh
-    - name: 'raven-js - unit and integration tests'
-      if: branch = 4.x
-      node_js: '8'
-      addons:
-        chrome: stable
-        firefox: latest
-      script: scripts/raven-js.sh
     - stage: Deploy
       name: '@sentry/packages - pack and zeus upload'
-      node_js: '10'
+      node_js: '12'
       script: scripts/pack-and-upload.sh || [[ ! "$TRAVIS_BRANCH" =~ ^release/ ]]
 
 notifications:

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+## 5.12.2
+
+- [apm] ref: Check if Tracing integration is enabled before dropping transaction
+
 ## 5.12.1
 
 - [apm] ref: If `maxTransactionTimeout` = `0` there is no timeout

README.md

@@ -37,6 +37,7 @@ package. Please refer to the README and instructions of those SDKs for more deta
   including integrations for React, Angular, Ember, Vue and Backbone
 - [`@sentry/node`](https://github.com/getsentry/sentry-javascript/tree/master/packages/node): SDK for Node, including
   integrations for Express, Koa, Loopback, Sails and Connect
+- [`@sentry/react-native`](https://github.com/getsentry/sentry-react-native): SDK for React Native with support for native crashes
 - [`@sentry/integrations`](https://github.com/getsentry/sentry-javascript/tree/master/packages/integrations): Pluggable
   integrations that can be used to enhance JS SDKs
 - [`@sentry/electron`](https://github.com/getsentry/sentry-electron): SDK for Electron with support for native crashes
@@ -77,7 +78,8 @@ Besides the high-level SDKs, this repository contains shared packages, helpers a
 development. If you're thinking about contributing to or creating a JavaScript-based SDK, have a look at the resources
 below:
 
-
+- [`@sentry/apm`](https://github.com/getsentry/sentry-javascript/tree/master/packages/apm): Provides Integrations and
+extensions for APM
 - [`@sentry/hub`](https://github.com/getsentry/sentry-javascript/tree/master/packages/hub): Global state management of
   SDKs
 - [`@sentry/minimal`](https://github.com/getsentry/sentry-javascript/tree/master/packages/minimal): Minimal SDK for

packages/apm/README.md

@@ -7,8 +7,6 @@
 
 # Sentry APM Extensions
 
-# TODO
-
 [![npm version](https://img.shields.io/npm/v/@sentry/apm.svg)](https://www.npmjs.com/package/@sentry/apm)
 [![npm dm](https://img.shields.io/npm/dm/@sentry/apm.svg)](https://www.npmjs.com/package/@sentry/apm)
 [![npm dt](https://img.shields.io/npm/dt/@sentry/apm.svg)](https://www.npmjs.com/package/@sentry/apm)

packages/apm/src/integrations/tracing.ts

@@ -5,7 +5,6 @@ import {
   isMatchingPattern,
   logger,
   supportsNativeFetch,
-  timestampWithMs,
 } from '@sentry/utils';
 
 import { Span as SpanClass } from '../span';
@@ -43,6 +42,7 @@ interface TracingOptions {
   /**
    * The time to wait in ms until the transaction will be finished. The transaction will use the end timestamp of
    * the last finished span as the endtime for the transaction.
+   * Time is in ms.
    *
    * Default: 500
    */
@@ -65,14 +65,15 @@ interface TracingOptions {
   tracesSampleRate: number;
 
   /**
-   * The maximum time a transaction can be before it will be dropped. This is for some edge cases where a browser
-   * completely freezes the JS state and picks it up later. So after this timeout, the SDK will not send the event.
-   * If you want to have an unlimited timeout set it to 0.
-   * Time is in ms.
+   * The maximum duration of a transaction before it will be discarded. This is for some edge cases where a browser
+   * completely freezes the JS state and picks it up later (background tabs).
+   * So after this duration, the SDK will not send the event.
+   * If you want to have an unlimited duration set it to 0.
+   * Time is in seconds.
    *
-   * Default: 600000 = 10min
+   * Default: 600
    */
-  maxTransactionTimeout: number;
+  maxTransactionDuration: number;
 }
 
 /** JSDoc */
@@ -127,7 +128,7 @@ export class Tracing implements Integration {
     const defaultTracingOrigins = ['localhost', /^\//];
     const defaults = {
       idleTimeout: 500,
-      maxTransactionTimeout: 600000,
+      maxTransactionDuration: 600,
       shouldCreateSpanForRequest(url: string): boolean {
         const origins = (_options && _options.tracingOrigins) || defaultTracingOrigins;
         return (
@@ -194,18 +195,21 @@ export class Tracing implements Integration {
       });
     }
 
-    // This EventProcessor makes sure that we never send an transaction that is older than maxTransactionTimeout
+    // This EventProcessor makes sure that the transaction is not longer than maxTransactionDuration
     addGlobalEventProcessor((event: Event) => {
       const self = getCurrentHub().getIntegration(Tracing);
       if (!self) {
         return event;
       }
 
       if (
+        Tracing.options.maxTransactionDuration !== 0 &&
+        Tracing._isEnabled() &&
         event.type === 'transaction' &&
         event.timestamp &&
-        Tracing.options.maxTransactionTimeout !== 0 &&
-        timestampWithMs() > event.timestamp + Tracing.options.maxTransactionTimeout
+        event.start_timestamp &&
+        (event.timestamp - event.start_timestamp > Tracing.options.maxTransactionDuration ||
+          event.timestamp - event.start_timestamp < 0)
       ) {
         return null;
       }
@@ -302,16 +306,8 @@ export class Tracing implements Integration {
   public static finishIdleTransaction(): void {
     const active = Tracing._activeTransaction as SpanClass;
     if (active) {
-      if (
-        Tracing.options.maxTransactionTimeout !== 0 &&
-        timestampWithMs() > active.startTimestamp + Tracing.options.maxTransactionTimeout
-      ) {
-        // If we reached the max timeout of the transaction, we will just not finish it and therefore discard it.
-        Tracing._activeTransaction = undefined;
-      } else {
-        // true = use timestamp of last span
-        active.finish(true);
-      }
+      // true = use timestamp of last span
+      active.finish(true);
     }
   }