diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index c52d503eb92b..ecf7608dac3a 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -34,6 +34,10 @@ class Profiler extends React.Component { * Made protected for the React Native SDK to access */ protected _mountSpan: Span | undefined = undefined; + /** + * The span that represents the duration of time between shouldComponentUpdate and componentDidUpdate + */ + protected _updateSpan: Span | undefined = undefined; // eslint-disable-next-line @typescript-eslint/member-ordering public static defaultProps: Partial = { @@ -66,8 +70,8 @@ class Profiler extends React.Component { } } - public componentDidUpdate({ updateProps, includeUpdates = true }: ProfilerProps): void { - // Only generate an update span if hasUpdateSpan is true, if there is a valid mountSpan, + public shouldComponentUpdate({ updateProps, includeUpdates = true }: ProfilerProps): boolean { + // Only generate an update span if includeUpdates 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 (includeUpdates && this._mountSpan && updateProps !== this.props.updateProps) { @@ -75,20 +79,26 @@ class Profiler extends React.Component { // 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(); - this._mountSpan.startChild({ + this._updateSpan = this._mountSpan.startChild({ data: { changedProps, }, description: `<${this.props.name}>`, - endTimestamp: now, op: REACT_UPDATE_OP, startTimestamp: now, }); } } + + return true; + } + + public componentDidUpdate(): void { + if (this._updateSpan) { + this._updateSpan.finish(); + this._updateSpan = undefined; + } } // If a component is unmounted, we can say it is no longer on the screen. diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx index 2e458ed32871..96988f24b44b 100644 --- a/packages/react/test/profiler.test.tsx +++ b/packages/react/test/profiler.test.tsx @@ -36,6 +36,7 @@ jest.mock('@sentry/browser', () => ({ beforeEach(() => { mockStartChild.mockClear(); + mockFinish.mockClear(); activeTransaction = new MockSpan({ op: 'pageload' }); }); @@ -100,7 +101,9 @@ describe('withProfiler', () => { }); it('is not created if hasRenderSpan is false', () => { - const ProfiledComponent = withProfiler(() =>

Testing

, { includeRender: false }); + const ProfiledComponent = withProfiler(() =>

Testing

, { + includeRender: false, + }); expect(mockStartChild).toHaveBeenCalledTimes(0); const component = render(); @@ -115,6 +118,7 @@ describe('withProfiler', () => { const ProfiledComponent = withProfiler((props: { num: number }) =>
{props.num}
); const { rerender } = render(); expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockFinish).toHaveBeenCalledTimes(1); // Dispatch new props rerender(); @@ -122,25 +126,25 @@ describe('withProfiler', () => { expect(mockStartChild).toHaveBeenLastCalledWith({ data: { changedProps: ['num'] }, description: `<${UNKNOWN_COMPONENT}>`, - endTimestamp: expect.any(Number), op: REACT_UPDATE_OP, startTimestamp: expect.any(Number), }); - + expect(mockFinish).toHaveBeenCalledTimes(2); // New props yet again rerender(); expect(mockStartChild).toHaveBeenCalledTimes(3); expect(mockStartChild).toHaveBeenLastCalledWith({ data: { changedProps: ['num'] }, description: `<${UNKNOWN_COMPONENT}>`, - endTimestamp: expect.any(Number), op: REACT_UPDATE_OP, startTimestamp: expect.any(Number), }); + expect(mockFinish).toHaveBeenCalledTimes(3); // Should not create spans if props haven't changed rerender(); expect(mockStartChild).toHaveBeenCalledTimes(3); + expect(mockFinish).toHaveBeenCalledTimes(3); }); it('does not get created if hasUpdateSpan is false', () => {