Skip to content

Commit c936899

Browse files
AbhiPrasadAbhiPrasad
committed
Add canBeCancelled
1 parent c2881ac commit c936899

File tree

3 files changed

+145
-111
lines changed

3 files changed

+145
-111
lines changed

‎packages/apm/src/integrations/tracing.ts

+20-25
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ export interface TracingOptions {
113113
interface Activity {
114114
name: string;
115115
span?: Span;
116+
canBeCancelled?: boolean;
116117
}
117118

118119
const global = getGlobalObject<Window>();
119120
const defaultTracingOrigins = ['localhost', /^\//];
120-
const SPAN_IGNORE_KEY = '__sentry_delete_span';
121121

122122
/**
123123
* Tracing Integration
@@ -476,11 +476,6 @@ export class Tracing implements Integration {
476476
return span;
477477
}
478478

479-
// if a span is supposed to be ignored, don't add it to the transaction
480-
if (span.data[SPAN_IGNORE_KEY]) {
481-
return false;
482-
}
483-
484479
// We cancel all pending spans with status "cancelled" to indicate the idle transaction was finished early
485480
if (!span.endTimestamp) {
486481
span.endTimestamp = endTimestamp;
@@ -776,12 +771,15 @@ export class Tracing implements Integration {
776771
* @param name Name of the activity, can be any string (Only used internally to identify the activity)
777772
* @param spanContext If provided a Span with the SpanContext will be created.
778773
* @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.
774+
* @param options _parentSpanId_ | Set a custom parent span id for the activity's span.
779775
*/
780776
public static pushActivity(
781777
name: string,
782778
spanContext?: SpanContext,
783779
options?: {
784780
autoPopAfter?: number;
781+
parentSpanId?: string;
782+
canBeCancelled?: boolean;
785783
},
786784
): number {
787785
const activeTransaction = Tracing._activeTransaction;
@@ -796,7 +794,11 @@ export class Tracing implements Integration {
796794
const hub = _getCurrentHub();
797795
if (hub) {
798796
const span = activeTransaction.startChild(spanContext);
797+
if (options && options.parentSpanId) {
798+
span.parentSpanId = options.parentSpanId;
799+
}
799800
Tracing._activities[Tracing._currentIndex] = {
801+
canBeCancelled: options && options.canBeCancelled,
800802
name,
801803
span,
802804
};
@@ -858,11 +860,12 @@ export class Tracing implements Integration {
858860
delete Tracing._activities[id];
859861
}
860862

861-
const count = Object.keys(Tracing._activities).length;
863+
const activities = Object.values(Tracing._activities);
864+
const cancelledActivities = activities.filter(({ canBeCancelled }) => !!canBeCancelled);
862865

863-
Tracing._log('[Tracing] activies count', count);
866+
Tracing._log('[Tracing] activies count', activities.length);
864867

865-
if (count === 0 && Tracing._activeTransaction) {
868+
if ((activities.length === 0 || activities.length === cancelledActivities.length) && Tracing._activeTransaction) {
866869
const timeout = Tracing.options && Tracing.options.idleTimeout;
867870
Tracing._log(`[Tracing] Flushing Transaction in ${timeout}ms`);
868871
// We need to add the timeout here to have the real endtimestamp of the transaction
@@ -875,28 +878,20 @@ export class Tracing implements Integration {
875878
}
876879

877880
/**
878-
* Cancels an activity if it exists
881+
* Get span based on activity id
879882
*/
880-
public static cancelActivity(id: number): void {
883+
public static getActivitySpan(id: number): Span | undefined {
881884
if (!id) {
882-
return;
885+
return undefined;
883886
}
887+
if (Tracing._getCurrentHub) {
888+
const hub = Tracing._getCurrentHub();
884889

885-
const activity = Tracing._activities[id];
886-
887-
if (activity) {
888-
Tracing._log(`[Tracing] cancelActivity ${activity.name}#${id}`);
889-
if (activity.span) {
890-
// Ignore the span in the transaction
891-
activity.span.setData(SPAN_IGNORE_KEY, true);
890+
if (hub) {
891+
return Tracing._activities[id].span;
892892
}
893-
894-
// tslint:disable-next-line: no-dynamic-delete
895-
delete Tracing._activities[id];
896-
897-
const count = Object.keys(Tracing._activities).length;
898-
Tracing._log('[Tracing] activies count', count);
899893
}
894+
return undefined;
900895
}
901896
}
902897

‎packages/react/src/profiler.tsx

+123-86
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { getCurrentHub } from '@sentry/browser';
2-
import { Integration, IntegrationClass } from '@sentry/types';
3-
import { logger } from '@sentry/utils';
2+
import { Integration, IntegrationClass, Span } from '@sentry/types';
3+
import { logger, timestampWithMs } from '@sentry/utils';
44
import * as hoistNonReactStatic from 'hoist-non-react-statics';
55
import * as React from 'react';
66

@@ -10,11 +10,6 @@ const TRACING_GETTER = ({
1010
id: 'Tracing',
1111
} as any) as IntegrationClass<Integration>;
1212

13-
// https://stackoverflow.com/questions/52702466/detect-react-reactdom-development-production-build
14-
function isReactInDevMode(): boolean {
15-
return '_self' in React.createElement('div');
16-
}
17-
1813
/**
1914
*
2015
* Based on implementation from Preact:
@@ -44,103 +39,100 @@ function afterNextFrame(callback: Function): void {
4439
timeout = window.setTimeout(done, 100);
4540
}
4641

47-
let profilerCount = 0;
48-
49-
const profiledComponents: {
50-
[key: string]: number;
51-
} = {};
52-
53-
/**
54-
* getInitActivity pushes activity based on React component mount
55-
* @param name displayName of component that started activity
56-
*/
57-
const getInitActivity = (name: string): number | null => {
58-
const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
59-
60-
if (tracingIntegration === null) {
61-
logger.warn(
62-
`Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`,
63-
);
64-
65-
return null;
66-
}
67-
68-
// tslint:disable-next-line:no-unsafe-any
69-
const activity = (tracingIntegration as any).constructor.pushActivity(name, {
70-
description: `<${name}>`,
71-
op: 'react',
72-
}) as number;
73-
74-
/**
75-
* If an activity was already generated, this the component is in React.StrictMode.
76-
* React.StrictMode will call constructors and setState hooks twice, effectively
77-
* creating redundant spans for every render (ex. two <App /> spans, two <Link /> spans)
78-
*
79-
* React.StrictMode only has this behaviour in Development Mode
80-
* See: https://reactjs.org/docs/strict-mode.html
81-
*
82-
* To account for this, we track all profiled components, and cancel activities that
83-
* we recognize to be coming from redundant push activity calls. It is important to note
84-
* that it is the first call to push activity that is invalid, as that is the one caused
85-
* by React.StrictMode.
86-
*
87-
*/
88-
if (isReactInDevMode()) {
89-
// We can make the guarantee here that if a redundant activity exists, it comes right
90-
// before the current activity, hence having a profilerCount one less than the existing count.
91-
const redundantActivity = profiledComponents[`${name}${profilerCount - 1}`];
92-
93-
if (redundantActivity) {
94-
// tslint:disable-next-line: no-unsafe-any
95-
(tracingIntegration as any).constructor.cancelActivity(redundantActivity);
96-
} else {
97-
// If an redundant activity didn't exist, we can store the current activity to
98-
// check later. We have to do this inside an else block because of the case of
99-
// the edge case where two components may share a single components name.
100-
profiledComponents[`${name}${profilerCount}`] = activity;
101-
}
102-
}
42+
function warnAboutTracing(name: string): void {
43+
logger.warn(
44+
`Unable to profile component ${name} due to invalid Tracing Integration. Please make sure to setup the Tracing integration.`,
45+
);
46+
}
10347

104-
profilerCount += 1;
105-
return activity;
106-
};
48+
enum ReactOp {
49+
Mount = 'mount',
50+
Visible = 'visible',
51+
}
10752

10853
export type ProfilerProps = {
10954
name: string;
11055
};
11156

11257
class Profiler extends React.Component<ProfilerProps> {
113-
public activity: number | null;
58+
public tracingIntegration: Integration | null = getCurrentHub().getIntegration(TRACING_GETTER);
59+
public mountInfo: {
60+
activity: number | null;
61+
span: Span | null;
62+
} = {
63+
activity: null,
64+
span: null,
65+
};
66+
public visibleActivity: number | null = null;
11467

11568
public constructor(props: ProfilerProps) {
11669
super(props);
11770

118-
this.activity = getInitActivity(this.props.name);
71+
if (this.tracingIntegration === null) {
72+
warnAboutTracing(props.name);
73+
} else {
74+
// tslint:disable-next-line:no-unsafe-any
75+
const activity = (this.tracingIntegration as any).constructor.pushActivity(props.name, {
76+
data: {
77+
update: 0,
78+
},
79+
description: `<${props.name}>`,
80+
op: `react.${ReactOp.Mount}`,
81+
}) as number;
82+
83+
if (activity) {
84+
this.mountInfo.activity = activity;
85+
// tslint:disable-next-line: no-unsafe-any
86+
this.mountInfo.span = (this.tracingIntegration as any).constructor.getActivitySpan(activity);
87+
}
88+
}
11989
}
12090

12191
// If a component mounted, we can finish the mount activity.
12292
public componentDidMount(): void {
123-
afterNextFrame(this.finishProfile);
124-
}
93+
afterNextFrame(() => {
94+
if (this.tracingIntegration === null) {
95+
return;
96+
}
12597

126-
// Sometimes a component will unmount first, so we make
127-
// sure to also finish the mount activity here.
128-
public componentWillUnmount(): void {
129-
afterNextFrame(this.finishProfile);
130-
}
98+
if (this.mountInfo.activity) {
99+
// tslint:disable-next-line:no-unsafe-any
100+
(this.tracingIntegration as any).constructor.popActivity(this.mountInfo.activity);
101+
this.mountInfo.activity = null;
102+
}
131103

132-
public finishProfile = () => {
133-
if (!this.activity) {
134-
return;
135-
}
104+
if (this.mountInfo.span) {
105+
// tslint:disable-next-line:no-unsafe-any
106+
this.visibleActivity = (this.tracingIntegration as any).constructor.pushActivity(
107+
this.props.name,
108+
{
109+
description: `<${this.props.name}>`,
110+
op: `react.${ReactOp.Visible}`,
111+
},
112+
{ parentSpanId: this.mountInfo.span.spanId, canBeCancelled: true },
113+
) as number;
114+
}
115+
});
116+
}
136117

137-
const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
138-
if (tracingIntegration !== null) {
118+
public componentDidUpdate(): void {
119+
if (this.tracingIntegration !== null && this.mountInfo.span && this.mountInfo.span.data.update) {
139120
// tslint:disable-next-line:no-unsafe-any
140-
(tracingIntegration as any).constructor.popActivity(this.activity);
141-
this.activity = null;
121+
this.mountInfo.span.setData('update', (this.mountInfo.span.data.update += 1));
142122
}
143-
};
123+
}
124+
125+
// If a component doesn't mount, the visible activity will be end when the
126+
// transaction ends.
127+
public componentWillUnmount(): void {
128+
afterNextFrame(() => {
129+
if (this.visibleActivity && this.tracingIntegration !== null) {
130+
// tslint:disable-next-line:no-unsafe-any
131+
(this.tracingIntegration as any).constructor.popActivity(this.visibleActivity);
132+
this.visibleActivity = null;
133+
}
134+
});
135+
}
144136

145137
public render(): React.ReactNode {
146138
return this.props.children;
@@ -179,14 +171,59 @@ function withProfiler<P extends object>(WrappedComponent: React.ComponentType<P>
179171
* @param name displayName of component being profiled
180172
*/
181173
function useProfiler(name: string): void {
182-
const [activity] = React.useState(() => getInitActivity(name));
174+
const [mountActivity] = React.useState(() => {
175+
const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
176+
177+
if (tracingIntegration !== null) {
178+
// tslint:disable-next-line: no-unsafe-any
179+
return (tracingIntegration as any).constructor.pushActivity(name, {
180+
description: `<${name}>`,
181+
op: `react.${ReactOp.Mount}`,
182+
startTimestamp: timestampWithMs(),
183+
}) as number;
184+
}
185+
186+
warnAboutTracing(name);
187+
return null;
188+
});
189+
190+
const [visibleActivity] = React.useState(() => {
191+
const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
192+
193+
if (tracingIntegration !== null) {
194+
// tslint:disable-next-line: no-unsafe-any
195+
return (tracingIntegration as any).constructor.pushActivity(
196+
name,
197+
{
198+
description: `<${name}>`,
199+
op: `react.${ReactOp.Visible}`,
200+
startTimestamp: timestampWithMs(),
201+
},
202+
{ autoPopAfter: 0 },
203+
) as number;
204+
}
205+
206+
warnAboutTracing(name);
207+
return null;
208+
});
183209

184210
React.useEffect(() => {
185211
afterNextFrame(() => {
186212
const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
213+
214+
if (tracingIntegration !== null) {
215+
// tslint:disable-next-line:no-unsafe-any
216+
(tracingIntegration as any).constructor.popActivity(mountActivity);
217+
}
218+
});
219+
220+
// tslint:disable-next-line: no-void-expression
221+
return afterNextFrame(() => {
222+
const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
223+
187224
if (tracingIntegration !== null) {
188225
// tslint:disable-next-line:no-unsafe-any
189-
(tracingIntegration as any).constructor.popActivity(activity);
226+
(tracingIntegration as any).constructor.popActivity(visibleActivity);
190227
}
191228
});
192229
}, []);

‎packages/react/test/profiler.test.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { renderHook } from '@testing-library/react-hooks';
33
import * as React from 'react';
44

55
import { UNKNOWN_COMPONENT, useProfiler, withProfiler } from '../src/profiler';
6+
import { Span } from '@sentry/types';
67

78
const mockPushActivity = jest.fn().mockReturnValue(1);
89
const mockPopActivity = jest.fn();
@@ -29,6 +30,7 @@ jest.mock('@sentry/browser', () => ({
2930
public setupOnce: () => void = jest.fn();
3031
public static pushActivity: () => void = mockPushActivity;
3132
public static popActivity: () => void = mockPopActivity;
33+
public static getActivity: () => Span = jest.fn().mockReturnValue({ span: { id: 'sldjflsf' } });
3234
}
3335

3436
if (!integrationIsNull) {

0 commit comments

Comments
 (0)