Skip to content

Commit d11088e

Browse files
committed
refactor: wip
1 parent 7e41c05 commit d11088e

4 files changed

Lines changed: 523 additions & 99 deletions

File tree

packages/utils/src/lib/performance-observer.int.test.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -178,16 +178,4 @@ describe('PerformanceObserverSink', () => {
178178

179179
expect(sink.getWrittenItems()).toHaveLength(2);
180180
});
181-
182-
it('throws error when subscribing with sink that is not open', () => {
183-
const closedSink = new MockSink();
184-
const observer = new PerformanceObserverSink({
185-
sink: closedSink,
186-
encodePerfEntry: encode,
187-
});
188-
189-
expect(() => observer.subscribe()).toThrow(
190-
'Sink MockSink must be opened before subscribing PerformanceObserver',
191-
);
192-
});
193181
});

packages/utils/src/lib/profiler/profiler.int.test.ts

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ describe('Profiler Integration', () => {
169169

170170
it('should create proper DevTools payloads for tracks', () => {
171171
profiler.measure('track-test', (): string => 'result', {
172-
success: (result: string) => ({
172+
success: result => ({
173173
properties: [['result', result]],
174174
tooltipText: 'Track test completed',
175175
}),
@@ -196,8 +196,8 @@ describe('Profiler Integration', () => {
196196
});
197197

198198
it('should merge track defaults with measurement options', () => {
199-
profiler.measure('sync-op', (): string => 'sync-result', {
200-
success: (result: string) => ({
199+
profiler.measure('sync-op', () => 'sync-result', {
200+
success: result => ({
201201
properties: [
202202
['operation', 'sync'],
203203
['result', result],
@@ -279,6 +279,24 @@ describe('Profiler Integration', () => {
279279
]),
280280
);
281281
});
282+
283+
it('should not create performance entries when disabled', async () => {
284+
profiler.setEnabled(false);
285+
286+
const syncResult = profiler.measure('disabled-sync', () => 'sync');
287+
expect(syncResult).toBe('sync');
288+
289+
const asyncResult = profiler.measureAsync(
290+
'disabled-async',
291+
async () => 'async',
292+
);
293+
await expect(asyncResult).resolves.toBe('async');
294+
295+
profiler.marker('disabled-marker');
296+
297+
expect(performance.getEntriesByType('mark')).toHaveLength(0);
298+
expect(performance.getEntriesByType('measure')).toHaveLength(0);
299+
});
282300
});
283301

284302
describe('NodeJS Profiler Integration', () => {
@@ -326,8 +344,8 @@ describe('NodeJS Profiler Integration', () => {
326344
});
327345

328346
it('should disable profiling and close sink', () => {
329-
nodejsProfiler.stop();
330-
expect(nodejsProfiler.isRunning()).toBe(false);
347+
nodejsProfiler.setEnabled(false);
348+
expect(nodejsProfiler.isEnabled()).toBe(false);
331349
expect(mockSink.isClosed()).toBe(true);
332350
expect(mockSink.close).toHaveBeenCalledTimes(1);
333351

@@ -339,10 +357,10 @@ describe('NodeJS Profiler Integration', () => {
339357
});
340358

341359
it('should re-enable profiling correctly', () => {
342-
nodejsProfiler.stop();
343-
nodejsProfiler.start();
360+
nodejsProfiler.setEnabled(false);
361+
nodejsProfiler.setEnabled(true);
344362

345-
expect(nodejsProfiler.isRunning()).toBe(true);
363+
expect(nodejsProfiler.isEnabled()).toBe(true);
346364
expect(mockSink.isClosed()).toBe(false);
347365
expect(mockSink.open).toHaveBeenCalledTimes(2);
348366

@@ -378,15 +396,15 @@ describe('NodeJS Profiler Integration', () => {
378396
enabled: true,
379397
});
380398

381-
const bufferedStats = bufferedProfiler.getStats();
399+
const bufferedStats = bufferedProfiler.stats;
382400
expect(bufferedStats.state).toBe('running');
383401
expect(bufferedStats.walOpen).toBe(true);
384402
expect(bufferedStats.isSubscribed).toBe(true);
385403
expect(bufferedStats.queued).toBe(0);
386404
expect(bufferedStats.dropped).toBe(0);
387405
expect(bufferedStats.written).toBe(0);
388406

389-
bufferedProfiler.stop();
407+
bufferedProfiler.setEnabled(false);
390408
});
391409

392410
it('should return correct getStats with dropped and written counts', () => {
@@ -402,15 +420,15 @@ describe('NodeJS Profiler Integration', () => {
402420

403421
expect(statsProfiler.measure('test-op', () => 'result')).toBe('result');
404422

405-
const stats = statsProfiler.getStats();
423+
const stats = statsProfiler.stats;
406424
expect(stats.state).toBe('running');
407425
expect(stats.walOpen).toBe(true);
408426
expect(stats.isSubscribed).toBe(true);
409427
expect(typeof stats.queued).toBe('number');
410428
expect(typeof stats.dropped).toBe('number');
411429
expect(typeof stats.written).toBe('number');
412430

413-
statsProfiler.stop();
431+
statsProfiler.setEnabled(false);
414432
});
415433

416434
it('should provide comprehensive queue statistics via getStats', () => {
@@ -425,7 +443,7 @@ describe('NodeJS Profiler Integration', () => {
425443
});
426444

427445
// Initial stats should be zero
428-
const initialStats = profiler.getStats();
446+
const initialStats = profiler.stats;
429447
expect(initialStats.state).toBe('running');
430448
expect(initialStats.walOpen).toBe(true);
431449
expect(initialStats.isSubscribed).toBe(true);
@@ -437,7 +455,7 @@ describe('NodeJS Profiler Integration', () => {
437455
profiler.measure('operation-1', () => 'result1');
438456
profiler.measure('operation-2', () => 'result2');
439457

440-
const statsAfterMeasurements = profiler.getStats();
458+
const statsAfterMeasurements = profiler.stats;
441459

442460
// Verify all stats are present and are numbers
443461
expect(typeof statsAfterMeasurements.queued).toBe('number');
@@ -450,9 +468,9 @@ describe('NodeJS Profiler Integration', () => {
450468
expect(statsAfterMeasurements.written).toBeGreaterThanOrEqual(0);
451469

452470
// Disable profiler to flush remaining items
453-
profiler.stop();
471+
profiler.setEnabled(false);
454472

455-
const finalStats = profiler.getStats();
473+
const finalStats = profiler.stats;
456474
expect(finalStats.state).toBe('idle'); // Should be idle
457475
expect(finalStats.walOpen).toBe(false); // WAL should be closed when disabled
458476
expect(finalStats.isSubscribed).toBe(false); // Should not be subscribed when disabled

packages/utils/src/lib/profiler/profiler.ts

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export type ProfilerOptions<T extends ActionTrackConfigs = ActionTrackConfigs> =
6363
*
6464
*/
6565
export class Profiler<T extends ActionTrackConfigs> {
66-
#enabled: boolean;
66+
#enabled: boolean = false;
6767
readonly #defaults: ActionTrackEntryPayload;
6868
readonly tracks: Record<keyof T, ActionTrackEntryPayload> | undefined;
6969
readonly #ctxOf: ReturnType<typeof measureCtx>;
@@ -77,14 +77,14 @@ export class Profiler<T extends ActionTrackConfigs> {
7777
* @param options.track - Default track name for measurements
7878
* @param options.trackGroup - Default track group for organization
7979
* @param options.color - Default color for track entries
80-
* @param options.enabled - Whether profiling is enabled (defaults to CP_PROFILING env var)
80+
* @param options.enabled - Whether profiling is enabled (defaults to false)
8181
*
8282
*/
8383
constructor(options: ProfilerOptions<T>) {
84-
const { tracks, prefix, enabled, ...defaults } = options;
84+
const { tracks, prefix, enabled = false, ...defaults } = options;
8585
const dataType = 'track-entry';
8686

87-
this.setEnabled(enabled ?? isEnvVarEnabled(PROFILER_ENABLED_ENV_VAR));
87+
this.#enabled = enabled;
8888
this.#defaults = { ...defaults, dataType };
8989
this.tracks = tracks
9090
? setupTracks({ ...defaults, dataType }, tracks)
@@ -284,10 +284,11 @@ export class NodejsProfiler<
284284
captureBufferedEntries,
285285
flushThreshold,
286286
maxQueueSize,
287+
enabled,
287288
...profilerOptions
288289
} = options;
289-
290-
super(profilerOptions);
290+
const initialEnabled = enabled ?? isEnvVarEnabled(PROFILER_ENABLED_ENV_VAR);
291+
super({ ...profilerOptions, enabled: initialEnabled });
291292

292293
this.#sink = sink;
293294

@@ -299,7 +300,7 @@ export class NodejsProfiler<
299300
maxQueueSize,
300301
});
301302

302-
if (super.isEnabled()) {
303+
if (initialEnabled) {
303304
this.#transition('running');
304305
}
305306
}
@@ -320,19 +321,14 @@ export class NodejsProfiler<
320321
break;
321322

322323
case 'running->idle':
324+
case 'running->closed':
323325
super.setEnabled(false);
324326
this.#performanceObserverSink.unsubscribe();
325327
this.#sink.close();
326328
break;
327329

328330
case 'idle->closed':
329-
// No resources to clean up when idle
330-
break;
331-
332-
case 'running->closed':
333-
super.setEnabled(false);
334-
this.#performanceObserverSink.unsubscribe();
335-
this.#sink.close();
331+
// No-op, was not open
336332
break;
337333

338334
default:
@@ -342,16 +338,6 @@ export class NodejsProfiler<
342338
this.#state = next;
343339
}
344340

345-
/** Starts profiling (idle → running). */
346-
start(): void {
347-
this.#transition('running');
348-
}
349-
350-
/** Stops profiling (running → idle). */
351-
stop(): void {
352-
this.#transition('idle');
353-
}
354-
355341
/**
356342
* Closes profiler and releases resources. Idempotent, safe for exit handlers.
357343
* **Exit Handler Usage**: Call only this method from process exit handlers.
@@ -366,16 +352,21 @@ export class NodejsProfiler<
366352
}
367353

368354
/** @returns Whether profiler is in 'running' state */
369-
protected isRunning(): boolean {
355+
override isEnabled(): boolean {
370356
return this.#state === 'running';
371357
}
372358

373-
protected activeat(): boolean {
374-
return this.#state === 'running';
359+
/** Enables profiling (start/stop)*/
360+
override setEnabled(enabled: boolean): void {
361+
if (enabled) {
362+
this.#transition('running');
363+
} else {
364+
this.#transition('idle');
365+
}
375366
}
376367

377368
/** @returns Queue statistics and profiling state for monitoring */
378-
getStats() {
369+
get stats() {
379370
return {
380371
...this.#performanceObserverSink.getStats(),
381372
state: this.#state,

0 commit comments

Comments
 (0)