Skip to content

Commit 1a19e42

Browse files
authored
♻️ [Profiler] Remove sample-count threshold from profiler discard logic (#4566)
Co-authored-by: thomas.bertet <thomas.bertet@datadoghq.com>
1 parent 49cbec5 commit 1a19e42

5 files changed

Lines changed: 64 additions & 61 deletions

File tree

packages/rum/src/domain/profiling/profiler.spec.ts

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { mockProfiler } from '../../../test'
2828
import type { BrowserProfilerTrace } from '../../types'
2929
import { mockedTrace } from './test-utils/mockedTrace'
3030
import { createRumProfiler } from './profiler'
31-
import type { ProfilerTrace } from './types'
31+
import type { ProfilerTrace, RUMProfilerConfiguration } from './types'
3232
import type { ProfilingContextManager } from './profilingContext'
3333
import { startProfilingContext } from './profilingContext'
3434
import type { ProfileEventPayload } from './transport/assembly'
@@ -56,7 +56,7 @@ describe('profiler', () => {
5656

5757
let lifeCycle = new LifeCycle()
5858

59-
function setupProfiler(currentView?: ViewHistoryEntry) {
59+
function setupProfiler(currentView?: ViewHistoryEntry, profilerConfigOverrides?: Partial<RUMProfilerConfiguration>) {
6060
const sessionManager = createRumSessionManagerMock().setId('session-id-1')
6161
lifeCycle = new LifeCycle()
6262
const hooks = createHooks()
@@ -122,8 +122,8 @@ describe('profiler', () => {
122122
{
123123
sampleIntervalMs: 10,
124124
collectIntervalMs: 60000, // 1min
125-
minNumberOfSamples: 0,
126125
minProfileDurationMs: 0,
126+
...profilerConfigOverrides,
127127
}
128128
)
129129
return {
@@ -933,6 +933,66 @@ describe('profiler', () => {
933933
expect(findTrackedSessionSpy).toHaveBeenCalledWith(expectedStartTime)
934934
})
935935

936+
describe('discard logic', () => {
937+
it('should discard profile when duration is below threshold and there are no long tasks', async () => {
938+
const clock = mockClock()
939+
const { profiler } = setupProfiler(undefined, { minProfileDurationMs: 5000 })
940+
941+
profiler.start()
942+
expect(profiler.isRunning()).toBe(true)
943+
944+
clock.tick(100)
945+
profiler.stop()
946+
expect(profiler.isStopped()).toBe(true)
947+
948+
await waitNextMicrotask()
949+
await waitNextMicrotask()
950+
951+
expect(interceptor.requests.length).toBe(0)
952+
})
953+
954+
it('should send profile when below duration threshold if a long task is present', async () => {
955+
const clock = mockClock()
956+
const { profiler, addLongTask } = setupProfiler(undefined, { minProfileDurationMs: 5000 })
957+
958+
profiler.start()
959+
expect(profiler.isRunning()).toBe(true)
960+
961+
addLongTask({
962+
id: 'long-task-id',
963+
startClocks: clocksNow(),
964+
duration: 50 as Duration,
965+
entryType: RumPerformanceEntryType.LONG_ANIMATION_FRAME,
966+
})
967+
clock.tick(100)
968+
969+
profiler.stop()
970+
expect(profiler.isStopped()).toBe(true)
971+
972+
await waitNextMicrotask()
973+
await waitNextMicrotask()
974+
975+
expect(interceptor.requests.length).toBe(1)
976+
})
977+
978+
it('should send profile when duration threshold is met', async () => {
979+
const clock = mockClock()
980+
const { profiler } = setupProfiler(undefined, { minProfileDurationMs: 100 })
981+
982+
profiler.start()
983+
expect(profiler.isRunning()).toBe(true)
984+
985+
clock.tick(200)
986+
profiler.stop()
987+
expect(profiler.isStopped()).toBe(true)
988+
989+
await waitNextMicrotask()
990+
await waitNextMicrotask()
991+
992+
expect(interceptor.requests.length).toBe(1)
993+
})
994+
})
995+
936996
it('should restart profiling when session expires while paused and then renews', async () => {
937997
const { profiler, profilingContextManager } = setupProfiler()
938998

packages/rum/src/domain/profiling/profiler.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import type {
3131
RUMProfilerConfiguration,
3232
RumProfilerStoppedInstance,
3333
} from './types'
34-
import { getNumberOfSamples } from './utils/getNumberOfSamples'
3534
import type { ProfilingContextManager } from './profilingContext'
3635
import { getCustomOrDefaultViewName } from './utils/getCustomOrDefaultViewName'
3736
import { assembleProfilingPayload } from './transport/assembly'
@@ -43,7 +42,6 @@ export const DEFAULT_RUM_PROFILER_CONFIGURATION: RUMProfilerConfiguration = {
4342
sampleIntervalMs: 10, // Sample stack trace every 10ms
4443
collectIntervalMs: 60000, // Collect data every minute
4544
minProfileDurationMs: 5000, // Require at least 5 seconds of profile data to reduce noise and cost
46-
minNumberOfSamples: 50, // Require at least 50 samples (~500 ms) to report a profile to reduce noise and cost
4745
}
4846

4947
export function createRumProfiler(
@@ -239,9 +237,8 @@ export function createRumProfiler(
239237
const actions = actionHistory.findAll(startClocks.relative, duration)
240238
const vitals = vitalHistory.findAll(startClocks.relative, duration)
241239
const isBelowDurationThreshold = duration < profilerConfiguration.minProfileDurationMs
242-
const isBelowSampleThreshold = getNumberOfSamples(trace.samples) < profilerConfiguration.minNumberOfSamples
243240

244-
if (longTasks.length === 0 && (isBelowDurationThreshold || isBelowSampleThreshold)) {
241+
if (longTasks.length === 0 && isBelowDurationThreshold) {
245242
// Skip very short profiles to reduce noise and cost, but keep them if they contain long tasks.
246243
return
247244
}

packages/rum/src/domain/profiling/types/rumProfiler.types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,4 @@ export interface RUMProfilerConfiguration {
5858
sampleIntervalMs: number // Sample stack trace every x milliseconds (defaults to 10ms for Unix, 16ms on Windows)
5959
collectIntervalMs: number // Interval for collecting RUM Profiles (defaults to 1min)
6060
minProfileDurationMs: number // Minimum duration of a profile for it be sent (defaults to 5s). Profiles shorter than this duration are discarded.
61-
minNumberOfSamples: number // Minimum number of samples to be collected before it can be sent (defaults to 50). Profiles with fewer samples are discarded.
6261
}

packages/rum/src/domain/profiling/utils/getNumberOfSamples.spec.ts

Lines changed: 0 additions & 36 deletions
This file was deleted.

packages/rum/src/domain/profiling/utils/getNumberOfSamples.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)