diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index a6ce61fe4192..0e904ab5ddf2 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/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(); 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); + } + } } /** diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 8cf86ed89009..89d5d3f73a2f 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -10,6 +10,11 @@ const TRACING_GETTER = ({ id: 'Tracing', } as any) as IntegrationClass; +// 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,6 +44,12 @@ 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 @@ -46,18 +57,52 @@ function afterNextFrame(callback: Function): void { 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 spans, two 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 { public activity: number | null; + public constructor(props: ProfilerProps) { super(props);