From 01165db19e58c62c8bf1140aa2275eef52d8d779 Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 30 Aug 2024 11:26:02 -0400 Subject: [PATCH] feat(profiling): Expose profiler as top level primitive (#13512) We are about to enter a public beta for continuous profiling, which means it is time to expose this from under the wraps of the integration and align it with how the profiler is exposed in python and iOS SDKs --------- Co-authored-by: Luca Forstner --- packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/core/src/index.ts | 1 + packages/core/src/profiling.ts | 72 +++++++++++++++++++ packages/google-cloud-serverless/src/index.ts | 1 + packages/node/src/index.ts | 1 + packages/profiling-node/src/integration.ts | 14 ++-- .../test/spanProfileUtils.test.ts | 63 +++++++++++----- .../test/spanProfileUtils.worker.test.ts | 5 +- packages/types/src/index.ts | 1 + packages/types/src/profiling.ts | 17 +++++ 12 files changed, 153 insertions(+), 25 deletions(-) create mode 100644 packages/core/src/profiling.ts diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index be3f002dcbb8..0895eb86e364 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -123,6 +123,7 @@ export { withMonitor, withScope, zodErrorsIntegration, + profiler, } from '@sentry/node'; export { init } from './server/sdk'; diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 7b05f8df3a86..27f48c2da0db 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -107,6 +107,7 @@ export { trpcMiddleware, addOpenTelemetryInstrumentation, zodErrorsIntegration, + profiler, } from '@sentry/node'; export { diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index e49e4163af31..d4ca06f85584 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -128,6 +128,7 @@ export { trpcMiddleware, addOpenTelemetryInstrumentation, zodErrorsIntegration, + profiler, } from '@sentry/node'; export { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 792bf3572934..24cea1bea7ca 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -100,6 +100,7 @@ export { sessionTimingIntegration } from './integrations/sessiontiming'; export { zodErrorsIntegration } from './integrations/zoderrors'; export { thirdPartyErrorFilterIntegration } from './integrations/third-party-errors-filter'; export { metrics } from './metrics/exports'; +export { profiler } from './profiling'; export type { MetricData } from '@sentry/types'; export { metricsDefault } from './metrics/exports-default'; export { BrowserMetricsAggregator } from './metrics/browser-aggregator'; diff --git a/packages/core/src/profiling.ts b/packages/core/src/profiling.ts new file mode 100644 index 000000000000..446eebb73671 --- /dev/null +++ b/packages/core/src/profiling.ts @@ -0,0 +1,72 @@ +import type { Profiler, ProfilingIntegration } from '@sentry/types'; +import { logger } from '@sentry/utils'; + +import { getClient } from './currentScopes'; +import { DEBUG_BUILD } from './debug-build'; + +function isProfilingIntegrationWithProfiler( + integration: ProfilingIntegration | undefined, +): integration is ProfilingIntegration { + return ( + !!integration && + typeof integration['_profiler'] !== 'undefined' && + typeof integration['_profiler']['start'] === 'function' && + typeof integration['_profiler']['stop'] === 'function' + ); +} +/** + * Starts the Sentry continuous profiler. + * This mode is exclusive with the transaction profiler and will only work if the profilesSampleRate is set to a falsy value. + * In continuous profiling mode, the profiler will keep reporting profile chunks to Sentry until it is stopped, which allows for continuous profiling of the application. + */ +function startProfiler(): void { + const client = getClient(); + if (!client) { + DEBUG_BUILD && logger.warn('No Sentry client available, profiling is not started'); + return; + } + + const integration = client.getIntegrationByName>('ProfilingIntegration'); + + if (!integration) { + DEBUG_BUILD && logger.warn('ProfilingIntegration is not available'); + return; + } + + if (!isProfilingIntegrationWithProfiler(integration)) { + DEBUG_BUILD && logger.warn('Profiler is not available on profiling integration.'); + return; + } + + integration._profiler.start(); +} + +/** + * Stops the Sentry continuous profiler. + * Calls to stop will stop the profiler and flush the currently collected profile data to Sentry. + */ +function stopProfiler(): void { + const client = getClient(); + if (!client) { + DEBUG_BUILD && logger.warn('No Sentry client available, profiling is not started'); + return; + } + + const integration = client.getIntegrationByName>('ProfilingIntegration'); + if (!integration) { + DEBUG_BUILD && logger.warn('ProfilingIntegration is not available'); + return; + } + + if (!isProfilingIntegrationWithProfiler(integration)) { + DEBUG_BUILD && logger.warn('Profiler is not available on profiling integration.'); + return; + } + + integration._profiler.stop(); +} + +export const profiler: Profiler = { + startProfiler, + stopProfiler, +}; diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 9a501307e79f..f58305c64f6c 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -107,6 +107,7 @@ export { trpcMiddleware, addOpenTelemetryInstrumentation, zodErrorsIntegration, + profiler, } from '@sentry/node'; export { diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 9ef89ab42fb7..5f4adf315fd5 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -127,6 +127,7 @@ export { spanToBaggageHeader, trpcMiddleware, zodErrorsIntegration, + profiler, } from '@sentry/core'; export type { diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index 50f240bff732..c1a96015f0c4 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -9,7 +9,7 @@ import { spanToJSON, } from '@sentry/core'; import type { NodeClient } from '@sentry/node'; -import type { Event, Integration, IntegrationFn, Profile, ProfileChunk, Span } from '@sentry/types'; +import type { Event, IntegrationFn, Profile, ProfileChunk, ProfilingIntegration, Span } from '@sentry/types'; import { LRUMap, logger, uuid4 } from '@sentry/utils'; @@ -159,6 +159,7 @@ interface ChunkData { timer: NodeJS.Timeout | undefined; startTraceID: string; } + class ContinuousProfiler { private _profilerId = uuid4(); private _client: NodeClient | undefined = undefined; @@ -384,12 +385,8 @@ class ContinuousProfiler { } } -export interface ProfilingIntegration extends Integration { - _profiler: ContinuousProfiler; -} - /** Exported only for tests. */ -export const _nodeProfilingIntegration = ((): ProfilingIntegration => { +export const _nodeProfilingIntegration = ((): ProfilingIntegration => { if (DEBUG_BUILD && ![16, 18, 20, 22].includes(NODE_MAJOR)) { logger.warn( `[Profiling] You are using a Node.js version that does not have prebuilt binaries (${NODE_VERSION}).`, @@ -407,7 +404,10 @@ export const _nodeProfilingIntegration = ((): ProfilingIntegration => { const options = client.getOptions(); const mode = - (options.profilesSampleRate === undefined || options.profilesSampleRate === 0) && !options.profilesSampler + (options.profilesSampleRate === undefined || + options.profilesSampleRate === null || + options.profilesSampleRate === 0) && + !options.profilesSampler ? 'continuous' : 'span'; switch (mode) { diff --git a/packages/profiling-node/test/spanProfileUtils.test.ts b/packages/profiling-node/test/spanProfileUtils.test.ts index 4a90caa0f353..f65556f57ab4 100644 --- a/packages/profiling-node/test/spanProfileUtils.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.test.ts @@ -2,10 +2,11 @@ import * as Sentry from '@sentry/node'; import { getMainCarrier } from '@sentry/core'; import type { NodeClientOptions } from '@sentry/node/build/types/types'; +import type { ProfilingIntegration } from '@sentry/types'; import type { ProfileChunk, Transport } from '@sentry/types'; import { GLOBAL_OBJ, createEnvelope, logger } from '@sentry/utils'; import { CpuProfilerBindings } from '../src/cpu_profiler'; -import { type ProfilingIntegration, _nodeProfilingIntegration } from '../src/integration'; +import { _nodeProfilingIntegration } from '../src/integration'; function makeClientWithHooks(): [Sentry.NodeClient, Transport] { const integration = _nodeProfilingIntegration(); @@ -299,7 +300,7 @@ describe('automated span instrumentation', () => { Sentry.setCurrentClient(client); client.init(); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -390,7 +391,7 @@ describe('continuous profiling', () => { }); afterEach(() => { const client = Sentry.getClient(); - const integration = client?.getIntegrationByName('ProfilingIntegration'); + const integration = client?.getIntegrationByName>('ProfilingIntegration'); if (integration) { integration._profiler.stop(); @@ -432,7 +433,7 @@ describe('continuous profiling', () => { const transportSpy = jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -446,7 +447,7 @@ describe('continuous profiling', () => { expect(profile.client_sdk.version).toEqual(expect.stringMatching(/\d+\.\d+\.\d+/)); }); - it('initializes the continuous profiler and binds the sentry client', () => { + it('initializes the continuous profiler', () => { const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); const [client] = makeContinuousProfilingClient(); @@ -455,14 +456,13 @@ describe('continuous profiling', () => { expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } integration._profiler.start(); expect(integration._profiler).toBeDefined(); - expect(integration._profiler['_client']).toBe(client); }); it('starts a continuous profile', () => { @@ -473,7 +473,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -490,7 +490,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -509,7 +509,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -529,7 +529,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -548,7 +548,7 @@ describe('continuous profiling', () => { client.init(); expect(startProfilingSpy).not.toHaveBeenCalledTimes(1); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -604,7 +604,7 @@ describe('continuous profiling', () => { const transportSpy = jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -632,7 +632,7 @@ describe('continuous profiling', () => { }, }); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -692,7 +692,7 @@ describe('span profiling mode', () => { Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' }); expect(startProfilingSpy).toHaveBeenCalled(); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); @@ -703,6 +703,10 @@ describe('span profiling mode', () => { }); }); describe('continuous profiling mode', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it.each([ ['profilesSampleRate=0', makeClientOptions({ profilesSampleRate: 0 })], ['profilesSampleRate=undefined', makeClientOptions({ profilesSampleRate: undefined })], @@ -739,7 +743,7 @@ describe('continuous profiling mode', () => { jest.spyOn(transport, 'send').mockReturnValue(Promise.resolve({})); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } @@ -750,4 +754,31 @@ describe('continuous profiling mode', () => { Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' }); expect(startProfilingSpy).toHaveBeenCalledTimes(callCount); }); + + it('top level methods proxy to integration', () => { + const client = new Sentry.NodeClient({ + ...makeClientOptions({ profilesSampleRate: undefined }), + dsn: 'https://7fa19397baaf433f919fbe02228d5470@o1137848.ingest.sentry.io/6625302', + tracesSampleRate: 1, + transport: _opts => + Sentry.makeNodeTransport({ + url: 'https://7fa19397baaf433f919fbe02228d5470@o1137848.ingest.sentry.io/6625302', + recordDroppedEvent: () => { + return undefined; + }, + }), + integrations: [_nodeProfilingIntegration()], + }); + + Sentry.setCurrentClient(client); + client.init(); + + const startProfilingSpy = jest.spyOn(CpuProfilerBindings, 'startProfiling'); + const stopProfilingSpy = jest.spyOn(CpuProfilerBindings, 'stopProfiling'); + + Sentry.profiler.startProfiler(); + expect(startProfilingSpy).toHaveBeenCalledTimes(1); + Sentry.profiler.stopProfiler(); + expect(stopProfilingSpy).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/profiling-node/test/spanProfileUtils.worker.test.ts b/packages/profiling-node/test/spanProfileUtils.worker.test.ts index a119f80292d5..12727aebc954 100644 --- a/packages/profiling-node/test/spanProfileUtils.worker.test.ts +++ b/packages/profiling-node/test/spanProfileUtils.worker.test.ts @@ -9,7 +9,8 @@ jest.setTimeout(10000); import * as Sentry from '@sentry/node'; import type { Transport } from '@sentry/types'; -import { type ProfilingIntegration, _nodeProfilingIntegration } from '../src/integration'; +import { type ProfilingIntegration } from '@sentry/types'; +import { _nodeProfilingIntegration } from '../src/integration'; function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] { const integration = _nodeProfilingIntegration(); @@ -49,7 +50,7 @@ it('worker threads context', () => { }, }); - const integration = client.getIntegrationByName('ProfilingIntegration'); + const integration = client.getIntegrationByName>('ProfilingIntegration'); if (!integration) { throw new Error('Profiling integration not found'); } diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 1022e69ad49e..b100c1e9c26a 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -171,4 +171,5 @@ export type { Metrics, } from './metrics'; export type { ParameterizedString } from './parameterize'; +export type { ContinuousProfiler, ProfilingIntegration, Profiler } from './profiling'; export type { ViewHierarchyData, ViewHierarchyWindow } from './view-hierarchy'; diff --git a/packages/types/src/profiling.ts b/packages/types/src/profiling.ts index 8f5f4cc2e890..9ecba8ced48a 100644 --- a/packages/types/src/profiling.ts +++ b/packages/types/src/profiling.ts @@ -1,6 +1,23 @@ +import type { Client } from './client'; import type { DebugImage } from './debugMeta'; +import type { Integration } from './integration'; import type { MeasurementUnit } from './measurement'; +export interface ContinuousProfiler { + initialize(client: T): void; + start(): void; + stop(): void; +} + +export interface ProfilingIntegration extends Integration { + _profiler: ContinuousProfiler; +} + +export interface Profiler { + startProfiler(): void; + stopProfiler(): void; +} + export type ThreadId = string; export type FrameId = number; export type StackId = number;