fix(react): prevent duplicate spans in strict mode

packages/apm/src/integrations/tracing.ts

@@ -1,3 +1,4 @@
+// tslint:disable: max-file-line-count
 import { Hub } from '@sentry/hub';
 import { Event, EventProcessor, Integration, Severity, Span, SpanContext, TransactionContext } from '@sentry/types';
 import {
@@ -116,6 +117,7 @@ interface Activity {
 
 const global = getGlobalObject<Window>();
 const defaultTracingOrigins = ['localhost', /^\//];
+const SPAN_IGNORE_KEY = '__sentry_delete_span';
 
 /**
  * Tracing Integration
@@ -474,6 +476,11 @@ export class Tracing implements Integration {
             return span;
           }
 
+          // if a span is supposed to be ignored, don't add it to the transaction
+          if (span.data[SPAN_IGNORE_KEY]) {
+            return false;
+          }
+
           // We cancel all pending spans with status "cancelled" to indicate the idle transaction was finished early
           if (!span.endTimestamp) {
             span.endTimestamp = endTimestamp;
@@ -764,7 +771,7 @@ export class Tracing implements Integration {
   }
 
   /**
-   * Starts tracking for a specifc activity
+   * Starts tracking for a specific activity
    *
    * @param name Name of the activity, can be any string (Only used internally to identify the activity)
    * @param spanContext If provided a Span with the SpanContext will be created.
@@ -866,6 +873,31 @@ export class Tracing implements Integration {
       }, timeout);
     }
   }
+
+  /**
+   * Cancels an activity if it exists
+   */
+  public static cancelActivity(id: number): void {
+    if (!id) {
+      return;
+    }
+
+    const activity = Tracing._activities[id];
+
+    if (activity) {
+      Tracing._log(`[Tracing] cancelActivity ${activity.name}#${id}`);
+      if (activity.span) {
+        // Ignore the span in the transaction
+        activity.span.setData(SPAN_IGNORE_KEY, true);
+      }
+
+      // tslint:disable-next-line: no-dynamic-delete
+      delete Tracing._activities[id];
+
+      const count = Object.keys(Tracing._activities).length;
+      Tracing._log('[Tracing] activies count', count);
+    }
+  }
 }
 
 /**

packages/react/src/profiler.tsx

@@ -10,6 +10,11 @@ const TRACING_GETTER = ({
   id: 'Tracing',
 } as any) as IntegrationClass<Integration>;
 
+// https://stackoverflow.com/questions/52702466/detect-react-reactdom-development-production-build
+function isReactInDevMode(): boolean {
+  return '_self' in React.createElement('div');
+}
+
 /**
  *
  * Based on implementation from Preact:
@@ -39,25 +44,65 @@ function afterNextFrame(callback: Function): void {
   timeout = window.setTimeout(done, 100);
 }
 
+let profilerCount = 0;
+
+const profiledComponents: {
+  [key: string]: number;
+} = {};
+
 /**
  * getInitActivity pushes activity based on React component mount
  * @param name displayName of component that started activity
  */
 const getInitActivity = (name: string): number | null => {
   const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
 
-  if (tracingIntegration !== null) {
-    // tslint:disable-next-line:no-unsafe-any
-    return (tracingIntegration as any).constructor.pushActivity(name, {
-      description: `<${name}>`,
-      op: 'react',
-    });
+  if (tracingIntegration === null) {
+    logger.warn(
+      `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`,
+    );
+
+    return null;
   }
 
-  logger.warn(
-    `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`,
-  );
-  return null;
+  // tslint:disable-next-line:no-unsafe-any
+  const activity = (tracingIntegration as any).constructor.pushActivity(name, {
+    description: `<${name}>`,
+    op: 'react',
+  }) as number;
+
+  /**
+   * If an activity was already generated, this the component is in React.StrictMode.
+   * React.StrictMode will call constructors and setState hooks twice, effectively
+   * creating redundant spans for every render (ex. two <App /> spans, two <Link /> spans)
+   *
+   * React.StrictMode only has this behaviour in Development Mode
+   * See: https://reactjs.org/docs/strict-mode.html
+   *
+   * To account for this, we track all profiled components, and cancel activities that
+   * we recognize to be coming from redundant push activity calls. It is important to note
+   * that it is the first call to push activity that is invalid, as that is the one caused
+   * by React.StrictMode.
+   *
+   */
+  if (isReactInDevMode()) {
+    // We can make the guarantee here that if a redundant activity exists, it comes right
+    // before the current activity, hence having a profilerCount one less than the existing count.
+    const redundantActivity = profiledComponents[`${name}${profilerCount - 1}`];
+
+    if (redundantActivity) {
+      // tslint:disable-next-line: no-unsafe-any
+      (tracingIntegration as any).constructor.cancelActivity(redundantActivity);
+    } else {
+      // If an redundant activity didn't exist, we can store the current activity to
+      // check later. We have to do this inside an else block because of the case of
+      // the edge case where two components may share a single components name.
+      profiledComponents[`${name}${profilerCount}`] = activity;
+    }
+  }
+
+  profilerCount += 1;
+  return activity;
 };
 
 export type ProfilerProps = {
@@ -66,6 +111,7 @@ export type ProfilerProps = {
 
 class Profiler extends React.Component<ProfilerProps> {
   public activity: number | null;
+
   public constructor(props: ProfilerProps) {
     super(props);