From e997d54c0fb5382d2371118ef9f4429be1dbdc1c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 15 Jun 2020 17:35:48 -0400 Subject: [PATCH 01/18] fix(react): check for event processor --- packages/react/src/index.ts | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index fc67e410912c..3d8842ad43d6 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -1,22 +1,24 @@ import { addGlobalEventProcessor, SDK_VERSION } from '@sentry/browser'; function createReactEventProcessor(): void { - addGlobalEventProcessor(event => { - event.sdk = { - ...event.sdk, - name: 'sentry.javascript.react', - packages: [ - ...((event.sdk && event.sdk.packages) || []), - { - name: 'npm:@sentry/react', - version: SDK_VERSION, - }, - ], - version: SDK_VERSION, - }; + if (addGlobalEventProcessor) { + addGlobalEventProcessor(event => { + event.sdk = { + ...event.sdk, + name: 'sentry.javascript.react', + packages: [ + ...((event.sdk && event.sdk.packages) || []), + { + name: 'npm:@sentry/react', + version: SDK_VERSION, + }, + ], + version: SDK_VERSION, + }; - return event; - }); + return event; + }); + } } export * from '@sentry/browser'; From c2551373b3c5c7259c3270fc80874e40717fcd86 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 09:53:03 -0400 Subject: [PATCH 02/18] ref: use global tracing integration --- packages/react/src/profiler.tsx | 50 ++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 8cf86ed89009..2ac8f3e1f2e2 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -39,24 +39,37 @@ function afterNextFrame(callback: Function): void { timeout = window.setTimeout(done, 100); } +let globalTracingIntegration: Integration | null = null; +const getTracingIntegration = () => { + if (globalTracingIntegration) { + return globalTracingIntegration; + } + + globalTracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); + return globalTracingIntegration; +}; + +/** JSDOC */ +function warnAboutTracing(name: string): void { + if (globalTracingIntegration === null) { + logger.warn( + `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`, + ); + } +} + /** * 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) { + if (globalTracingIntegration !== null) { // tslint:disable-next-line:no-unsafe-any - return (tracingIntegration as any).constructor.pushActivity(name, { + return (globalTracingIntegration as any).constructor.pushActivity(name, { description: `<${name}>`, op: 'react', }); } - - logger.warn( - `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`, - ); return null; }; @@ -65,11 +78,17 @@ export type ProfilerProps = { }; class Profiler extends React.Component { - public activity: number | null; + public activity: number | null = null; + public tracingIntegration: Integration | null = getTracingIntegration(); + public constructor(props: ProfilerProps) { super(props); - this.activity = getInitActivity(this.props.name); + if (this.tracingIntegration) { + this.activity = getInitActivity(this.props.name); + } else { + warnAboutTracing(this.props.name); + } } // If a component mounted, we can finish the mount activity. @@ -84,16 +103,13 @@ class Profiler extends React.Component { } public finishProfile = () => { - if (!this.activity) { + if (!this.activity || !this.tracingIntegration) { return; } - const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); - if (tracingIntegration !== null) { - // tslint:disable-next-line:no-unsafe-any - (tracingIntegration as any).constructor.popActivity(this.activity); - this.activity = null; - } + // tslint:disable-next-line:no-unsafe-any + (this.tracingIntegration as any).constructor.popActivity(this.activity); + this.activity = null; }; public render(): React.ReactNode { From 8bc23ce55f214434d1d94ee54361d43c02171fb5 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 10:02:07 -0400 Subject: [PATCH 03/18] ref: extract into popActivity --- packages/react/src/profiler.tsx | 47 ++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 2ac8f3e1f2e2..e92b8c41c306 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -59,18 +59,32 @@ function warnAboutTracing(name: string): void { } /** - * getInitActivity pushes activity based on React component mount + * pushActivity creates an new react activity * @param name displayName of component that started activity */ -const getInitActivity = (name: string): number | null => { - if (globalTracingIntegration !== null) { - // tslint:disable-next-line:no-unsafe-any - return (globalTracingIntegration as any).constructor.pushActivity(name, { - description: `<${name}>`, - op: 'react', - }); +const pushActivity = (name: string): number | null => { + if (globalTracingIntegration === null) { + return null; } - return null; + + // tslint:disable-next-line:no-unsafe-any + return (globalTracingIntegration as any).constructor.pushActivity(name, { + description: `<${name}>`, + op: 'react', + }); +}; + +/** + * popActivity removes a React activity if it exists + * @param activity id of activity that is being popped + */ +const popActivity = (activity: number | null): void => { + if (activity === null || globalTracingIntegration === null) { + return; + } + + // tslint:disable-next-line:no-unsafe-any + (globalTracingIntegration as any).constructor.popActivity(activity); }; export type ProfilerProps = { @@ -79,13 +93,13 @@ export type ProfilerProps = { class Profiler extends React.Component { public activity: number | null = null; - public tracingIntegration: Integration | null = getTracingIntegration(); public constructor(props: ProfilerProps) { super(props); - if (this.tracingIntegration) { - this.activity = getInitActivity(this.props.name); + // We should check for tracing integration per Profiler instance + if (getTracingIntegration()) { + this.activity = pushActivity(this.props.name); } else { warnAboutTracing(this.props.name); } @@ -103,12 +117,7 @@ class Profiler extends React.Component { } public finishProfile = () => { - if (!this.activity || !this.tracingIntegration) { - return; - } - - // tslint:disable-next-line:no-unsafe-any - (this.tracingIntegration as any).constructor.popActivity(this.activity); + popActivity(this.activity); this.activity = null; }; @@ -149,7 +158,7 @@ function withProfiler

(WrappedComponent: React.ComponentType

* @param name displayName of component being profiled */ function useProfiler(name: string): void { - const [activity] = React.useState(() => getInitActivity(name)); + const [activity] = React.useState(() => pushActivity(name)); React.useEffect(() => { afterNextFrame(() => { From dc0d9ba81e8ae3af0fd1453c1b33c0b849b64acd Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 10:20:19 -0400 Subject: [PATCH 04/18] ref: add disabled and extra activities: --- packages/react/src/profiler.tsx | 47 ++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index e92b8c41c306..91e674149fbc 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -1,5 +1,5 @@ import { getCurrentHub } from '@sentry/browser'; -import { Integration, IntegrationClass } from '@sentry/types'; +import { Integration, IntegrationClass, Span } from '@sentry/types'; import { logger } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; @@ -88,20 +88,41 @@ const popActivity = (activity: number | null): void => { }; export type ProfilerProps = { + // The name of the component being profiled. name: string; + // If the Profiler is disabled. False by default. + disabled?: boolean; }; +/** + * The Profiler component leverages Sentry's Tracing integration to generate + * spans based on component lifecycles. + */ class Profiler extends React.Component { public activity: number | null = null; + // The activity representing when a component was mounted onto a page. + public mountInfo: { + activity: number | null; + span: Span | null; + } = { + activity: null, + span: null, + }; + // The activity representing how long a component was on the page. + public visibleActivity: number | null = null; public constructor(props: ProfilerProps) { super(props); + const { name, disabled = false } = this.props; + + if (disabled) { + return; + } - // We should check for tracing integration per Profiler instance if (getTracingIntegration()) { - this.activity = pushActivity(this.props.name); + this.activity = pushActivity(name); } else { - warnAboutTracing(this.props.name); + warnAboutTracing(name); } } @@ -117,8 +138,10 @@ class Profiler extends React.Component { } public finishProfile = () => { - popActivity(this.activity); - this.activity = null; + afterNextFrame(() => { + popActivity(this.activity); + this.activity = null; + }); }; public render(): React.ReactNode { @@ -131,13 +154,17 @@ class Profiler extends React.Component { * component in a {@link Profiler} component. * * @param WrappedComponent component that is wrapped by Profiler - * @param name displayName of component being profiled + * @param options the {@link ProfilerProps} you can pass into the Profiler */ -function withProfiler

(WrappedComponent: React.ComponentType

, name?: string): React.FC

{ - const componentDisplayName = name || WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT; +function withProfiler

( + WrappedComponent: React.ComponentType

, + options?: Partial, +): React.FC

{ + const componentDisplayName = + (options && options.name) || WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT; const Wrapped: React.FC

= (props: P) => ( - + ); From 4eddf1a11f5ca39d6247437cb06ba5b6c23a7577 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 10:40:47 -0400 Subject: [PATCH 05/18] ref: Add visibleActivity: --- packages/react/src/profiler.tsx | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 91e674149fbc..105756ff3af9 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -62,16 +62,20 @@ function warnAboutTracing(name: string): void { * pushActivity creates an new react activity * @param name displayName of component that started activity */ -const pushActivity = (name: string): number | null => { +const pushActivity = (name: string, op: string, options?: Object): number | null => { if (globalTracingIntegration === null) { return null; } // tslint:disable-next-line:no-unsafe-any - return (globalTracingIntegration as any).constructor.pushActivity(name, { - description: `<${name}>`, - op: 'react', - }); + return (globalTracingIntegration as any).constructor.pushActivity( + name, + { + description: `<${name}>`, + op: `react.${op}`, + }, + options, + ); }; /** @@ -99,10 +103,10 @@ export type ProfilerProps = { * spans based on component lifecycles. */ class Profiler extends React.Component { - public activity: number | null = null; - // The activity representing when a component was mounted onto a page. public mountInfo: { + // The activity representing when a component was mounted onto a page. activity: number | null; + // The span from the mountInfo activity span: Span | null; } = { activity: null, @@ -120,7 +124,7 @@ class Profiler extends React.Component { } if (getTracingIntegration()) { - this.activity = pushActivity(name); + this.mountInfo.activity = pushActivity(name, 'mount'); } else { warnAboutTracing(name); } @@ -128,21 +132,23 @@ class Profiler extends React.Component { // If a component mounted, we can finish the mount activity. public componentDidMount(): void { - afterNextFrame(this.finishProfile); - } + afterNextFrame(() => { + popActivity(this.mountInfo.activity); + this.mountInfo.activity = null; - // Sometimes a component will unmount first, so we make - // sure to also finish the mount activity here. - public componentWillUnmount(): void { - afterNextFrame(this.finishProfile); + this.visibleActivity = pushActivity(this.props.name, 'visible'); + }); } - public finishProfile = () => { + // If a component doesn't mount, the visible activity will be end when the + public componentWillUnmount(): void { afterNextFrame(() => { - popActivity(this.activity); - this.activity = null; + popActivity(this.visibleActivity); + this.visibleActivity = null; }); - }; + } + + public finishProfile = () => {}; public render(): React.ReactNode { return this.props.children; From 75913f074b91e1e2239fc2f237906254b438cf49 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 13:04:40 -0400 Subject: [PATCH 06/18] ref: Rework rest of integration --- packages/apm/src/integrations/tracing.ts | 31 +++++- packages/react/src/profiler.tsx | 119 ++++++++++++++++------- 2 files changed, 112 insertions(+), 38 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index a6ce61fe4192..fc7029665c46 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -769,12 +769,14 @@ export class Tracing implements Integration { * @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. * @param options _autoPopAfter_ | Time in ms, if provided the activity will be popped automatically after this timeout. This can be helpful in cases where you cannot gurantee your application knows the state and calls `popActivity` for sure. + * @param options _parentSpanId_ | Set a custom parent span id for the activity's span. */ public static pushActivity( name: string, spanContext?: SpanContext, options?: { autoPopAfter?: number; + parentSpanId?: string; }, ): number { const activeTransaction = Tracing._activeTransaction; @@ -789,6 +791,9 @@ export class Tracing implements Integration { const hub = _getCurrentHub(); if (hub) { const span = activeTransaction.startChild(spanContext); + if (options && options.parentSpanId) { + span.parentSpanId = options.parentSpanId; + } Tracing._activities[Tracing._currentIndex] = { name, span, @@ -817,8 +822,12 @@ export class Tracing implements Integration { /** * Removes activity and finishes the span in case there is one + * @param id the id of the activity being removed + * @param spanData span data that can be updated + * @param finish if a span should be finished after the activity is removed + * */ - public static popActivity(id: number, spanData?: { [key: string]: any }): void { + public static popActivity(id: number, spanData?: { [key: string]: any }, finish: boolean = true): void { // The !id is on purpose to also fail with 0 // Since 0 is returned by push activity in case there is no active transaction if (!id) { @@ -845,7 +854,9 @@ export class Tracing implements Integration { if (Tracing.options && Tracing.options.debug && Tracing.options.debug.spanDebugTimingInfo) { Tracing._addSpanDebugInfo(span); } - span.finish(); + if (finish) { + span.finish(); + } } // tslint:disable-next-line: no-dynamic-delete delete Tracing._activities[id]; @@ -866,6 +877,22 @@ export class Tracing implements Integration { }, timeout); } } + + /** + * Get span based on activity id + */ + public static getActivitySpan(id: number): Span | undefined { + if (!id) { + return undefined; + } + if (Tracing._getCurrentHub) { + const hub = Tracing._getCurrentHub(); + if (hub) { + return Tracing._activities[id].span; + } + } + return undefined; + } } /** diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 105756ff3af9..4ca90c591c39 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -1,6 +1,6 @@ import { getCurrentHub } from '@sentry/browser'; -import { Integration, IntegrationClass, Span } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { Integration, IntegrationClass, Span, SpanContext } from '@sentry/types'; +import { logger, timestampWithMs } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; @@ -49,20 +49,31 @@ const getTracingIntegration = () => { return globalTracingIntegration; }; -/** JSDOC */ +/** + * Warn if tracing integration not configured. Will only warn once. + */ function warnAboutTracing(name: string): void { if (globalTracingIntegration === null) { logger.warn( - `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`, + `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure the Tracing integration is setup properly.`, ); } } /** - * pushActivity creates an new react activity + * pushActivity creates an new react activity. + * Is a no-op if Tracing integration is not valid * @param name displayName of component that started activity */ -const pushActivity = (name: string, op: string, options?: Object): number | null => { +function pushActivity( + name: string, + op: string, + context?: SpanContext, + options?: { + autoPopAfter?: number; + parentSpanId?: string; + }, +): number | null { if (globalTracingIntegration === null) { return null; } @@ -73,29 +84,43 @@ const pushActivity = (name: string, op: string, options?: Object): number | null { description: `<${name}>`, op: `react.${op}`, + ...context, }, options, ); -}; +} /** - * popActivity removes a React activity if it exists + * popActivity removes a React activity. + * Is a no-op if invalid Tracing integration or invalid activity id. * @param activity id of activity that is being popped + * @param finish if a span should be finished after the activity is removed */ -const popActivity = (activity: number | null): void => { +function popActivity(activity: number | null, finish: boolean = true): void { if (activity === null || globalTracingIntegration === null) { return; } // tslint:disable-next-line:no-unsafe-any - (globalTracingIntegration as any).constructor.popActivity(activity); -}; + (globalTracingIntegration as any).constructor.popActivity(activity, undefined, finish); +} + +function getActivitySpan(activity: number | null): Span | undefined { + if (globalTracingIntegration === null) { + return undefined; + } + + // tslint:disable-next-line:no-unsafe-any + return (globalTracingIntegration as any).constructor.getActivitySpan(activity) as Span | undefined; +} export type ProfilerProps = { // The name of the component being profiled. name: string; // If the Profiler is disabled. False by default. disabled?: boolean; + // If component updates should be displayed as spans. False by default. + generateUpdateSpans?: boolean; }; /** @@ -103,17 +128,17 @@ export type ProfilerProps = { * spans based on component lifecycles. */ class Profiler extends React.Component { - public mountInfo: { - // The activity representing when a component was mounted onto a page. - activity: number | null; - // The span from the mountInfo activity - span: Span | null; - } = { - activity: null, - span: null, - }; + // The activity representing how long it takes to mount a component. + public mountActivity: number | null = null; + // The spanId of the mount activity + public mountSpanId: string | null = null; // The activity representing how long a component was on the page. - public visibleActivity: number | null = null; + public renderActivity: number | null = null; + + public static defaultProps: Partial = { + disabled: false, + generateUpdateSpans: false, + }; public constructor(props: ProfilerProps) { super(props); @@ -124,7 +149,7 @@ class Profiler extends React.Component { } if (getTracingIntegration()) { - this.mountInfo.activity = pushActivity(name, 'mount'); + this.mountActivity = pushActivity(name, 'mount'); } else { warnAboutTracing(name); } @@ -133,23 +158,44 @@ class Profiler extends React.Component { // If a component mounted, we can finish the mount activity. public componentDidMount(): void { afterNextFrame(() => { - popActivity(this.mountInfo.activity); - this.mountInfo.activity = null; + const span = getActivitySpan(this.mountActivity); + if (span) { + this.mountSpanId = span.spanId; + } + popActivity(this.mountActivity); + this.mountActivity = null; - this.visibleActivity = pushActivity(this.props.name, 'visible'); + // If we were able to obtain the spanId of the mount activity, we should set the + // next activity as a child to the component mount activity. + const options = span ? { parentSpanId: span.spanId } : {}; + this.renderActivity = pushActivity(this.props.name, 'render', {}, options); }); } - // If a component doesn't mount, the visible activity will be end when the + public componentDidUpdate(prevProps: ProfilerProps): void { + if (prevProps.generateUpdateSpans && this.mountSpanId) { + const now = timestampWithMs(); + const updateActivity = pushActivity( + prevProps.name, + 'update', + { + endTimestamp: now, + startTimestamp: now, + }, + { parentSpanId: this.mountSpanId }, + ); + popActivity(updateActivity, false); + } + } + + // If a component doesn't mount, the render activity will be end when the public componentWillUnmount(): void { afterNextFrame(() => { - popActivity(this.visibleActivity); - this.visibleActivity = null; + popActivity(this.renderActivity, false); + this.renderActivity = null; }); } - public finishProfile = () => {}; - public render(): React.ReactNode { return this.props.children; } @@ -191,15 +237,16 @@ function withProfiler

( * @param name displayName of component being profiled */ function useProfiler(name: string): void { - const [activity] = React.useState(() => pushActivity(name)); + const [activity] = React.useState(() => pushActivity(name, 'mount')); React.useEffect(() => { afterNextFrame(() => { - const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); - if (tracingIntegration !== null) { - // tslint:disable-next-line:no-unsafe-any - (tracingIntegration as any).constructor.popActivity(activity); - } + popActivity(activity); + const renderActivity = pushActivity(name, 'render'); + + return () => { + popActivity(renderActivity); + }; }); }, []); } From 371a2473fdab0075553c581d03a877431af73934 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 14:59:24 -0400 Subject: [PATCH 07/18] ref: create children from span --- packages/apm/src/integrations/tracing.ts | 12 +++-- packages/react/src/profiler.tsx | 67 ++++++++++++------------ 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index fc7029665c46..f1f6d05bab3e 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 { @@ -827,7 +828,7 @@ export class Tracing implements Integration { * @param finish if a span should be finished after the activity is removed * */ - public static popActivity(id: number, spanData?: { [key: string]: any }, finish: boolean = true): void { + public static popActivity(id: number, spanData?: { [key: string]: any }): void { // The !id is on purpose to also fail with 0 // Since 0 is returned by push activity in case there is no active transaction if (!id) { @@ -854,9 +855,7 @@ export class Tracing implements Integration { if (Tracing.options && Tracing.options.debug && Tracing.options.debug.spanDebugTimingInfo) { Tracing._addSpanDebugInfo(span); } - if (finish) { - span.finish(); - } + span.finish(); } // tslint:disable-next-line: no-dynamic-delete delete Tracing._activities[id]; @@ -888,7 +887,10 @@ export class Tracing implements Integration { if (Tracing._getCurrentHub) { const hub = Tracing._getCurrentHub(); if (hub) { - return Tracing._activities[id].span; + const activity = Tracing._activities[id]; + if (activity) { + return activity.span; + } } } return undefined; diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 4ca90c591c39..a093da91e30f 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -1,5 +1,5 @@ import { getCurrentHub } from '@sentry/browser'; -import { Integration, IntegrationClass, Span, SpanContext } from '@sentry/types'; +import { Integration, IntegrationClass, Span } from '@sentry/types'; import { logger, timestampWithMs } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; @@ -68,7 +68,6 @@ function warnAboutTracing(name: string): void { function pushActivity( name: string, op: string, - context?: SpanContext, options?: { autoPopAfter?: number; parentSpanId?: string; @@ -84,7 +83,6 @@ function pushActivity( { description: `<${name}>`, op: `react.${op}`, - ...context, }, options, ); @@ -96,13 +94,13 @@ function pushActivity( * @param activity id of activity that is being popped * @param finish if a span should be finished after the activity is removed */ -function popActivity(activity: number | null, finish: boolean = true): void { +function popActivity(activity: number | null): void { if (activity === null || globalTracingIntegration === null) { return; } // tslint:disable-next-line:no-unsafe-any - (globalTracingIntegration as any).constructor.popActivity(activity, undefined, finish); + (globalTracingIntegration as any).constructor.popActivity(activity, undefined); } function getActivitySpan(activity: number | null): Span | undefined { @@ -130,14 +128,15 @@ export type ProfilerProps = { class Profiler extends React.Component { // The activity representing how long it takes to mount a component. public mountActivity: number | null = null; - // The spanId of the mount activity - public mountSpanId: string | null = null; + // The span of the mount activity + public span: Span | undefined = undefined; // The activity representing how long a component was on the page. public renderActivity: number | null = null; + public renderSpan: Span | undefined = undefined; public static defaultProps: Partial = { disabled: false, - generateUpdateSpans: false, + generateUpdateSpans: true, }; public constructor(props: ProfilerProps) { @@ -157,42 +156,42 @@ class Profiler extends React.Component { // If a component mounted, we can finish the mount activity. public componentDidMount(): void { - afterNextFrame(() => { - const span = getActivitySpan(this.mountActivity); - if (span) { - this.mountSpanId = span.spanId; - } - popActivity(this.mountActivity); - this.mountActivity = null; - - // If we were able to obtain the spanId of the mount activity, we should set the - // next activity as a child to the component mount activity. - const options = span ? { parentSpanId: span.spanId } : {}; - this.renderActivity = pushActivity(this.props.name, 'render', {}, options); - }); + // afterNextFrame(() => { + this.span = getActivitySpan(this.mountActivity); + popActivity(this.mountActivity); + this.mountActivity = null; + + // If we were able to obtain the spanId of the mount activity, we should set the + // next activity as a child to the component mount activity. + if (this.span) { + this.renderSpan = this.span.startChild({ + description: `<${this.props.name}>`, + op: `react.render`, + }); + } + // }); } public componentDidUpdate(prevProps: ProfilerProps): void { - if (prevProps.generateUpdateSpans && this.mountSpanId) { + if (prevProps.generateUpdateSpans && this.span && prevProps !== this.props) { const now = timestampWithMs(); - const updateActivity = pushActivity( - prevProps.name, - 'update', - { - endTimestamp: now, - startTimestamp: now, - }, - { parentSpanId: this.mountSpanId }, - ); - popActivity(updateActivity, false); + this.span.startChild({ + description: `<${prevProps.name}>`, + endTimestamp: now, + op: `react.update`, + startTimestamp: now, + }); } } // If a component doesn't mount, the render activity will be end when the public componentWillUnmount(): void { afterNextFrame(() => { - popActivity(this.renderActivity, false); - this.renderActivity = null; + if (this.renderSpan) { + this.renderSpan.finish(); + } + // popActivity(this.renderActivity); + // this.renderActivity = null; }); } From 33337c2650394e113508c6e5c9c600ee7db4584f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 15:19:58 -0400 Subject: [PATCH 08/18] ref: change update logic --- packages/react/src/profiler.tsx | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index a093da91e30f..084d74f4b338 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -119,6 +119,8 @@ export type ProfilerProps = { disabled?: boolean; // If component updates should be displayed as spans. False by default. generateUpdateSpans?: boolean; + // props from child component + updateProps: { [key: string]: any }; }; /** @@ -174,13 +176,18 @@ class Profiler extends React.Component { public componentDidUpdate(prevProps: ProfilerProps): void { if (prevProps.generateUpdateSpans && this.span && prevProps !== this.props) { - const now = timestampWithMs(); - this.span.startChild({ - description: `<${prevProps.name}>`, - endTimestamp: now, - op: `react.update`, - startTimestamp: now, - }); + const changedProps = Object.keys(prevProps).filter(k => prevProps.updateProps[k] !== this.props.updateProps[k]); + if (changedProps.length > 0) { + const now = timestampWithMs(); + const updateSpan = this.span.startChild({ + description: `<${prevProps.name}>`, + endTimestamp: now, + op: `react.update`, + startTimestamp: now, + }); + + updateSpan.setData('changedProps', changedProps); + } } } @@ -209,13 +216,13 @@ class Profiler extends React.Component { */ function withProfiler

( WrappedComponent: React.ComponentType

, - options?: Partial, + options?: Pick, Exclude>, ): React.FC

{ const componentDisplayName = (options && options.name) || WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT; const Wrapped: React.FC

= (props: P) => ( - + ); From 995da2d05f6296f000e424ad4153e04c3838a843 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 16:01:29 -0400 Subject: [PATCH 09/18] delete some code --- packages/react/src/profiler.tsx | 93 +++++++++++++-------------------- 1 file changed, 37 insertions(+), 56 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 084d74f4b338..03fecd7b0cc9 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -10,35 +10,6 @@ const TRACING_GETTER = ({ id: 'Tracing', } as any) as IntegrationClass; -/** - * - * Based on implementation from Preact: - * https:github.com/preactjs/preact/blob/9a422017fec6dab287c77c3aef63c7b2fef0c7e1/hooks/src/index.js#L301-L313 - * - * Schedule a callback to be invoked after the browser has a chance to paint a new frame. - * Do this by combining requestAnimationFrame (rAF) + setTimeout to invoke a callback after - * the next browser frame. - * - * Also, schedule a timeout in parallel to the the rAF to ensure the callback is invoked - * even if RAF doesn't fire (for example if the browser tab is not visible) - * - * This is what we use to tell if a component activity has finished - * - */ -function afterNextFrame(callback: Function): void { - let timeout: number | undefined; - let raf: number; - - const done = () => { - window.clearTimeout(timeout); - window.cancelAnimationFrame(raf); - window.setTimeout(callback); - }; - - raf = window.requestAnimationFrame(done); - timeout = window.setTimeout(done, 100); -} - let globalTracingIntegration: Integration | null = null; const getTracingIntegration = () => { if (globalTracingIntegration) { @@ -119,6 +90,8 @@ export type ProfilerProps = { disabled?: boolean; // If component updates should be displayed as spans. False by default. generateUpdateSpans?: boolean; + // If time component is on page should be displayed as spans. True by default. + generateRenderSpans?: boolean; // props from child component updateProps: { [key: string]: any }; }; @@ -131,14 +104,14 @@ class Profiler extends React.Component { // The activity representing how long it takes to mount a component. public mountActivity: number | null = null; // The span of the mount activity - public span: Span | undefined = undefined; - // The activity representing how long a component was on the page. - public renderActivity: number | null = null; + public mountSpan: Span | undefined = undefined; + // The span of the render public renderSpan: Span | undefined = undefined; public static defaultProps: Partial = { disabled: false, - generateUpdateSpans: true, + generateRenderSpans: true, + generateUpdateSpans: false, }; public constructor(props: ProfilerProps) { @@ -158,28 +131,26 @@ class Profiler extends React.Component { // If a component mounted, we can finish the mount activity. public componentDidMount(): void { - // afterNextFrame(() => { - this.span = getActivitySpan(this.mountActivity); + this.mountSpan = getActivitySpan(this.mountActivity); popActivity(this.mountActivity); this.mountActivity = null; // If we were able to obtain the spanId of the mount activity, we should set the // next activity as a child to the component mount activity. - if (this.span) { - this.renderSpan = this.span.startChild({ + if (this.mountSpan) { + this.renderSpan = this.mountSpan.startChild({ description: `<${this.props.name}>`, op: `react.render`, }); } - // }); } public componentDidUpdate(prevProps: ProfilerProps): void { - if (prevProps.generateUpdateSpans && this.span && prevProps !== this.props) { + if (prevProps.generateUpdateSpans && this.mountSpan && prevProps.updateProps !== this.props.updateProps) { const changedProps = Object.keys(prevProps).filter(k => prevProps.updateProps[k] !== this.props.updateProps[k]); if (changedProps.length > 0) { const now = timestampWithMs(); - const updateSpan = this.span.startChild({ + const updateSpan = this.mountSpan.startChild({ description: `<${prevProps.name}>`, endTimestamp: now, op: `react.update`, @@ -193,13 +164,9 @@ class Profiler extends React.Component { // If a component doesn't mount, the render activity will be end when the public componentWillUnmount(): void { - afterNextFrame(() => { - if (this.renderSpan) { - this.renderSpan.finish(); - } - // popActivity(this.renderActivity); - // this.renderActivity = null; - }); + if (this.renderSpan) { + this.renderSpan.finish(); + } } public render(): React.ReactNode { @@ -243,17 +210,31 @@ function withProfiler

( * @param name displayName of component being profiled */ function useProfiler(name: string): void { - const [activity] = React.useState(() => pushActivity(name, 'mount')); + const [mountActivity] = React.useState(() => { + if (getTracingIntegration()) { + return pushActivity(name, 'mount'); + } + + warnAboutTracing(name); + return null; + }); React.useEffect(() => { - afterNextFrame(() => { - popActivity(activity); - const renderActivity = pushActivity(name, 'render'); - - return () => { - popActivity(renderActivity); - }; - }); + const mountSpan = getActivitySpan(mountActivity); + popActivity(mountActivity); + + const renderSpan = mountSpan + ? mountSpan.startChild({ + description: `<${name}>`, + op: `react.render`, + }) + : undefined; + + return () => { + if (renderSpan) { + renderSpan.finish(); + } + }; }, []); } From be57d6f4bb922a1ef1598b91109e4ea651c003b4 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 17:22:15 -0400 Subject: [PATCH 10/18] generate spans properly --- packages/react/src/profiler.tsx | 46 +++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 03fecd7b0cc9..9313a549cfbe 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -88,10 +88,10 @@ export type ProfilerProps = { name: string; // If the Profiler is disabled. False by default. disabled?: boolean; - // If component updates should be displayed as spans. False by default. - generateUpdateSpans?: boolean; - // If time component is on page should be displayed as spans. True by default. - generateRenderSpans?: boolean; + // If time component is on page should be displayed as spans. False by default. + hasRenderSpan?: boolean; + // If component updates should be displayed as spans. True by default. + hasUpdateSpan?: boolean; // props from child component updateProps: { [key: string]: any }; }; @@ -110,8 +110,8 @@ class Profiler extends React.Component { public static defaultProps: Partial = { disabled: false, - generateRenderSpans: true, - generateUpdateSpans: false, + hasRenderSpan: false, + hasUpdateSpan: true, }; public constructor(props: ProfilerProps) { @@ -135,18 +135,21 @@ class Profiler extends React.Component { popActivity(this.mountActivity); this.mountActivity = null; + const { name, hasRenderSpan = false } = this.props; + // If we were able to obtain the spanId of the mount activity, we should set the // next activity as a child to the component mount activity. - if (this.mountSpan) { + if (this.mountSpan && hasRenderSpan) { this.renderSpan = this.mountSpan.startChild({ - description: `<${this.props.name}>`, + description: `<${name}>`, op: `react.render`, }); } } public componentDidUpdate(prevProps: ProfilerProps): void { - if (prevProps.generateUpdateSpans && this.mountSpan && prevProps.updateProps !== this.props.updateProps) { + const { hasUpdateSpan = true } = prevProps; + if (hasUpdateSpan && this.mountSpan && prevProps.updateProps !== this.props.updateProps) { const changedProps = Object.keys(prevProps).filter(k => prevProps.updateProps[k] !== this.props.updateProps[k]); if (changedProps.length > 0) { const now = timestampWithMs(); @@ -209,8 +212,18 @@ function withProfiler

( * Requires React 16.8 or above. * @param name displayName of component being profiled */ -function useProfiler(name: string): void { +function useProfiler( + name: string, + options?: { + disabled?: boolean; + hasRenderSpan?: boolean; + }, +): void { const [mountActivity] = React.useState(() => { + if (options && options.disabled) { + return null; + } + if (getTracingIntegration()) { return pushActivity(name, 'mount'); } @@ -223,12 +236,13 @@ function useProfiler(name: string): void { const mountSpan = getActivitySpan(mountActivity); popActivity(mountActivity); - const renderSpan = mountSpan - ? mountSpan.startChild({ - description: `<${name}>`, - op: `react.render`, - }) - : undefined; + const renderSpan = + mountSpan && options && options.hasRenderSpan + ? mountSpan.startChild({ + description: `<${name}>`, + op: `react.render`, + }) + : undefined; return () => { if (renderSpan) { From fc8b16617ad9acfde9dcb2fe6ae6897ee6caa8cb Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 16 Jun 2020 17:49:18 -0400 Subject: [PATCH 11/18] chore: update comments --- packages/react/src/profiler.tsx | 41 +++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 9313a549cfbe..fb910fe9569d 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -74,6 +74,11 @@ function popActivity(activity: number | null): void { (globalTracingIntegration as any).constructor.popActivity(activity, undefined); } +/** + * Obtain a span given an activity id. + * Is a no-op if invalid Tracing integration. + * @param activity activity id associated with obtained span + */ function getActivitySpan(activity: number | null): Span | undefined { if (globalTracingIntegration === null) { return undefined; @@ -86,13 +91,14 @@ function getActivitySpan(activity: number | null): Span | undefined { export type ProfilerProps = { // The name of the component being profiled. name: string; - // If the Profiler is disabled. False by default. + // If the Profiler is disabled. False by default. This is useful if you want to disable profilers + // in certain environments. disabled?: boolean; // If time component is on page should be displayed as spans. False by default. hasRenderSpan?: boolean; // If component updates should be displayed as spans. True by default. hasUpdateSpan?: boolean; - // props from child component + // props given to component being profiled. updateProps: { [key: string]: any }; }; @@ -147,25 +153,33 @@ class Profiler extends React.Component { } } - public componentDidUpdate(prevProps: ProfilerProps): void { - const { hasUpdateSpan = true } = prevProps; - if (hasUpdateSpan && this.mountSpan && prevProps.updateProps !== this.props.updateProps) { - const changedProps = Object.keys(prevProps).filter(k => prevProps.updateProps[k] !== this.props.updateProps[k]); + public componentDidUpdate({ updateProps, hasUpdateSpan = true }: ProfilerProps): void { + // Only generate an update span if hasUpdateSpan is true, if there is a valid mountSpan, + // and if the updateProps have changed. It is ok to not do a deep equality check here as it is expensive. + // We are just trying to give baseline clues for further investigation. + if (hasUpdateSpan && this.mountSpan && updateProps !== this.props.updateProps) { + // See what props haved changed between the previous props, and the current props. This is + // set as data on the span. We just store the prop keys as the values could be potenially very large. + const changedProps = Object.keys(updateProps).filter(k => updateProps[k] !== this.props.updateProps[k]); if (changedProps.length > 0) { + // The update span is a point in time span with 0 duration, just signifying that the component + // has been updated. const now = timestampWithMs(); - const updateSpan = this.mountSpan.startChild({ - description: `<${prevProps.name}>`, + this.mountSpan.startChild({ + data: { + changedProps, + }, + description: `<${this.props.name}>`, endTimestamp: now, op: `react.update`, startTimestamp: now, }); - - updateSpan.setData('changedProps', changedProps); } } } - // If a component doesn't mount, the render activity will be end when the + // If a component is unmounted, we can say it is no longer on the screen. + // This means we can finish the span representing the component render. public componentWillUnmount(): void { if (this.renderSpan) { this.renderSpan.finish(); @@ -179,13 +193,16 @@ class Profiler extends React.Component { /** * withProfiler is a higher order component that wraps a - * component in a {@link Profiler} component. + * component in a {@link Profiler} component. It is recommended that + * the higher order component be used over the regular {@link Profiler} component. * * @param WrappedComponent component that is wrapped by Profiler * @param options the {@link ProfilerProps} you can pass into the Profiler */ function withProfiler

( WrappedComponent: React.ComponentType

, + // We do not want to have `updateProps` given in options, it is instead filled through + // the HOC. options?: Pick, Exclude>, ): React.FC

{ const componentDisplayName = From ef2f3d7af6384642000bc0923ea4baa8d086ff6b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 09:29:37 -0400 Subject: [PATCH 12/18] test: Revamp for rewrite --- packages/react/src/profiler.tsx | 7 +- packages/react/test/profiler.test.tsx | 253 +++++++++++++++++--------- 2 files changed, 168 insertions(+), 92 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index fb910fe9569d..b10c5be4998b 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -71,7 +71,7 @@ function popActivity(activity: number | null): void { } // tslint:disable-next-line:no-unsafe-any - (globalTracingIntegration as any).constructor.popActivity(activity, undefined); + (globalTracingIntegration as any).constructor.popActivity(activity); } /** @@ -201,15 +201,14 @@ class Profiler extends React.Component { */ function withProfiler

( WrappedComponent: React.ComponentType

, - // We do not want to have `updateProps` given in options, it is instead filled through - // the HOC. + // We do not want to have `updateProps` given in options, it is instead filled through the HOC. options?: Pick, Exclude>, ): React.FC

{ const componentDisplayName = (options && options.name) || WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT; const Wrapped: React.FC

= (props: P) => ( - + ); diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx index 84a6f3e546b5..5b9ddf250ac6 100644 --- a/packages/react/test/profiler.test.tsx +++ b/packages/react/test/profiler.test.tsx @@ -1,14 +1,28 @@ +import { SpanContext } from '@sentry/types'; import { render } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; import * as React from 'react'; import { UNKNOWN_COMPONENT, useProfiler, withProfiler } from '../src/profiler'; - +/*( +for key in SENTRY_FEATURES: + SENTRY_FEATURES[key] = True + +SENTRY_APM_SAMPLING = 1 +)*/ +const TEST_SPAN_ID = '518999beeceb49af'; + +const mockSpanFinish = jest.fn(); +const mockStartChild = jest.fn((spanArgs: SpanContext) => ({ ...spanArgs, finish: mockSpanFinish })); +const TEST_SPAN = { + spanId: TEST_SPAN_ID, + startChild: mockStartChild, +}; +const TEST_TIMESTAMP = '123456'; const mockPushActivity = jest.fn().mockReturnValue(1); const mockPopActivity = jest.fn(); const mockLoggerWarn = jest.fn(); - -let integrationIsNull = false; +const mockGetActivitySpan = jest.fn().mockReturnValue(TEST_SPAN); jest.mock('@sentry/utils', () => ({ logger: { @@ -16,6 +30,7 @@ jest.mock('@sentry/utils', () => ({ mockLoggerWarn(message); }, }, + timestampWithMs: () => TEST_TIMESTAMP, })); jest.mock('@sentry/browser', () => ({ @@ -29,26 +44,23 @@ jest.mock('@sentry/browser', () => ({ public setupOnce: () => void = jest.fn(); public static pushActivity: () => void = mockPushActivity; public static popActivity: () => void = mockPopActivity; + public static getActivitySpan: () => void = mockGetActivitySpan; } - - if (!integrationIsNull) { - return new MockIntegration('test'); - } - - return null; + return new MockIntegration('test'); }, }), })); -describe('withProfiler', () => { - beforeEach(() => { - jest.useFakeTimers(); - mockPushActivity.mockClear(); - mockPopActivity.mockClear(); - mockLoggerWarn.mockClear(); - integrationIsNull = false; - }); +beforeEach(() => { + mockPushActivity.mockClear(); + mockPopActivity.mockClear(); + mockLoggerWarn.mockClear(); + mockGetActivitySpan.mockClear(); + mockStartChild.mockClear(); + mockSpanFinish.mockClear(); +}); +describe('withProfiler', () => { it('sets displayName properly', () => { const TestComponent = () =>

Hello World

; @@ -59,7 +71,7 @@ describe('withProfiler', () => { it('sets a custom displayName', () => { const TestComponent = () =>

Hello World

; - const ProfiledComponent = withProfiler(TestComponent, 'BestComponent'); + const ProfiledComponent = withProfiler(TestComponent, { name: 'BestComponent' }); expect(ProfiledComponent.displayName).toBe('profiler(BestComponent)'); }); @@ -68,95 +80,160 @@ describe('withProfiler', () => { expect(ProfiledComponent.displayName).toBe(`profiler(${UNKNOWN_COMPONENT})`); }); - it('popActivity() is called when unmounted', () => { - const ProfiledComponent = withProfiler(() =>

Hello World

); - - expect(mockPopActivity).toHaveBeenCalledTimes(0); - const profiler = render(); - profiler.unmount(); - - jest.runAllTimers(); + describe('mount span', () => { + it('does not get created if Profiler is disabled', () => { + const ProfiledComponent = withProfiler(() =>

Testing

, { disabled: true }); + expect(mockPushActivity).toHaveBeenCalledTimes(0); + render(); + expect(mockPushActivity).toHaveBeenCalledTimes(0); + }); - expect(mockPopActivity).toHaveBeenCalledTimes(1); - expect(mockPopActivity).toHaveBeenLastCalledWith(1); + it('is created when a component is mounted', () => { + const ProfiledComponent = withProfiler(() =>

Testing

); + + expect(mockPushActivity).toHaveBeenCalledTimes(0); + expect(mockGetActivitySpan).toHaveBeenCalledTimes(0); + expect(mockPopActivity).toHaveBeenCalledTimes(0); + + render(); + + expect(mockPushActivity).toHaveBeenCalledTimes(1); + expect(mockPushActivity).toHaveBeenLastCalledWith( + UNKNOWN_COMPONENT, + { + description: `<${UNKNOWN_COMPONENT}>`, + op: 'react.mount', + }, + undefined, + ); + expect(mockGetActivitySpan).toHaveBeenCalledTimes(1); + expect(mockGetActivitySpan).toHaveBeenLastCalledWith(1); + + expect(mockPopActivity).toHaveBeenCalledTimes(1); + expect(mockPopActivity).toHaveBeenLastCalledWith(1); + }); }); - it('pushActivity() is called when mounted', () => { - const ProfiledComponent = withProfiler(() =>

Testing

); + describe('render span', () => { + it('does not get created by default', () => { + const ProfiledComponent = withProfiler(() =>

Testing

); + expect(mockStartChild).toHaveBeenCalledTimes(0); + render(); + expect(mockStartChild).toHaveBeenCalledTimes(0); + }); - expect(mockPushActivity).toHaveBeenCalledTimes(0); - render(); - expect(mockPushActivity).toHaveBeenCalledTimes(1); - expect(mockPushActivity).toHaveBeenLastCalledWith(UNKNOWN_COMPONENT, { - description: `<${UNKNOWN_COMPONENT}>`, - op: 'react', + it('is created when given hasRenderSpan option', () => { + const ProfiledComponent = withProfiler(() =>

Testing

, { hasRenderSpan: true }); + expect(mockStartChild).toHaveBeenCalledTimes(0); + const component = render(); + expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenLastCalledWith({ + description: `<${UNKNOWN_COMPONENT}>`, + op: 'react.render', + }); + + expect(mockSpanFinish).toHaveBeenCalledTimes(0); + component.unmount(); + expect(mockSpanFinish).toHaveBeenCalledTimes(1); }); }); - it('does not start an activity when integration is disabled', () => { - integrationIsNull = true; - const ProfiledComponent = withProfiler(() =>

Hello World

); - - expect(mockPushActivity).toHaveBeenCalledTimes(0); - expect(mockLoggerWarn).toHaveBeenCalledTimes(0); - - const profiler = render(); - expect(mockPopActivity).toHaveBeenCalledTimes(0); - expect(mockPushActivity).toHaveBeenCalledTimes(0); + describe('update span', () => { + it('is created when component is updated', () => { + const ProfiledComponent = withProfiler((props: { num: number }) =>
{props.num}
); + const { rerender } = render(); + expect(mockStartChild).toHaveBeenCalledTimes(0); + + // Dispatch new props + rerender(); + expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenLastCalledWith({ + data: { changedProps: ['num'] }, + description: `<${UNKNOWN_COMPONENT}>`, + endTimestamp: TEST_TIMESTAMP, + op: 'react.update', + startTimestamp: TEST_TIMESTAMP, + }); + + // New props yet again + rerender(); + expect(mockStartChild).toHaveBeenCalledTimes(2); + expect(mockStartChild).toHaveBeenLastCalledWith({ + data: { changedProps: ['num'] }, + description: `<${UNKNOWN_COMPONENT}>`, + endTimestamp: TEST_TIMESTAMP, + op: 'react.update', + startTimestamp: TEST_TIMESTAMP, + }); + + // Should not create spans if props haven't changed + rerender(); + expect(mockStartChild).toHaveBeenCalledTimes(2); + }); - expect(mockLoggerWarn).toHaveBeenCalledTimes(1); + it('does not get created if hasUpdateSpan is false', () => { + const ProfiledComponent = withProfiler((props: { num: number }) =>
{props.num}
, { + hasUpdateSpan: false, + }); + const { rerender } = render(); + expect(mockStartChild).toHaveBeenCalledTimes(0); - profiler.unmount(); - expect(mockPopActivity).toHaveBeenCalledTimes(0); + // Dispatch new props + rerender(); + expect(mockStartChild).toHaveBeenCalledTimes(0); + }); }); }); describe('useProfiler()', () => { - beforeEach(() => { - jest.useFakeTimers(); - mockPushActivity.mockClear(); - mockPopActivity.mockClear(); - mockLoggerWarn.mockClear(); - integrationIsNull = false; - }); - - it('popActivity() is called when unmounted', () => { - // tslint:disable-next-line: no-void-expression - const profiler = renderHook(() => useProfiler('Example')); - expect(mockPopActivity).toHaveBeenCalledTimes(0); - profiler.unmount(); - - jest.runAllTimers(); - - expect(mockPopActivity).toHaveBeenCalled(); - expect(mockPopActivity).toHaveBeenLastCalledWith(1); - }); + describe('mount span', () => { + it('does not get created if Profiler is disabled', () => { + // tslint:disable-next-line: no-void-expression + renderHook(() => useProfiler('Example', { disabled: true })); + expect(mockPushActivity).toHaveBeenCalledTimes(0); + }); - it('pushActivity() is called when mounted', () => { - expect(mockPushActivity).toHaveBeenCalledTimes(0); - // tslint:disable-next-line: no-void-expression - const profiler = renderHook(() => useProfiler('Example')); - profiler.unmount(); - expect(mockPushActivity).toHaveBeenCalledTimes(1); - expect(mockPushActivity).toHaveBeenLastCalledWith('Example', { - description: ``, - op: 'react', + it('is created when a component is mounted', () => { + // tslint:disable-next-line: no-void-expression + renderHook(() => useProfiler('Example')); + + expect(mockPushActivity).toHaveBeenCalledTimes(1); + expect(mockPushActivity).toHaveBeenLastCalledWith( + 'Example', + { + description: '', + op: 'react.mount', + }, + undefined, + ); + expect(mockGetActivitySpan).toHaveBeenCalledTimes(1); + expect(mockGetActivitySpan).toHaveBeenLastCalledWith(1); + + expect(mockPopActivity).toHaveBeenCalledTimes(1); + expect(mockPopActivity).toHaveBeenLastCalledWith(1); }); }); - it('does not start an activity when integration is disabled', () => { - integrationIsNull = true; - expect(mockPushActivity).toHaveBeenCalledTimes(0); - expect(mockLoggerWarn).toHaveBeenCalledTimes(0); + describe('render span', () => { + it('does not get created by default', () => { + // tslint:disable-next-line: no-void-expression + renderHook(() => useProfiler('Example')); + expect(mockStartChild).toHaveBeenCalledTimes(0); + }); - // tslint:disable-next-line: no-void-expression - const profiler = renderHook(() => useProfiler('Example')); - expect(mockPopActivity).toHaveBeenCalledTimes(0); - expect(mockPushActivity).toHaveBeenCalledTimes(0); + it('is created when given hasRenderSpan option', () => { + // tslint:disable-next-line: no-void-expression + const component = renderHook(() => useProfiler('Example', { hasRenderSpan: true })); - expect(mockLoggerWarn).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenLastCalledWith({ + description: '', + op: 'react.render', + }); - profiler.unmount(); - expect(mockPopActivity).toHaveBeenCalledTimes(0); + expect(mockSpanFinish).toHaveBeenCalledTimes(0); + component.unmount(); + expect(mockSpanFinish).toHaveBeenCalledTimes(1); + }); }); }); From 3c5a8d64a06d7e8e4f82043f27fcf8e78b8bc177 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 09:35:27 -0400 Subject: [PATCH 13/18] ref: Remove parentSpanId option --- packages/apm/src/integrations/tracing.ts | 7 +------ packages/react/src/profiler.tsx | 21 +++++---------------- packages/react/test/profiler.test.tsx | 24 ++++++++---------------- 3 files changed, 14 insertions(+), 38 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index f1f6d05bab3e..2f2124789bfb 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -770,15 +770,13 @@ export class Tracing implements Integration { * @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. * @param options _autoPopAfter_ | Time in ms, if provided the activity will be popped automatically after this timeout. This can be helpful in cases where you cannot gurantee your application knows the state and calls `popActivity` for sure. - * @param options _parentSpanId_ | Set a custom parent span id for the activity's span. */ public static pushActivity( name: string, spanContext?: SpanContext, options?: { autoPopAfter?: number; - parentSpanId?: string; - }, + },∂ ): number { const activeTransaction = Tracing._activeTransaction; @@ -792,9 +790,6 @@ export class Tracing implements Integration { const hub = _getCurrentHub(); if (hub) { const span = activeTransaction.startChild(spanContext); - if (options && options.parentSpanId) { - span.parentSpanId = options.parentSpanId; - } Tracing._activities[Tracing._currentIndex] = { name, span, diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index b10c5be4998b..3d24e11bb5d6 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -36,27 +36,16 @@ function warnAboutTracing(name: string): void { * Is a no-op if Tracing integration is not valid * @param name displayName of component that started activity */ -function pushActivity( - name: string, - op: string, - options?: { - autoPopAfter?: number; - parentSpanId?: string; - }, -): number | null { +function pushActivity(name: string, op: string): number | null { if (globalTracingIntegration === null) { return null; } // tslint:disable-next-line:no-unsafe-any - return (globalTracingIntegration as any).constructor.pushActivity( - name, - { - description: `<${name}>`, - op: `react.${op}`, - }, - options, - ); + return (globalTracingIntegration as any).constructor.pushActivity(name, { + description: `<${name}>`, + op: `react.${op}`, + }); } /** diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx index 5b9ddf250ac6..8e575df2421e 100644 --- a/packages/react/test/profiler.test.tsx +++ b/packages/react/test/profiler.test.tsx @@ -98,14 +98,10 @@ describe('withProfiler', () => { render(); expect(mockPushActivity).toHaveBeenCalledTimes(1); - expect(mockPushActivity).toHaveBeenLastCalledWith( - UNKNOWN_COMPONENT, - { - description: `<${UNKNOWN_COMPONENT}>`, - op: 'react.mount', - }, - undefined, - ); + expect(mockPushActivity).toHaveBeenLastCalledWith(UNKNOWN_COMPONENT, { + description: `<${UNKNOWN_COMPONENT}>`, + op: 'react.mount', + }); expect(mockGetActivitySpan).toHaveBeenCalledTimes(1); expect(mockGetActivitySpan).toHaveBeenLastCalledWith(1); @@ -198,14 +194,10 @@ describe('useProfiler()', () => { renderHook(() => useProfiler('Example')); expect(mockPushActivity).toHaveBeenCalledTimes(1); - expect(mockPushActivity).toHaveBeenLastCalledWith( - 'Example', - { - description: '', - op: 'react.mount', - }, - undefined, - ); + expect(mockPushActivity).toHaveBeenLastCalledWith('Example', { + description: '', + op: 'react.mount', + }); expect(mockGetActivitySpan).toHaveBeenCalledTimes(1); expect(mockGetActivitySpan).toHaveBeenLastCalledWith(1); From cb2ce415bb794ddd42527144e921df54daf1c5ca Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 09:49:53 -0400 Subject: [PATCH 14/18] Rework render span --- packages/apm/src/integrations/tracing.ts | 2 +- packages/react/src/profiler.tsx | 51 +++++++++----------- packages/react/test/profiler.test.tsx | 60 +++++++++++++----------- 3 files changed, 56 insertions(+), 57 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index 2f2124789bfb..dfd680a65995 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -776,7 +776,7 @@ export class Tracing implements Integration { spanContext?: SpanContext, options?: { autoPopAfter?: number; - },∂ + }, ): number { const activeTransaction = Tracing._activeTransaction; diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 3d24e11bb5d6..fa7649af71c4 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -83,7 +83,7 @@ export type ProfilerProps = { // If the Profiler is disabled. False by default. This is useful if you want to disable profilers // in certain environments. disabled?: boolean; - // If time component is on page should be displayed as spans. False by default. + // If time component is on page should be displayed as spans. True by default. hasRenderSpan?: boolean; // If component updates should be displayed as spans. True by default. hasUpdateSpan?: boolean; @@ -105,7 +105,7 @@ class Profiler extends React.Component { public static defaultProps: Partial = { disabled: false, - hasRenderSpan: false, + hasRenderSpan: true, hasUpdateSpan: true, }; @@ -129,17 +129,6 @@ class Profiler extends React.Component { this.mountSpan = getActivitySpan(this.mountActivity); popActivity(this.mountActivity); this.mountActivity = null; - - const { name, hasRenderSpan = false } = this.props; - - // If we were able to obtain the spanId of the mount activity, we should set the - // next activity as a child to the component mount activity. - if (this.mountSpan && hasRenderSpan) { - this.renderSpan = this.mountSpan.startChild({ - description: `<${name}>`, - op: `react.render`, - }); - } } public componentDidUpdate({ updateProps, hasUpdateSpan = true }: ProfilerProps): void { @@ -170,8 +159,17 @@ class Profiler extends React.Component { // If a component is unmounted, we can say it is no longer on the screen. // This means we can finish the span representing the component render. public componentWillUnmount(): void { - if (this.renderSpan) { - this.renderSpan.finish(); + const { name, hasRenderSpan = true } = this.props; + + if (this.mountSpan && hasRenderSpan) { + // If we were able to obtain the spanId of the mount activity, we should set the + // next activity as a child to the component mount activity. + this.mountSpan.startChild({ + description: `<${name}>`, + endTimestamp: timestampWithMs(), + op: `react.render`, + startTimestamp: this.mountSpan.endTimestamp, + }); } } @@ -219,9 +217,9 @@ function withProfiler

( */ function useProfiler( name: string, - options?: { - disabled?: boolean; - hasRenderSpan?: boolean; + options: { disabled?: boolean; hasRenderSpan?: boolean } = { + disabled: false, + hasRenderSpan: true, }, ): void { const [mountActivity] = React.useState(() => { @@ -241,17 +239,14 @@ function useProfiler( const mountSpan = getActivitySpan(mountActivity); popActivity(mountActivity); - const renderSpan = - mountSpan && options && options.hasRenderSpan - ? mountSpan.startChild({ - description: `<${name}>`, - op: `react.render`, - }) - : undefined; - return () => { - if (renderSpan) { - renderSpan.finish(); + if (mountSpan && options.hasRenderSpan) { + mountSpan.startChild({ + description: `<${name}>`, + endTimestamp: timestampWithMs(), + op: `react.render`, + startTimestamp: mountSpan.endTimestamp, + }); } }; }, []); diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx index 8e575df2421e..cd65d68fbd0b 100644 --- a/packages/react/test/profiler.test.tsx +++ b/packages/react/test/profiler.test.tsx @@ -12,8 +12,7 @@ SENTRY_APM_SAMPLING = 1 )*/ const TEST_SPAN_ID = '518999beeceb49af'; -const mockSpanFinish = jest.fn(); -const mockStartChild = jest.fn((spanArgs: SpanContext) => ({ ...spanArgs, finish: mockSpanFinish })); +const mockStartChild = jest.fn((spanArgs: SpanContext) => ({ ...spanArgs })); const TEST_SPAN = { spanId: TEST_SPAN_ID, startChild: mockStartChild, @@ -57,7 +56,6 @@ beforeEach(() => { mockLoggerWarn.mockClear(); mockGetActivitySpan.mockClear(); mockStartChild.mockClear(); - mockSpanFinish.mockClear(); }); describe('withProfiler', () => { @@ -111,26 +109,30 @@ describe('withProfiler', () => { }); describe('render span', () => { - it('does not get created by default', () => { + it('is created on unmount', () => { const ProfiledComponent = withProfiler(() =>

Testing

); expect(mockStartChild).toHaveBeenCalledTimes(0); - render(); - expect(mockStartChild).toHaveBeenCalledTimes(0); - }); - it('is created when given hasRenderSpan option', () => { - const ProfiledComponent = withProfiler(() =>

Testing

, { hasRenderSpan: true }); - expect(mockStartChild).toHaveBeenCalledTimes(0); const component = render(); + component.unmount(); + expect(mockStartChild).toHaveBeenCalledTimes(1); - expect(mockStartChild).toHaveBeenLastCalledWith({ - description: `<${UNKNOWN_COMPONENT}>`, - op: 'react.render', - }); + expect(mockStartChild).toHaveBeenLastCalledWith( + expect.objectContaining({ + description: `<${UNKNOWN_COMPONENT}>`, + op: 'react.render', + }), + ); + }); - expect(mockSpanFinish).toHaveBeenCalledTimes(0); + it('is not created if hasRenderSpan is false', () => { + const ProfiledComponent = withProfiler(() =>

Testing

, { hasRenderSpan: false }); + expect(mockStartChild).toHaveBeenCalledTimes(0); + + const component = render(); component.unmount(); - expect(mockSpanFinish).toHaveBeenCalledTimes(1); + + expect(mockStartChild).toHaveBeenCalledTimes(0); }); }); @@ -207,25 +209,27 @@ describe('useProfiler()', () => { }); describe('render span', () => { - it('does not get created by default', () => { + it('does not get created when hasRenderSpan is false', () => { // tslint:disable-next-line: no-void-expression - renderHook(() => useProfiler('Example')); + const component = renderHook(() => useProfiler('Example', { hasRenderSpan: false })); + expect(mockStartChild).toHaveBeenCalledTimes(0); + component.unmount(); expect(mockStartChild).toHaveBeenCalledTimes(0); }); - it('is created when given hasRenderSpan option', () => { + it('is created by default', () => { // tslint:disable-next-line: no-void-expression - const component = renderHook(() => useProfiler('Example', { hasRenderSpan: true })); - - expect(mockStartChild).toHaveBeenCalledTimes(1); - expect(mockStartChild).toHaveBeenLastCalledWith({ - description: '', - op: 'react.render', - }); + const component = renderHook(() => useProfiler('Example')); - expect(mockSpanFinish).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(0); component.unmount(); - expect(mockSpanFinish).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenLastCalledWith( + expect.objectContaining({ + description: '', + op: 'react.render', + }), + ); }); }); }); From b43352ef3399ee80a1a2b7d37188e0214b28ff0c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 09:52:01 -0400 Subject: [PATCH 15/18] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb29698e4da4..e939c29e8cd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - [core] fix: Call `bindClient` when creating new `Hub` to make integrations work automatically (#2665) - [gatsby] feat: Add @sentry/gatsby package (#2652) - [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls` +- [react] ref: Refactor Profiler to account for update and render (#2677) ## 5.17.0 From fe9061248f5da5d4c03dcf56699f389923fe9d13 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 09:57:14 -0400 Subject: [PATCH 16/18] ref: Change getActivitySpan to not check hub --- CHANGELOG.md | 1 + packages/apm/src/integrations/tracing.ts | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e939c29e8cd6..28a3aa98f59a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - [gatsby] feat: Add @sentry/gatsby package (#2652) - [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls` - [react] ref: Refactor Profiler to account for update and render (#2677) +- [tracing] feat: Add ability to get span from activity using `getActivitySpan` (#2677) ## 5.17.0 diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index dfd680a65995..a1267b306ec2 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -879,14 +879,9 @@ export class Tracing implements Integration { if (!id) { return undefined; } - if (Tracing._getCurrentHub) { - const hub = Tracing._getCurrentHub(); - if (hub) { - const activity = Tracing._activities[id]; - if (activity) { - return activity.span; - } - } + const activity = Tracing._activities[id]; + if (activity) { + return activity.span; } return undefined; } From 4e7eed58d8561c4f828e6e44de2ab020edc9fbc0 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 09:58:52 -0400 Subject: [PATCH 17/18] remove params --- packages/apm/src/integrations/tracing.ts | 1 - packages/react/src/profiler.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index a1267b306ec2..d97bb626d26f 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -820,7 +820,6 @@ export class Tracing implements Integration { * Removes activity and finishes the span in case there is one * @param id the id of the activity being removed * @param spanData span data that can be updated - * @param finish if a span should be finished after the activity is removed * */ public static popActivity(id: number, spanData?: { [key: string]: any }): void { diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index fa7649af71c4..5fab8b4c3cd3 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -52,7 +52,6 @@ function pushActivity(name: string, op: string): number | null { * popActivity removes a React activity. * Is a no-op if invalid Tracing integration or invalid activity id. * @param activity id of activity that is being popped - * @param finish if a span should be finished after the activity is removed */ function popActivity(activity: number | null): void { if (activity === null || globalTracingIntegration === null) { From 2743af0da22abf9cb84c8653cc5c630d93e67a06 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 17 Jun 2020 10:12:28 -0400 Subject: [PATCH 18/18] cleanup --- packages/react/src/profiler.tsx | 6 +++--- packages/react/test/profiler.test.tsx | 16 +++++----------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 5fab8b4c3cd3..8ef7d6966f63 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -50,7 +50,7 @@ function pushActivity(name: string, op: string): number | null { /** * popActivity removes a React activity. - * Is a no-op if invalid Tracing integration or invalid activity id. + * Is a no-op if Tracing integration is not valid. * @param activity id of activity that is being popped */ function popActivity(activity: number | null): void { @@ -64,11 +64,11 @@ function popActivity(activity: number | null): void { /** * Obtain a span given an activity id. - * Is a no-op if invalid Tracing integration. + * Is a no-op if Tracing integration is not valid. * @param activity activity id associated with obtained span */ function getActivitySpan(activity: number | null): Span | undefined { - if (globalTracingIntegration === null) { + if (activity === null || globalTracingIntegration === null) { return undefined; } diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx index cd65d68fbd0b..0da9d9545099 100644 --- a/packages/react/test/profiler.test.tsx +++ b/packages/react/test/profiler.test.tsx @@ -4,24 +4,18 @@ import { renderHook } from '@testing-library/react-hooks'; import * as React from 'react'; import { UNKNOWN_COMPONENT, useProfiler, withProfiler } from '../src/profiler'; -/*( -for key in SENTRY_FEATURES: - SENTRY_FEATURES[key] = True -SENTRY_APM_SAMPLING = 1 -)*/ const TEST_SPAN_ID = '518999beeceb49af'; +const TEST_TIMESTAMP = '123456'; const mockStartChild = jest.fn((spanArgs: SpanContext) => ({ ...spanArgs })); -const TEST_SPAN = { - spanId: TEST_SPAN_ID, - startChild: mockStartChild, -}; -const TEST_TIMESTAMP = '123456'; const mockPushActivity = jest.fn().mockReturnValue(1); const mockPopActivity = jest.fn(); const mockLoggerWarn = jest.fn(); -const mockGetActivitySpan = jest.fn().mockReturnValue(TEST_SPAN); +const mockGetActivitySpan = jest.fn().mockReturnValue({ + spanId: TEST_SPAN_ID, + startChild: mockStartChild, +}); jest.mock('@sentry/utils', () => ({ logger: {