feat(react): Track duration of React component updates (#5531)

Author: 0Calories

packages/react/src/profiler.tsx

@@ -34,6 +34,10 @@ class Profiler extends React.Component<ProfilerProps> {
    * 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<ProfilerProps> = {
@@ -66,29 +70,35 @@ class Profiler extends React.Component<ProfilerProps> {
     }
   }
 
-  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) {
       // 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();
-        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.

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(() => <h1>Testing</h1>, { includeRender: false });
+      const ProfiledComponent = withProfiler(() => <h1>Testing</h1>, {
+        includeRender: false,
+      });
       expect(mockStartChild).toHaveBeenCalledTimes(0);
 
       const component = render(<ProfiledComponent />);
@@ -115,32 +118,33 @@ describe('withProfiler', () => {
       const ProfiledComponent = withProfiler((props: { num: number }) => <div>{props.num}</div>);
       const { rerender } = render(<ProfiledComponent num={0} />);
       expect(mockStartChild).toHaveBeenCalledTimes(1);
+      expect(mockFinish).toHaveBeenCalledTimes(1);
 
       // Dispatch new props
       rerender(<ProfiledComponent num={1} />);
       expect(mockStartChild).toHaveBeenCalledTimes(2);
       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(<ProfiledComponent num={2} />);
       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(<ProfiledComponent num={2} />);
       expect(mockStartChild).toHaveBeenCalledTimes(3);
+      expect(mockFinish).toHaveBeenCalledTimes(3);
     });
 
     it('does not get created if hasUpdateSpan is false', () => {