fix: Make APM optional in gatsby package (getsentry#2752)

AbhiPrasad · web-flow · commit cd7d8872ca11 · 2020-07-20T08:11:39.000-04:00
* fix: Make APM optional in gatsby package

* ref: Use idleTimout if no activities occur in idle transaction
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 - [react] feat: Export `createReduxEnhancer` to log redux actions as breadcrumbs, and attach state as an extra. (#2717)
 - [tracing] feat: `Add @sentry/tracing` (#2719)
+- [gatsby] fix: Make APM optional in gatsby package (#2752)
+- [tracing] ref: Use idleTimout if no activities occur in idle transaction (#2752)
 
 ## 5.19.2
 
diff --git a/packages/gatsby/README.md b/packages/gatsby/README.md
@@ -24,7 +24,7 @@ Register the package as a plugin in `gastby-config.js`:
 }
 ```
 
-Options will be passed directly to `Sentry.init`. The `environment` value defaults to `NODE_ENV` (or `development` if not set).
+Options will be passed directly to `Sentry.init`. See all available options in [our docs](https://docs.sentry.io/error-reporting/configuration/?platform=javascript). The `environment` value defaults to `NODE_ENV` (or `development` if not set).
 
 ## GitHub Actions
 
@@ -38,6 +38,26 @@ The `release` value is inferred from `COMMIT_REF`.
 
 To automatically capture the `release` value on Vercel you will need to register appropriate [system environment variable](https://vercel.com/docs/v2/build-step#system-environment-variables) (e.g. `VERCEL_GITHUB_COMMIT_SHA`) in your project.
 
+## Sentry Performance
+
+To enable Tracing support, supply the `tracesSampleRate` to the options and make sure you have installed the `@sentry/tracing` package.
+
+```javascript
+{
+  // ...
+  plugins: [
+    {
+      resolve: "@sentry/gatsby",
+      options: {
+          dsn: process.env.SENTRY_DSN, // this is the default
+          tracesSampleRate: 1, // this is just to test, you should lower this in production
+      }
+    },
+    // ...
+  ]
+}
+```
+
 ## Links
 
 - [Official SDK Docs](https://docs.sentry.io/quickstart/)
diff --git a/packages/gatsby/gatsby-browser.js b/packages/gatsby/gatsby-browser.js
@@ -1,12 +1,28 @@
 exports.onClientEntry = function(_, pluginParams) {
-  require.ensure(['@sentry/react', '@sentry/apm'], function(require) {
+  require.ensure(['@sentry/react'], function(require) {
     const Sentry = require('@sentry/react');
-    const TracingIntegration = require('@sentry/apm').Integrations.Tracing;
+
+    let TracingIntegration = undefined;
+    let BrowserTracingIntegration = undefined;
+    try {
+      BrowserTracingIntegration = require('@sentry/tracing').Integrations.BrowserTracing;
+    } catch (_) {}
+    try {
+      /** @deprecated Remove when @sentry/apm is no longer used */
+      TracingIntegration = require('@sentry/apm').Integrations.Tracing;
+    } catch (_) {}
+
     const tracesSampleRate = pluginParams.tracesSampleRate !== undefined ? pluginParams.tracesSampleRate : 0;
     const integrations = [...(pluginParams.integrations || [])];
+
     if (tracesSampleRate) {
-      integrations.push(new TracingIntegration());
+      if (BrowserTracingIntegration) {
+        integrations.push(new BrowserTracingIntegration());
+      } else if (TracingIntegration) {
+        integrations.push(new TracingIntegration());
+      }
     }
+
     Sentry.init({
       environment: process.env.NODE_ENV || 'development',
       release: __SENTRY_RELEASE__,
@@ -15,6 +31,7 @@ exports.onClientEntry = function(_, pluginParams) {
       tracesSampleRate,
       integrations,
     });
+
     Sentry.addGlobalEventProcessor(event => {
       event.sdk = {
         ...event.sdk,
diff --git a/packages/tracing/src/idletransaction.ts b/packages/tracing/src/idletransaction.ts
@@ -70,6 +70,10 @@ export class IdleTransaction extends Transaction {
 
   private readonly _beforeFinishCallbacks: BeforeFinishCallback[] = [];
 
+  // If a transaction is created and no activities are added, we want to make sure that
+  // it times out properly. This is cleared and not used when activities are added.
+  private _initTimeout: any;
+
   public constructor(
     transactionContext: TransactionContext,
     private readonly _idleHub?: Hub,
@@ -188,6 +192,11 @@ export class IdleTransaction extends Transaction {
    * @param spanId The span id that represents the activity
    */
   private _pushActivity(spanId: string): void {
+    if (this._initTimeout) {
+      // tslint:disable-next-line: no-unsafe-any
+      clearTimeout(this._initTimeout);
+      this._initTimeout = undefined;
+    }
     logger.log(`[Tracing] pushActivity: ${spanId}`);
     this.activities[spanId] = true;
     logger.log('[Tracing] new activities count', Object.keys(this.activities).length);
@@ -235,12 +244,25 @@ export class IdleTransaction extends Transaction {
    */
   public initSpanRecorder(maxlen?: number): void {
     if (!this.spanRecorder) {
+      this._initTimeout = setTimeout(() => {
+        if (!this._finished) {
+          this.finish();
+        }
+      }, this._idleTimeout);
+
       const pushActivity = (id: string) => {
+        if (this._finished) {
+          return;
+        }
         this._pushActivity(id);
       };
       const popActivity = (id: string) => {
+        if (this._finished) {
+          return;
+        }
         this._popActivity(id);
       };
+
       this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanId, maxlen);
 
       // Start heartbeat so that transactions do not run forever.
diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts
@@ -1,7 +1,7 @@
 import { BrowserClient } from '@sentry/browser';
 import { Hub } from '@sentry/hub';
 
-import { IdleTransaction, IdleTransactionSpanRecorder } from '../src/idletransaction';
+import { IdleTransaction, IdleTransactionSpanRecorder, DEFAULT_IDLE_TIMEOUT } from '../src/idletransaction';
 import { Span } from '../src/span';
 import { SpanStatus } from '../src/spanstatus';
 
@@ -145,6 +145,25 @@ describe('IdleTransaction', () => {
     }
   });
 
+  describe('_initTimeout', () => {
+    it('finishes if no activities are added to the transaction', () => {
+      const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, 1000);
+      transaction.initSpanRecorder(10);
+
+      jest.runTimersToTime(DEFAULT_IDLE_TIMEOUT);
+      expect(transaction.endTimestamp).toBeDefined();
+    });
+
+    it('does not finish if a activity is started', () => {
+      const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, 1000);
+      transaction.initSpanRecorder(10);
+      transaction.startChild({});
+
+      jest.runTimersToTime(DEFAULT_IDLE_TIMEOUT);
+      expect(transaction.endTimestamp).toBeUndefined();
+    });
+  });
+
   describe('heartbeat', () => {
     it('does not start heartbeat if there is no span recorder', () => {
       const transaction = new IdleTransaction({ name: 'foo' }, hub, 1000);
@@ -164,12 +183,14 @@ describe('IdleTransaction', () => {
       jest.runOnlyPendingTimers();
       expect(mockFinish).toHaveBeenCalledTimes(0);
     });
+
     it('finishes a transaction after 3 beats', () => {
       const transaction = new IdleTransaction({ name: 'foo' }, hub, 1000);
       const mockFinish = jest.spyOn(transaction, 'finish');
       transaction.initSpanRecorder(10);
 
       expect(mockFinish).toHaveBeenCalledTimes(0);
+      transaction.startChild({});
 
       // Beat 1
       jest.runOnlyPendingTimers();
@@ -190,6 +211,7 @@ describe('IdleTransaction', () => {
       transaction.initSpanRecorder(10);
 
       expect(mockFinish).toHaveBeenCalledTimes(0);
+      transaction.startChild({});
 
       // Beat 1
       jest.runOnlyPendingTimers();
