From 9787f3ec48a2419c4a89e4664b0ed3f75134a249 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 11:46:44 -0400 Subject: [PATCH 01/10] ref(profiling): reinitialize profilerId on explicit stop calls --- packages/profiling-node/src/integration.ts | 58 +++++++++++++++------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index c1a96015f0c4..5808f6d1c654 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -17,7 +17,7 @@ import { CpuProfilerBindings } from './cpu_profiler'; import { DEBUG_BUILD } from './debug-build'; import { NODE_MAJOR, NODE_VERSION } from './nodeVersion'; import { MAX_PROFILE_DURATION_MS, maybeProfileSpan, stopSpanProfile } from './spanProfileUtils'; -import type { RawChunkCpuProfile, RawThreadCpuProfile } from './types'; +import type { RawThreadCpuProfile } from './types'; import { ProfileFormat } from './types'; import { PROFILER_THREAD_NAME } from './utils'; @@ -161,7 +161,7 @@ interface ChunkData { } class ContinuousProfiler { - private _profilerId = uuid4(); + private _profilerId: string | undefined; private _client: NodeClient | undefined = undefined; private _chunkData: ChunkData | undefined = undefined; @@ -175,12 +175,27 @@ class ContinuousProfiler { } /** - * Recursively schedules chunk profiling to start and stop at a set interval. - * Once the user calls stop(), the current chunk will be stopped and flushed to Sentry and no new chunks will - * will be started. To restart continuous mode after calling stop(), the user must call start() again. + * Initializes a new profilerId session and schedules chunk profiling. * @returns void */ public start(): void { + if (this._profilerId) { + DEBUG_BUILD && + logger.log( + '[Profiling] Profiler session is already in progress, please ensure you call stop before starting a new profile.', + ); + return; + } + + this._setupSpanChunkInstrumentation(); + this._chunkStart(); + } + + /** + * Stop profiler and initializes profiling of the next chunk, this method should only be called from + * the chunk timer callback. + */ + public _chunkStart(): void { if (!this._client) { // The client is not attached to the profiler if the user has not enabled continuous profiling. // In this case, calling start() and stop() is a noop action.The reason this exists is because @@ -193,7 +208,7 @@ class ContinuousProfiler { logger.log( `[Profiling] Chunk with chunk_id ${this._chunkData.id} is still running, current chunk will be stopped a new chunk will be started.`, ); - this.stop(); + this._chunkStop(); } const traceId = @@ -207,6 +222,14 @@ class ContinuousProfiler { * @returns void */ public stop(): void { + this._chunkStop(); + this._teardownSpanChunkInstrumentation(); + } + + /** + * Stops profiling of the current chunks and flushes the profile to Sentry + */ + public _chunkStop(): void { if (this._chunkData?.timer) { global.clearTimeout(this._chunkData.timer); this._chunkData.timer = undefined; @@ -223,12 +246,17 @@ class ContinuousProfiler { return; } - const profile = this._stopChunkProfiling(this._chunkData); + const profile = CpuProfilerBindings.stopProfiling(this._chunkData.id, ProfileFormat.CHUNK); if (!profile) { DEBUG_BUILD && logger.log(`[Profiling] _chunkiledStartTraceID to collect profile for: ${this._chunkData.id}`); return; } + if (!this._profilerId) { + DEBUG_BUILD && + logger.log('[Profiling] Profile chunk does not contain a valid profiler_id, this is a bug in the SDK'); + return; + } if (profile) { DEBUG_BUILD && logger.log(`[Profiling] Sending profile chunk ${this._chunkData.id}.`); } @@ -287,29 +315,19 @@ class ContinuousProfiler { }); } - /** - * Stops the profile and clears chunk instrumentation from global scope - * @returns void - */ - private _stopChunkProfiling(chunk: ChunkData): RawChunkCpuProfile | null { - this._teardownSpanChunkInstrumentation(); - return CpuProfilerBindings.stopProfiling(chunk.id, ProfileFormat.CHUNK); - } - /** * Starts the profiler and registers the flush timer for a given chunk. * @param chunk */ private _startChunkProfiling(chunk: ChunkData): void { - this._setupSpanChunkInstrumentation(); CpuProfilerBindings.startProfiling(chunk.id); DEBUG_BUILD && logger.log(`[Profiling] starting profiling chunk: ${chunk.id}`); chunk.timer = global.setTimeout(() => { DEBUG_BUILD && logger.log(`[Profiling] Stopping profiling chunk: ${chunk.id}`); - this.stop(); + this._chunkStop(); DEBUG_BUILD && logger.log('[Profiling] Starting new profiling chunk.'); - setImmediate(this.start.bind(this)); + setImmediate(this._chunkStart.bind(this)); }, CHUNK_INTERVAL_MS); // Unref timeout so it doesn't keep the process alive. @@ -327,6 +345,7 @@ class ContinuousProfiler { return; } + this._profilerId = uuid4(); getGlobalScope().setContext('profile', { profiler_id: this._profilerId, }); @@ -338,6 +357,7 @@ class ContinuousProfiler { * Clear profiling information from global context when a profile is not running. */ private _teardownSpanChunkInstrumentation(): void { + this._profilerId = undefined; const globalScope = getGlobalScope(); globalScope.setContext('profile', {}); } From 34ceb06c72a20396c72e7156482078a88047af19 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 13:59:04 -0400 Subject: [PATCH 02/10] ref(profiling): simplify integration --- packages/profiling-node/src/integration.ts | 72 ++++++++++++++-------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index 5808f6d1c654..67c63248b2b7 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -172,6 +172,10 @@ class ContinuousProfiler { */ public initialize(client: NodeClient): void { this._client = client; + + // There is no off method to disable this, so we need to ensure to add the listener only once. This adds overhead + // to the event processing, but it is minimal as we short circuit if there is no profilerId active and return early. + this._client.on('beforeSendEvent', this._onBeforeSendThreadContextAssignment.bind(this)); } /** @@ -179,6 +183,11 @@ class ContinuousProfiler { * @returns void */ public start(): void { + if (!this._client) { + DEBUG_BUILD && logger.log('[Profiling] Failed to start, sentry client was never attached to the profiler.'); + return; + } + if (this._profilerId) { DEBUG_BUILD && logger.log( @@ -192,10 +201,22 @@ class ContinuousProfiler { } /** - * Stop profiler and initializes profiling of the next chunk, this method should only be called from - * the chunk timer callback. + * Stops the current chunk and flushes the profile to Sentry. + * @returns void */ - public _chunkStart(): void { + public stop(): void { + if (!this._client) { + DEBUG_BUILD && logger.log('[Profiling] Failed to stop, sentry client was never attached to the profiler.'); + return; + } + this._chunkStop(); + this._teardownSpanChunkInstrumentation(); + } + + /** + * Stop profiler and initializes profiling of the next chunk + */ + private _chunkStart(): void { if (!this._client) { // The client is not attached to the profiler if the user has not enabled continuous profiling. // In this case, calling start() and stop() is a noop action.The reason this exists is because @@ -211,25 +232,13 @@ class ContinuousProfiler { this._chunkStop(); } - const traceId = - getCurrentScope().getPropagationContext().traceId || getIsolationScope().getPropagationContext().traceId; - this._initializeChunk(traceId); - this._startChunkProfiling(this._chunkData!); - } - - /** - * Stops the current chunk and flushes the profile to Sentry. - * @returns void - */ - public stop(): void { - this._chunkStop(); - this._teardownSpanChunkInstrumentation(); + this._startChunkProfiling(); } /** * Stops profiling of the current chunks and flushes the profile to Sentry */ - public _chunkStop(): void { + private _chunkStop(): void { if (this._chunkData?.timer) { global.clearTimeout(this._chunkData.timer); this._chunkData.timer = undefined; @@ -276,7 +285,7 @@ class ContinuousProfiler { if (!chunk) { DEBUG_BUILD && logger.log(`[Profiling] Failed to create profile chunk for: ${this._chunkData.id}`); - this._reset(); + this._resetChunkData(); return; } @@ -285,7 +294,7 @@ class ContinuousProfiler { // the format may negatively impact the performance of the application. To avoid // blocking for too long, enqueue the next chunk start inside the next macrotask. // clear current chunk - this._reset(); + this._resetChunkData(); } /** @@ -319,7 +328,11 @@ class ContinuousProfiler { * Starts the profiler and registers the flush timer for a given chunk. * @param chunk */ - private _startChunkProfiling(chunk: ChunkData): void { + private _startChunkProfiling(): void { + const traceId = + getCurrentScope().getPropagationContext().traceId || getIsolationScope().getPropagationContext().traceId; + const chunk = this._initializeChunk(traceId); + CpuProfilerBindings.startProfiling(chunk.id); DEBUG_BUILD && logger.log(`[Profiling] starting profiling chunk: ${chunk.id}`); @@ -341,7 +354,9 @@ class ContinuousProfiler { private _setupSpanChunkInstrumentation(): void { if (!this._client) { DEBUG_BUILD && - logger.log('[Profiling] Failed to collect profile, sentry client was never attached to the profiler.'); + logger.log( + '[Profiling] Failed to initialize span profiling, sentry client was never attached to the profiler.', + ); return; } @@ -349,8 +364,14 @@ class ContinuousProfiler { getGlobalScope().setContext('profile', { profiler_id: this._profilerId, }); + } - this._client.on('beforeSendEvent', e => this._assignThreadIdContext(e)); + /** + * Assigns thread_id and thread name context to a profiled event if there is an active profiler session + */ + private _onBeforeSendThreadContextAssignment(event: Event): void { + if (!this._client || !this._profilerId) return; + this._assignThreadIdContext(event); } /** @@ -365,18 +386,19 @@ class ContinuousProfiler { /** * Initializes new profile chunk metadata */ - private _initializeChunk(traceId: string): void { + private _initializeChunk(traceId: string): ChunkData { this._chunkData = { id: uuid4(), startTraceID: traceId, timer: undefined, }; + return this._chunkData; } /** * Assigns thread_id and thread name context to a profiled event. */ - private _assignThreadIdContext(event: Event): any { + private _assignThreadIdContext(event: Event): void { if (!event?.['contexts']?.['profile']) { return; } @@ -400,7 +422,7 @@ class ContinuousProfiler { /** * Resets the current chunk state. */ - private _reset(): void { + private _resetChunkData(): void { this._chunkData = undefined; } } From 6831de7f2216f9b9014070a77fdcb1d31e96ee34 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 14:13:27 -0400 Subject: [PATCH 03/10] ref(profiling): update tests --- packages/profiling-node/src/integration.ts | 10 +-- .../test/spanProfileUtils.test.ts | 76 +++++-------------- 2 files changed, 22 insertions(+), 64 deletions(-) diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index 67c63248b2b7..bd4d149c92e5 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -188,14 +188,10 @@ class ContinuousProfiler { return; } - if (this._profilerId) { - DEBUG_BUILD && - logger.log( - '[Profiling] Profiler session is already in progress, please ensure you call stop before starting a new profile.', - ); - return; - } + // Flush any existing chunks before starting a new one. + this._chunkStop(); + // Restart the profiler session this._setupSpanChunkInstrumentation(); this._chunkStart(); } diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index f65556f57ab4..4ba3851ae6a2 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -394,7 +394,7 @@ describe('continuous profiling', () => { const integration = client?.getIntegrationByName>('ProfilingIntegration'); if (integration) { - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); } jest.clearAllMocks(); @@ -432,14 +432,9 @@ describe('continuous profiling', () => { client.init(); const transportSpy = jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(1000); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); jest.advanceTimersByTime(1000); const profile = transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[1] as ProfileChunk; @@ -455,14 +450,10 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); + Sentry.profiler.startProfiler(); const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); - - expect(integration._profiler).toBeDefined(); + expect(integration?._profiler).toBeDefined(); }); it('starts a continuous profile', () => { @@ -473,11 +464,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); expect(startProfilingSpy).toHaveBeenCalledTimes(1); }); @@ -490,12 +477,9 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); - integration._profiler.start(); + Sentry.profiler.startProfiler(); + Sentry.profiler.startProfiler(); + expect(startProfilingSpy).toHaveBeenCalledTimes(2); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); }); @@ -509,11 +493,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(5001); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); @@ -529,11 +509,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(5001); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); @@ -548,15 +524,11 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(1000); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); expect(stopProfilingSpy).toHaveBeenCalledTimes(1); jest.advanceTimersByTime(1000); @@ -603,14 +575,9 @@ describe('continuous profiling', () => { client.init(); const transportSpy = jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); jest.advanceTimersByTime(1000); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); jest.advanceTimersByTime(1000); expect(transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[0]?.type).toBe('profile_chunk'); @@ -640,7 +607,7 @@ describe('continuous profiling', () => { integration._profiler.start(); const profiledTransaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' }); profiledTransaction.end(); - integration._profiler.stop(); + Sentry.profiler.stopProfiler(); expect(transportSpy.mock.calls?.[1]?.[0]?.[1]?.[0]?.[1]).toMatchObject({ contexts: { @@ -658,7 +625,7 @@ describe('continuous profiling', () => { }); }); -describe('span profiling mode', () => { +describe('continuous profiling does not start in span profiling mode', () => { it.each([ ['profilesSampleRate=1', makeClientOptions({ profilesSampleRate: 1 })], ['profilesSampler is defined', makeClientOptions({ profilesSampler: () => 1 })], @@ -699,7 +666,7 @@ describe('span profiling mode', () => { } integration._profiler.start(); - expect(logSpy).toHaveBeenLastCalledWith('[Profiling] Profiler was never attached to the client.'); + expect(logSpy).toHaveBeenLastCalledWith('[Profiling] Failed to start, sentry client was never attached to the profiler.'); }); }); describe('continuous profiling mode', () => { @@ -742,12 +709,7 @@ describe('continuous profiling mode', () => { } jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - - const integration = client.getIntegrationByName>('ProfilingIntegration'); - if (!integration) { - throw new Error('Profiling integration not found'); - } - integration._profiler.start(); + Sentry.profiler.startProfiler(); const callCount = startProfilingSpy.mock.calls.length; expect(startProfilingSpy).toHaveBeenCalled(); From c08b54b38005e415e76020a4a0d3379e4d19482c Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 14:18:37 -0400 Subject: [PATCH 04/10] ref(profiling): assert that profilerId is updated --- .../test/spanProfileUtils.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 4ba3851ae6a2..57036e1bf93a 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -52,6 +52,11 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { return [client, client.getTransport() as Transport]; } +function getProfilerId(): string { + // @ts-expect-error accessing a pvt field + return Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration')?._profiler?._profilerId +} + function makeClientOptions( options: Omit, ): NodeClientOptions { @@ -500,6 +505,42 @@ describe('continuous profiling', () => { expect(startProfilingSpy).toHaveBeenCalledTimes(2); }); + it('chunks share the same profilerId', async () => { + const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); + const stopProfilingSpy = jest.spyOn(CpuProfilerBindings, 'stopProfiling'); + + const [client] = makeContinuousProfilingClient(); + Sentry.setCurrentClient(client); + client.init(); + + expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); + Sentry.profiler.startProfiler(); + const profilerId = getProfilerId(); + + jest.advanceTimersByTime(5001); + expect(stopProfilingSpy).toHaveBeenCalledTimes(1); + expect(startProfilingSpy).toHaveBeenCalledTimes(2); + expect(getProfilerId()).toBe(profilerId); + }); + + it('explicit calls to stop clear profilerId', async () => { + const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); + const stopProfilingSpy = jest.spyOn(CpuProfilerBindings, 'stopProfiling'); + + const [client] = makeContinuousProfilingClient(); + Sentry.setCurrentClient(client); + client.init(); + + expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); + Sentry.profiler.startProfiler(); + const profilerId = getProfilerId(); + Sentry.profiler.stopProfiler(); + Sentry.profiler.startProfiler(); + + expect(getProfilerId()).toEqual(expect.any(String)) + expect(getProfilerId()).not.toBe(profilerId); + }); + it('stops a continuous profile after interval', async () => { const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); const stopProfilingSpy = jest.spyOn(CpuProfilerBindings, 'stopProfiling'); From afe7977d89646ba3e7fa246a2d69af85ce6e3653 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 14:23:50 -0400 Subject: [PATCH 05/10] ref(profiling): remove unused call --- packages/profiling-node/test/spanProfileUtils.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 57036e1bf93a..492a8ec592da 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -525,7 +525,6 @@ describe('continuous profiling', () => { it('explicit calls to stop clear profilerId', async () => { const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); - const stopProfilingSpy = jest.spyOn(CpuProfilerBindings, 'stopProfiling'); const [client] = makeContinuousProfilingClient(); Sentry.setCurrentClient(client); From 37f90d3c116cf9aa0aa9ba29d6729cc14fe4515f Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 14:25:24 -0400 Subject: [PATCH 06/10] lint --- packages/profiling-node/test/spanProfileUtils.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 492a8ec592da..2a97eca883a4 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -54,7 +54,8 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { function getProfilerId(): string { // @ts-expect-error accessing a pvt field - return Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration')?._profiler?._profilerId + return Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration') + ?._profiler?._profilerId; } function makeClientOptions( @@ -536,7 +537,7 @@ describe('continuous profiling', () => { Sentry.profiler.stopProfiler(); Sentry.profiler.startProfiler(); - expect(getProfilerId()).toEqual(expect.any(String)) + expect(getProfilerId()).toEqual(expect.any(String)); expect(getProfilerId()).not.toBe(profilerId); }); @@ -706,7 +707,9 @@ describe('continuous profiling does not start in span profiling mode', () => { } integration._profiler.start(); - expect(logSpy).toHaveBeenLastCalledWith('[Profiling] Failed to start, sentry client was never attached to the profiler.'); + expect(logSpy).toHaveBeenLastCalledWith( + '[Profiling] Failed to start, sentry client was never attached to the profiler.', + ); }); }); describe('continuous profiling mode', () => { From 0926a08834f2a714a1d97938c4454cd7b9300268 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 14:38:39 -0400 Subject: [PATCH 07/10] lint --- packages/profiling-node/test/spanProfileUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 2a97eca883a4..57622a51ad22 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -53,8 +53,8 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { } function getProfilerId(): string { - // @ts-expect-error accessing a pvt field return Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration') + // @ts-expect-error accessing a pvt field ?._profiler?._profilerId; } From d8c837a06d1496e32b3781afd374257d1153fc69 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 12 Sep 2024 15:13:47 -0400 Subject: [PATCH 08/10] format --- packages/profiling-node/test/spanProfileUtils.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 57622a51ad22..25db2485318e 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -53,9 +53,11 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { } function getProfilerId(): string { - return Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration') - // @ts-expect-error accessing a pvt field - ?._profiler?._profilerId; + return ( + // @ts-expect-error accessing a pvt field + Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration')?._profiler + ?._profilerId + ); } function makeClientOptions( From 4825f28c5a4f2eede6888d0870acd22c62f9917a Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 16 Sep 2024 09:47:05 -0400 Subject: [PATCH 09/10] ref: update comment --- packages/profiling-node/src/integration.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index bd4d149c92e5..2eb0a59142ae 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -173,8 +173,9 @@ class ContinuousProfiler { public initialize(client: NodeClient): void { this._client = client; - // There is no off method to disable this, so we need to ensure to add the listener only once. This adds overhead - // to the event processing, but it is minimal as we short circuit if there is no profilerId active and return early. + // Attaches a listener to beforeSend which will add the threadId data to the event being sent. + // This adds a constant overhead to all events being sent which could be improved to only attach + // and detach the listener during a profiler session this._client.on('beforeSendEvent', this._onBeforeSendThreadContextAssignment.bind(this)); } From 2ee26bc5e6f83da5506e0994312b98ee73beae06 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 16 Sep 2024 10:00:24 -0400 Subject: [PATCH 10/10] ref: remove ignore --- packages/profiling-node/test/spanProfileUtils.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 25db2485318e..fd2c95ec79e4 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -54,10 +54,8 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { function getProfilerId(): string { return ( - // @ts-expect-error accessing a pvt field - Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration')?._profiler - ?._profilerId - ); + Sentry.getClient()?.getIntegrationByName>('ProfilingIntegration') as any + )?._profiler?._profilerId; } function makeClientOptions(