From f6d2a0a57a3737aabfd5193f417e37f9bbb27b77 Mon Sep 17 00:00:00 2001 From: Yash Suresh Chandra Date: Wed, 5 Aug 2026 15:51:37 +0530 Subject: [PATCH] fix(ember): Stop retaining component render payloads in beforeEntries Co-Authored-By: Claude Fable 5 --- .../addon/utils/instrumentEmberGlobals.ts | 28 +++++----- .../unit/instrument-ember-globals-test.ts | 51 +++++++++++++++++++ 2 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 packages/ember/tests/unit/instrument-ember-globals-test.ts diff --git a/packages/ember/addon/utils/instrumentEmberGlobals.ts b/packages/ember/addon/utils/instrumentEmberGlobals.ts index 291c18faf56b..dee0e0b406d4 100644 --- a/packages/ember/addon/utils/instrumentEmberGlobals.ts +++ b/packages/ember/addon/utils/instrumentEmberGlobals.ts @@ -19,9 +19,7 @@ type RenderEntry = { now: number; }; -interface RenderEntries { - [name: string]: RenderEntry; -} +export type RenderEntries = Map; /** This is global, so should only be run once in tests! */ export function instrumentGlobalsForPerformance(config: { @@ -127,26 +125,30 @@ function _instrumentEmberRunloop(config: { minimumRunloopQueueDuration?: number }); } -function processComponentRenderBefore(payload: Payload, beforeEntries: RenderEntries): void { +export function _processComponentRenderBefore(payload: Payload, beforeEntries: RenderEntries): void { const info = { payload, now: timestampInSeconds(), }; - beforeEntries[payload.object] = info; + beforeEntries.set(payload.object, info); } -function processComponentRenderAfter( +export function _processComponentRenderAfter( payload: Payload, beforeEntries: RenderEntries, op: string, minComponentDuration: number, ): void { - const begin = beforeEntries[payload.object]; + const begin = beforeEntries.get(payload.object); if (!begin) { return; } + // Remove the entry so the render payload (which references the component + // instance) is not retained forever in this module-scope map. + beforeEntries.delete(payload.object); + const now = timestampInSeconds(); const componentRenderDuration = now - begin.now; @@ -174,27 +176,27 @@ function _instrumentComponents(config: { const minComponentDuration = minimumComponentRenderDuration ?? 2; - const beforeEntries = {} as RenderEntries; - const beforeComponentDefinitionEntries = {} as RenderEntries; + const beforeEntries: RenderEntries = new Map(); + const beforeComponentDefinitionEntries: RenderEntries = new Map(); 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); }, }); } diff --git a/packages/ember/tests/unit/instrument-ember-globals-test.ts b/packages/ember/tests/unit/instrument-ember-globals-test.ts new file mode 100644 index 000000000000..24910709a95a --- /dev/null +++ b/packages/ember/tests/unit/instrument-ember-globals-test.ts @@ -0,0 +1,51 @@ +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 Map(); + const payload = { containerKey: 'component:test-component', initialRender: true as const, object: '' }; + + _processComponentRenderBefore(payload, beforeEntries); + assert.strictEqual(beforeEntries.size, 1, 'Entry is recorded when the render starts'); + + _processComponentRenderAfter(payload, beforeEntries, 'ui.ember.component.render', 1_000); + assert.strictEqual( + beforeEntries.size, + 0, + '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 Map(); + const payload = { containerKey: 'component:test-component', initialRender: true as const, object: '' }; + + _processComponentRenderBefore(payload, beforeEntries); + _processComponentRenderAfter(payload, beforeEntries, 'ui.ember.component.render', 0); + + assert.strictEqual(beforeEntries.size, 0, '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 Map(); + const trackedPayload = { containerKey: 'component:tracked', initialRender: true as const, object: '' }; + const unknownPayload = { containerKey: 'component:unknown', initialRender: true as const, object: '' }; + + _processComponentRenderBefore(trackedPayload, beforeEntries); + _processComponentRenderAfter(unknownPayload, beforeEntries, 'ui.ember.component.render', 1_000); + + assert.strictEqual(beforeEntries.size, 1, 'Unrelated in-flight entries are kept'); + assert.true(beforeEntries.has(trackedPayload.object), 'The in-flight entry is still tracked'); + }); +});