Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(profiling): reinitialize profilerId on explicit stop calls #13681

Merged
merged 10 commits into from
Sep 16, 2024
Merged
107 changes: 73 additions & 34 deletions packages/profiling-node/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;

Expand All @@ -172,15 +172,48 @@ class ContinuousProfiler {
*/
public initialize(client: NodeClient): void {
this._client = client;

// 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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on should return a function that deregisters a callback. We can use that to disable it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I missed that! I'll keep the same implementation I think bc of simplicity, but let me remove that comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it and left a comment for future optimization

}

/**
* 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._client) {
DEBUG_BUILD && logger.log('[Profiling] Failed to start, sentry client was never attached to the profiler.');
return;
}

// Flush any existing chunks before starting a new one.
this._chunkStop();

// Restart the profiler session
this._setupSpanChunkInstrumentation();
this._chunkStart();
}

/**
* Stops the current chunk and flushes the profile to Sentry.
* @returns 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
Expand All @@ -193,20 +226,16 @@ 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 =
getCurrentScope().getPropagationContext().traceId || getIsolationScope().getPropagationContext().traceId;
this._initializeChunk(traceId);
this._startChunkProfiling(this._chunkData!);
this._startChunkProfiling();
}

/**
* Stops the current chunk and flushes the profile to Sentry.
* @returns void
* Stops profiling of the current chunks and flushes the profile to Sentry
*/
public stop(): void {
private _chunkStop(): void {
if (this._chunkData?.timer) {
global.clearTimeout(this._chunkData.timer);
this._chunkData.timer = undefined;
Expand All @@ -223,12 +252,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}.`);
}
Expand All @@ -248,7 +282,7 @@ class ContinuousProfiler {

if (!chunk) {
DEBUG_BUILD && logger.log(`[Profiling] Failed to create profile chunk for: ${this._chunkData.id}`);
this._reset();
this._resetChunkData();
return;
}

Expand All @@ -257,7 +291,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();
}

/**
Expand Down Expand Up @@ -287,29 +321,23 @@ 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();
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}`);

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.
Expand All @@ -323,40 +351,51 @@ 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;
}

this._profilerId = uuid4();
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);
}

/**
* Clear profiling information from global context when a profile is not running.
*/
private _teardownSpanChunkInstrumentation(): void {
this._profilerId = undefined;
const globalScope = getGlobalScope();
globalScope.setContext('profile', {});
}

/**
* 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;
}
Expand All @@ -380,7 +419,7 @@ class ContinuousProfiler {
/**
* Resets the current chunk state.
*/
private _reset(): void {
private _resetChunkData(): void {
this._chunkData = undefined;
}
}
Expand Down
Loading
Loading