fix(ember): Stop retaining component render payloads in beforeEntries

packages/ember/addon/utils/instrumentEmberGlobals.ts

@@ -15,13 +15,10 @@ type Payload = {
 };
 
 type RenderEntry = {
-  payload: Payload;
   now: number;
 };
 
-interface RenderEntries {
-  [name: string]: RenderEntry;
-}
+export type RenderEntries = WeakMap<Payload, RenderEntry>;
 
 /** This is global, so should only be run once in tests! */
 export function instrumentGlobalsForPerformance(config: {
@@ -127,26 +124,26 @@ function _instrumentEmberRunloop(config: { minimumRunloopQueueDuration?: number
   });
 }
 
-function processComponentRenderBefore(payload: Payload, beforeEntries: RenderEntries): void {
-  const info = {
-    payload,
-    now: timestampInSeconds(),
-  };
-  beforeEntries[payload.object] = info;
+export function _processComponentRenderBefore(payload: Payload, beforeEntries: RenderEntries): void {
+  beforeEntries.set(payload, { now: timestampInSeconds() });
 }
 
-function processComponentRenderAfter(
+export function _processComponentRenderAfter(
   payload: Payload,
   beforeEntries: RenderEntries,
   op: string,
   minComponentDuration: number,
 ): void {
-  const begin = beforeEntries[payload.object];
+  const begin = beforeEntries.get(payload);
 
   if (!begin) {
     return;
   }
 
+  // A WeakMap entry cannot outlive its payload, but delete promptly anyway
+  // so a long-lived payload doesn't keep the entry around between renders.
+  beforeEntries.delete(payload);
+
   const now = timestampInSeconds();
   const componentRenderDuration = now - begin.now;
 
@@ -174,27 +171,27 @@ function _instrumentComponents(config: {
 
   const minComponentDuration = minimumComponentRenderDuration ?? 2;
 
-  const beforeEntries = {} as RenderEntries;
-  const beforeComponentDefinitionEntries = {} as RenderEntries;
+  const beforeEntries: RenderEntries = new WeakMap();
+  const beforeComponentDefinitionEntries: RenderEntries = new WeakMap();
 
   function _subscribeToRenderEvents(): void {
     subscribe('render.component', {
       before(_name: string, _timestamp: number, payload: Payload) {
-        processComponentRenderBefore(payload, beforeEntries);
+        _processComponentRenderBefore(payload, beforeEntries);
       },
 
       after(_name: string, _timestamp: number, payload: Payload, _beganIndex: number) {
-        processComponentRenderAfter(payload, beforeEntries, BROWSER_UI_RENDER_SPAN_OP, minComponentDuration);
+        _processComponentRenderAfter(payload, beforeEntries, BROWSER_UI_RENDER_SPAN_OP, minComponentDuration);
       },
     });
     if (enableComponentDefinitions) {
       subscribe('render.getComponentDefinition', {
         before(_name: string, _timestamp: number, payload: Payload) {
-          processComponentRenderBefore(payload, beforeComponentDefinitionEntries);
+          _processComponentRenderBefore(payload, beforeComponentDefinitionEntries);
         },
 
         after(_name: string, _timestamp: number, payload: Payload, _beganIndex: number) {
-          processComponentRenderAfter(payload, beforeComponentDefinitionEntries, GENERAL_FUNCTION_SPAN_OP, 0);
+          _processComponentRenderAfter(payload, beforeComponentDefinitionEntries, GENERAL_FUNCTION_SPAN_OP, 0);
         },
       });
     }

packages/ember/tests/unit/instrument-ember-globals-test.ts

@@ -0,0 +1,50 @@
+import type { RenderEntries } from '@sentry/ember/utils/instrumentEmberGlobals';
+import {
+  _processComponentRenderAfter,
+  _processComponentRenderBefore,
+} from '@sentry/ember/utils/instrumentEmberGlobals';
+import { setupTest } from 'ember-qunit';
+import { module, test } from 'qunit';
+import type { SentryTestContext } from '../helpers/setup-sentry';
+import { setupSentryTest } from '../helpers/setup-sentry';
+
+module('Unit | Utility | instrument-ember-globals', function (hooks) {
+  setupTest(hooks);
+  setupSentryTest(hooks);
+
+  test('_processComponentRenderAfter removes the entry recorded for the render', function (this: SentryTestContext, assert) {
+    const beforeEntries: RenderEntries = new WeakMap();
+    const payload = { containerKey: 'component:test-component', initialRender: true as const, object: '<ember123>' };
+
+    _processComponentRenderBefore(payload, beforeEntries);
+    assert.true(beforeEntries.has(payload), 'Entry is recorded when the render starts');
+
+    _processComponentRenderAfter(payload, beforeEntries, 'ui.ember.component.render', 1_000);
+    assert.false(
+      beforeEntries.has(payload),
+      'Entry is removed when the render finishes, so the payload (and the component instance it references) is not retained',
+    );
+  });
+
+  test('_processComponentRenderAfter removes the entry even when the render is long enough to create a span', function (this: SentryTestContext, assert) {
+    const beforeEntries: RenderEntries = new WeakMap();
+    const payload = { containerKey: 'component:test-component', initialRender: true as const, object: '<ember124>' };
+
+    _processComponentRenderBefore(payload, beforeEntries);
+    _processComponentRenderAfter(payload, beforeEntries, 'ui.ember.component.render', 0);
+
+    assert.false(beforeEntries.has(payload), 'Entry is removed after the span is created');
+  });
+
+  test('_processComponentRenderAfter without a matching before-entry leaves other entries alone', function (this: SentryTestContext, assert) {
+    const beforeEntries: RenderEntries = new WeakMap();
+    const trackedPayload = { containerKey: 'component:tracked', initialRender: true as const, object: '<ember125>' };
+    const unknownPayload = { containerKey: 'component:unknown', initialRender: true as const, object: '<ember126>' };
+
+    _processComponentRenderBefore(trackedPayload, beforeEntries);
+    _processComponentRenderAfter(unknownPayload, beforeEntries, 'ui.ember.component.render', 1_000);
+
+    assert.true(beforeEntries.has(trackedPayload), 'The in-flight entry is still tracked');
+    assert.false(beforeEntries.has(unknownPayload), 'The unknown payload was not added');
+  });
+});