From c711cfc41946284022500931f65314b66acfd352 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 13 Aug 2025 14:40:49 +0200 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20manage=20metric=20samp?= =?UTF-8?q?ling=20inside=20telemetry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - allow to apply a telemetry sample rate to telemetry events emitted before the configuration is applied - centralize telemetry sampling draws in telemetry.ts which now exposes `enabledMetrics` - expose replay metrics from recorderApi --- packages/core/src/domain/telemetry/index.ts | 2 +- .../src/domain/telemetry/telemetry.spec.ts | 29 ++++++++++++++--- .../core/src/domain/telemetry/telemetry.ts | 31 ++++++++++++++++--- packages/core/src/index.ts | 1 + packages/rum-core/src/boot/rumPublicApi.ts | 2 ++ packages/rum-core/src/boot/startRum.ts | 12 +++++-- .../domain/startCustomerDataTelemetry.spec.ts | 28 +++++++---------- .../src/domain/startCustomerDataTelemetry.ts | 9 +++--- packages/rum-core/test/noopRecorderApi.ts | 1 + packages/rum-slim/src/boot/stubRecorderApi.ts | 1 + .../rum/src/boot/lazyLoadRecorder.spec.ts | 7 ++--- packages/rum/src/boot/postStartStrategy.ts | 4 +-- packages/rum/src/boot/recorderApi.spec.ts | 8 ++--- packages/rum/src/boot/recorderApi.ts | 8 +++++ packages/rum/src/boot/startRecording.spec.ts | 2 +- packages/rum/src/boot/startRecording.ts | 4 +-- .../rum/src/domain/segmentCollection/index.ts | 2 +- .../startSegmentTelemetry.spec.ts | 21 +++---------- .../startSegmentTelemetry.ts | 8 ++--- .../domain/startRecorderInitTelemetry.spec.ts | 25 +++++---------- .../src/domain/startRecorderInitTelemetry.ts | 13 +++----- 21 files changed, 121 insertions(+), 97 deletions(-) diff --git a/packages/core/src/domain/telemetry/index.ts b/packages/core/src/domain/telemetry/index.ts index 0f0d6d01b5..4ea505a23c 100644 --- a/packages/core/src/domain/telemetry/index.ts +++ b/packages/core/src/domain/telemetry/index.ts @@ -1,4 +1,4 @@ -export type { Telemetry } from './telemetry' +export type { Telemetry, SampleRateByMetric } from './telemetry' export { TelemetryService, addTelemetryDebug, diff --git a/packages/core/src/domain/telemetry/telemetry.spec.ts b/packages/core/src/domain/telemetry/telemetry.spec.ts index 5d5f7a68ac..df1f7aef06 100644 --- a/packages/core/src/domain/telemetry/telemetry.spec.ts +++ b/packages/core/src/domain/telemetry/telemetry.spec.ts @@ -20,6 +20,7 @@ import { startTelemetryCollection, addTelemetryMetrics, addTelemetryDebug, + type SampleRateByMetric, } from './telemetry' import type { TelemetryEvent } from './telemetryEvent.types' import { StatusType, TelemetryType } from './rawTelemetryEvent.types' @@ -27,7 +28,7 @@ import { StatusType, TelemetryType } from './rawTelemetryEvent.types' const NETWORK_METRICS_KIND = 'Network metrics' const PERFORMANCE_METRICS_KIND = 'Performance metrics' -function startAndSpyTelemetry(configuration?: Partial) { +function startAndSpyTelemetry(configuration?: Partial, sampleRateByMetric: SampleRateByMetric = {}) { const observable = new Observable() const events: TelemetryEvent[] = [] @@ -42,7 +43,8 @@ function startAndSpyTelemetry(configuration?: Partial) { ...configuration, } as Configuration, hooks, - observable + observable, + sampleRateByMetric ) return { @@ -157,7 +159,10 @@ describe('telemetry', () => { describe('addTelemetryMetrics', () => { it('should collect metrics when sampled', async () => { - const { getTelemetryEvents } = startAndSpyTelemetry({ telemetrySampleRate: 100 }) + const { getTelemetryEvents } = startAndSpyTelemetry( + { telemetrySampleRate: 100 }, + { [PERFORMANCE_METRICS_KIND]: 100 } + ) addTelemetryMetrics(PERFORMANCE_METRICS_KIND, { speed: 1000 }) @@ -172,8 +177,22 @@ describe('telemetry', () => { ]) }) - it('should not notify metrics when not sampled', async () => { - const { getTelemetryEvents } = startAndSpyTelemetry({ telemetrySampleRate: 0 }) + it('should not notify metrics when telemetry not sampled', async () => { + const { getTelemetryEvents } = startAndSpyTelemetry( + { telemetrySampleRate: 0 }, + { [PERFORMANCE_METRICS_KIND]: 100 } + ) + + addTelemetryMetrics(PERFORMANCE_METRICS_KIND, { speed: 1000 }) + + expect(await getTelemetryEvents()).toEqual([]) + }) + + it('should not notify metrics when metric not sampled', async () => { + const { getTelemetryEvents } = startAndSpyTelemetry( + { telemetrySampleRate: 100 }, + { [PERFORMANCE_METRICS_KIND]: 0 } + ) addTelemetryMetrics(PERFORMANCE_METRICS_KIND, { speed: 1000 }) diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index cf0817808a..b8652c55bd 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -59,6 +59,11 @@ export const enum TelemetryService { export interface Telemetry { stop: () => void enabled: boolean + enabledMetrics: { [metricName: string]: boolean } +} + +export interface SampleRateByMetric { + [metricName: string]: number } const TELEMETRY_EXCLUDED_SITES: string[] = [INTAKE_SITE_US1_FED] @@ -78,17 +83,25 @@ export function startTelemetry( hooks: AbstractHooks, reportError: (error: RawError) => void, pageMayExitObservable: Observable, - createEncoder: (streamId: DeflateEncoderStreamId) => Encoder + createEncoder: (streamId: DeflateEncoderStreamId) => Encoder, + sampleRateByMetric: SampleRateByMetric = {} ): Telemetry { const observable = new Observable() const { stop } = startTelemetryTransport(configuration, reportError, pageMayExitObservable, createEncoder, observable) - const { enabled } = startTelemetryCollection(telemetryService, configuration, hooks, observable) + const { enabled, enabledMetrics } = startTelemetryCollection( + telemetryService, + configuration, + hooks, + observable, + sampleRateByMetric + ) return { stop, enabled, + enabledMetrics, } } @@ -96,7 +109,8 @@ export function startTelemetryCollection( telemetryService: TelemetryService, configuration: Configuration, hooks: AbstractHooks, - observable: Observable + observable: Observable, + sampleRateByMetric: SampleRateByMetric ) { const alreadySentEventsByKind: Record> = {} @@ -109,10 +123,18 @@ export function startTelemetryCollection( [TelemetryType.USAGE]: telemetryEnabled && performDraw(configuration.telemetryUsageSampleRate), } + const telemetryEnabledPerMetrics: { [metricName: string]: boolean } = {} + Object.keys(sampleRateByMetric).forEach((metricName) => { + telemetryEnabledPerMetrics[metricName] = telemetryEnabled && performDraw(sampleRateByMetric[metricName]) + }) + const runtimeEnvInfo = getRuntimeEnvInfo() const telemetryObservable = getTelemetryObservable() telemetryObservable.subscribe(({ rawEvent, kind }) => { - if (!telemetryEnabledPerType[rawEvent.type!]) { + if ( + !telemetryEnabledPerType[rawEvent.type!] || + (kind in telemetryEnabledPerMetrics && !telemetryEnabledPerMetrics[kind]) + ) { return } @@ -154,6 +176,7 @@ export function startTelemetryCollection( return { enabled: telemetryEnabled, + enabledMetrics: telemetryEnabledPerMetrics, } function toTelemetryEvent( diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c41486f253..a0cb419d4a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -40,6 +40,7 @@ export type { TelemetryUsageEvent, RawTelemetryUsage, RawTelemetryUsageFeature, + SampleRateByMetric, } from './domain/telemetry' export { startTelemetry, diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index c54ecc1aa6..44ec2cddd6 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -13,6 +13,7 @@ import type { Account, RumInternalContext, Telemetry, + SampleRateByMetric, } from '@datadog/browser-core' import { ContextManagerMethod, @@ -443,6 +444,7 @@ export interface RecorderApi { isRecording: () => boolean getReplayStats: (viewId: string) => ReplayStats | undefined getSessionReplayLink: () => string | undefined + getTelemetrySampleRateByMetric: (configuration: RumConfiguration) => SampleRateByMetric | undefined } export interface ProfilerApi { diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 2bcc60b8b1..cdd1309691 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -37,7 +37,7 @@ import { createLocationChangeObservable } from '../browser/locationChangeObserva import type { RumConfiguration } from '../domain/configuration' import type { ViewOptions } from '../domain/view/trackViews' import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' -import { startCustomerDataTelemetry } from '../domain/startCustomerDataTelemetry' +import { startCustomerDataTelemetry, CUSTOMER_DATA_METRIC_NAME } from '../domain/startCustomerDataTelemetry' import type { PageStateHistory } from '../domain/contexts/pageStateHistory' import { startPageStateHistory } from '../domain/contexts/pageStateHistory' import { startDisplayContext } from '../domain/contexts/displayContext' @@ -94,13 +94,19 @@ export function startRum( }) cleanupTasks.push(() => pageMayExitSubscription.unsubscribe()) + const sampleRateByMetric = { + [CUSTOMER_DATA_METRIC_NAME]: configuration.customerDataTelemetrySampleRate, + ...recorderApi.getTelemetrySampleRateByMetric(configuration), + } + const telemetry = startTelemetry( TelemetryService.RUM, configuration, hooks, reportError, pageMayExitObservable, - createEncoder + createEncoder, + sampleRateByMetric ) cleanupTasks.push(telemetry.stop) @@ -118,7 +124,7 @@ export function startRum( createEncoder ) cleanupTasks.push(() => batch.stop()) - startCustomerDataTelemetry(configuration, telemetry, lifeCycle, batch.flushController.flushObservable) + startCustomerDataTelemetry(telemetry, lifeCycle, batch.flushController.flushObservable) } else { startRumEventBridge(lifeCycle) } diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index 1ef4bdf796..bb6c301296 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -2,12 +2,14 @@ import type { FlushEvent, Context, Telemetry } from '@datadog/browser-core' import { Observable, resetExperimentalFeatures } from '@datadog/browser-core' import type { Clock, MockTelemetry } from '@datadog/browser-core/test' import { mockClock, startMockTelemetry } from '@datadog/browser-core/test' -import { mockRumConfiguration } from '../../test' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' import { LifeCycle, LifeCycleEventType } from './lifeCycle' -import { MEASURES_PERIOD_DURATION, startCustomerDataTelemetry } from './startCustomerDataTelemetry' -import type { RumConfiguration } from './configuration' +import { + MEASURES_PERIOD_DURATION, + startCustomerDataTelemetry, + CUSTOMER_DATA_METRIC_NAME, +} from './startCustomerDataTelemetry' describe('customerDataTelemetry', () => { let clock: Clock @@ -17,12 +19,6 @@ describe('customerDataTelemetry', () => { let lifeCycle: LifeCycle const viewEvent = { type: RumEventType.VIEW } as RumEvent & Context - const config: Partial = { - telemetrySampleRate: 100, - customerDataTelemetrySampleRate: 100, - maxTelemetryEventsPerPage: 2, - } - function generateBatch({ eventNumber, batchBytesCount = 1, @@ -46,15 +42,18 @@ describe('customerDataTelemetry', () => { }) } - function setupCustomerTelemetryCollection(partialConfig: Partial = config) { - const configuration = mockRumConfiguration(partialConfig) + function setupCustomerTelemetryCollection(telemetryEnabled: boolean = true) { batchFlushObservable = new Observable() lifeCycle = new LifeCycle() fakeContextBytesCount = 1 telemetry = startMockTelemetry() - startCustomerDataTelemetry(configuration, { enabled: true } as Telemetry, lifeCycle, batchFlushObservable) + startCustomerDataTelemetry( + { enabledMetrics: { [CUSTOMER_DATA_METRIC_NAME]: telemetryEnabled } } as unknown as Telemetry, + lifeCycle, + batchFlushObservable + ) } beforeEach(() => { @@ -132,10 +131,7 @@ describe('customerDataTelemetry', () => { }) it('should not collect customer data telemetry when telemetry disabled', async () => { - setupCustomerTelemetryCollection({ - telemetrySampleRate: 100, - customerDataTelemetrySampleRate: 0, - }) + setupCustomerTelemetryCollection(false) generateBatch({ eventNumber: 1 }) clock.tick(MEASURES_PERIOD_DURATION) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index ff25065344..c15761b272 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -1,10 +1,10 @@ import type { Context, FlushEvent, Observable, Telemetry } from '@datadog/browser-core' -import { performDraw, ONE_SECOND, addTelemetryMetrics, setInterval } from '@datadog/browser-core' -import type { RumConfiguration } from './configuration' +import { ONE_SECOND, addTelemetryMetrics, setInterval } from '@datadog/browser-core' import type { LifeCycle } from './lifeCycle' import { LifeCycleEventType } from './lifeCycle' export const MEASURES_PERIOD_DURATION = 10 * ONE_SECOND +export const CUSTOMER_DATA_METRIC_NAME = 'Customer data measures' interface Measure extends Context { min: number @@ -22,12 +22,11 @@ let currentPeriodMeasures: CurrentPeriodMeasures let batchHasRumEvent: boolean export function startCustomerDataTelemetry( - configuration: RumConfiguration, telemetry: Telemetry, lifeCycle: LifeCycle, batchFlushObservable: Observable ) { - const customerDataTelemetryEnabled = telemetry.enabled && performDraw(configuration.customerDataTelemetrySampleRate) + const customerDataTelemetryEnabled = telemetry.enabledMetrics[CUSTOMER_DATA_METRIC_NAME] if (!customerDataTelemetryEnabled) { return } @@ -61,7 +60,7 @@ function sendCurrentPeriodMeasures() { return } - addTelemetryMetrics('Customer data measures', currentPeriodMeasures) + addTelemetryMetrics(CUSTOMER_DATA_METRIC_NAME, currentPeriodMeasures) initCurrentPeriodMeasures() } diff --git a/packages/rum-core/test/noopRecorderApi.ts b/packages/rum-core/test/noopRecorderApi.ts index f88518d773..ac46cee969 100644 --- a/packages/rum-core/test/noopRecorderApi.ts +++ b/packages/rum-core/test/noopRecorderApi.ts @@ -8,4 +8,5 @@ export const noopRecorderApi: RecorderApi = { onRumStart: noop, getReplayStats: () => undefined, getSessionReplayLink: () => undefined, + getTelemetrySampleRateByMetric: () => undefined, } diff --git a/packages/rum-slim/src/boot/stubRecorderApi.ts b/packages/rum-slim/src/boot/stubRecorderApi.ts index a98c21b4d1..9e3d21f285 100644 --- a/packages/rum-slim/src/boot/stubRecorderApi.ts +++ b/packages/rum-slim/src/boot/stubRecorderApi.ts @@ -13,5 +13,6 @@ export function makeRecorderApiStub() { isRecording: () => false, getReplayStats: () => undefined, getSessionReplayLink: () => getSessionReplayLinkStrategy(), + getTelemetrySampleRateByMetric: () => undefined, } } diff --git a/packages/rum/src/boot/lazyLoadRecorder.spec.ts b/packages/rum/src/boot/lazyLoadRecorder.spec.ts index d62a717c4e..83b93e6fec 100644 --- a/packages/rum/src/boot/lazyLoadRecorder.spec.ts +++ b/packages/rum/src/boot/lazyLoadRecorder.spec.ts @@ -6,9 +6,10 @@ import type { MockTelemetry } from '@datadog/browser-core/test' import { registerCleanupTask, startMockTelemetry, wait } from '@datadog/browser-core/test' import { createRumSessionManagerMock, mockRumConfiguration, mockViewHistory } from '../../../rum-core/test' import type { CreateDeflateWorker } from '../domain/deflate' -import { MockWorker } from '../../test' import { resetDeflateWorkerState } from '../domain/deflate' +import { MockWorker } from '../../test' import * as replayStats from '../domain/replayStats' +import { RECORDER_INIT_METRICS_TELEMETRY_NAME } from '../domain/startRecorderInitTelemetry' import { makeRecorderApi } from './recorderApi' import type { StartRecording } from './postStartStrategy' import { lazyLoadRecorder } from './lazyLoadRecorder' @@ -67,9 +68,7 @@ describe('lazyLoadRecorder', () => { }) const configuration = mockRumConfiguration({ - replayTelemetrySampleRate: 100, startSessionReplayRecordingManually: startSessionReplayRecordingManually ?? false, - telemetrySampleRate: 100, }) recorderApi = makeRecorderApi(loadRecorderSpy, createDeflateWorkerSpy) @@ -80,7 +79,7 @@ describe('lazyLoadRecorder', () => { sessionManager ?? createRumSessionManagerMock().setId('1234'), mockViewHistory(), worker, - { enabled: true } as Telemetry + { enabled: true, enabledMetrics: { [RECORDER_INIT_METRICS_TELEMETRY_NAME]: true } } as unknown as Telemetry ) } diff --git a/packages/rum/src/boot/postStartStrategy.ts b/packages/rum/src/boot/postStartStrategy.ts index d1a8878a23..f55afb881a 100644 --- a/packages/rum/src/boot/postStartStrategy.ts +++ b/packages/rum/src/boot/postStartStrategy.ts @@ -7,8 +7,8 @@ import type { RumSession, } from '@datadog/browser-rum-core' import { LifeCycleEventType, SessionReplayState } from '@datadog/browser-rum-core' -import { asyncRunOnReadyState, monitorError, Observable } from '@datadog/browser-core' import type { Telemetry, DeflateEncoder } from '@datadog/browser-core' +import { asyncRunOnReadyState, monitorError, Observable } from '@datadog/browser-core' import { getSessionReplayLink } from '../domain/getSessionReplayLink' import { startRecorderInitTelemetry } from '../domain/startRecorderInitTelemetry' import type { startRecording } from './startRecording' @@ -69,7 +69,7 @@ export function createPostStartStrategy( }) const observable = new Observable() - startRecorderInitTelemetry(configuration, telemetry, observable) + startRecorderInitTelemetry(telemetry, observable) const doStart = async (forced: boolean) => { observable.notify({ type: 'start', forced }) diff --git a/packages/rum/src/boot/recorderApi.spec.ts b/packages/rum/src/boot/recorderApi.spec.ts index 6513d99a13..99b8bcabb1 100644 --- a/packages/rum/src/boot/recorderApi.spec.ts +++ b/packages/rum/src/boot/recorderApi.spec.ts @@ -18,10 +18,10 @@ import { mockViewHistory, } from '../../../rum-core/test' import type { CreateDeflateWorker } from '../domain/deflate' -import { MockWorker } from '../../test' import { resetDeflateWorkerState } from '../domain/deflate' +import { MockWorker } from '../../test' import * as replayStats from '../domain/replayStats' -import type { RecorderInitMetrics } from '../domain/startRecorderInitTelemetry' +import { type RecorderInitMetrics, RECORDER_INIT_METRICS_TELEMETRY_NAME } from '../domain/startRecorderInitTelemetry' import { makeRecorderApi } from './recorderApi' import type { StartRecording } from './postStartStrategy' @@ -58,9 +58,7 @@ describe('makeRecorderApi', () => { }) const configuration = mockRumConfiguration({ - replayTelemetrySampleRate: 100, startSessionReplayRecordingManually: startSessionReplayRecordingManually ?? false, - telemetrySampleRate: 100, }) recorderApi = makeRecorderApi(loadRecorderSpy, createDeflateWorkerSpy) @@ -71,7 +69,7 @@ describe('makeRecorderApi', () => { sessionManager ?? createRumSessionManagerMock().setId('1234'), mockViewHistory(), worker, - { enabled: true } as Telemetry + { enabled: true, enabledMetrics: { [RECORDER_INIT_METRICS_TELEMETRY_NAME]: true } } as unknown as Telemetry ) } diff --git a/packages/rum/src/boot/recorderApi.ts b/packages/rum/src/boot/recorderApi.ts index 2eb7089c03..5e1e692523 100644 --- a/packages/rum/src/boot/recorderApi.ts +++ b/packages/rum/src/boot/recorderApi.ts @@ -22,6 +22,8 @@ import { getDeflateWorkerStatus, startDeflateWorker, } from '../domain/deflate' +import { SEGMENT_METRICS_TELEMETRY_NAME } from '../domain/segmentCollection' +import { RECORDER_INIT_METRICS_TELEMETRY_NAME } from '../domain/startRecorderInitTelemetry' import { isBrowserSupported } from './isBrowserSupported' import type { StartRecording } from './postStartStrategy' import { createPostStartStrategy } from './postStartStrategy' @@ -39,6 +41,7 @@ export function makeRecorderApi( onRumStart: noop, isRecording: () => false, getSessionReplayLink: () => undefined, + getTelemetrySampleRateByMetric: () => undefined, } } @@ -77,6 +80,11 @@ export function makeRecorderApi( getReplayStats: (viewId) => getDeflateWorkerStatus() === DeflateWorkerStatus.Initialized ? getReplayStatsImpl(viewId) : undefined, + + getTelemetrySampleRateByMetric: (configuration: RumConfiguration) => ({ + [SEGMENT_METRICS_TELEMETRY_NAME]: configuration.replayTelemetrySampleRate, + [RECORDER_INIT_METRICS_TELEMETRY_NAME]: configuration.replayTelemetrySampleRate, + }), } function onRumStart( diff --git a/packages/rum/src/boot/startRecording.spec.ts b/packages/rum/src/boot/startRecording.spec.ts index 969884d491..653b3f23d3 100644 --- a/packages/rum/src/boot/startRecording.spec.ts +++ b/packages/rum/src/boot/startRecording.spec.ts @@ -42,7 +42,7 @@ describe('startRecording', () => { const viewHistory = startViewHistory(lifeCycle) initialView(lifeCycle) - const mockTelemetry = { enabled: true } as Telemetry + const mockTelemetry = { enabled: true, enabledMetrics: {} } as Telemetry const recording = startRecording( lifeCycle, diff --git a/packages/rum/src/boot/startRecording.ts b/packages/rum/src/boot/startRecording.ts index 9061d938c2..1aaef2d2e2 100644 --- a/packages/rum/src/boot/startRecording.ts +++ b/packages/rum/src/boot/startRecording.ts @@ -5,10 +5,10 @@ import { LifeCycleEventType } from '@datadog/browser-rum-core' import type { SerializationStats } from '../domain/record' import { record } from '../domain/record' +import type { ReplayPayload } from '../domain/segmentCollection' import { startSegmentCollection, SEGMENT_BYTES_LIMIT, startSegmentTelemetry } from '../domain/segmentCollection' import type { BrowserRecord } from '../types' import { startRecordBridge } from '../domain/startRecordBridge' -import type { ReplayPayload } from '../domain/segmentCollection' export function startRecording( lifeCycle: LifeCycle, @@ -43,7 +43,7 @@ export function startRecording( addRecord = segmentCollection.addRecord cleanupTasks.push(segmentCollection.stop) - const segmentTelemetry = startSegmentTelemetry(configuration, telemetry, replayRequest.observable) + const segmentTelemetry = startSegmentTelemetry(telemetry, replayRequest.observable) cleanupTasks.push(segmentTelemetry.stop) } else { ;({ addRecord } = startRecordBridge(viewHistory)) diff --git a/packages/rum/src/domain/segmentCollection/index.ts b/packages/rum/src/domain/segmentCollection/index.ts index 364f174ac9..f69149fa6a 100644 --- a/packages/rum/src/domain/segmentCollection/index.ts +++ b/packages/rum/src/domain/segmentCollection/index.ts @@ -1,4 +1,4 @@ export { startSegmentCollection, setSegmentBytesLimit } from './segmentCollection' export { SEGMENT_BYTES_LIMIT } from './segmentCollection' export type { BrowserSegmentMetadataAndSegmentSizes, ReplayPayload } from './buildReplayPayload' -export { startSegmentTelemetry } from './startSegmentTelemetry' +export { startSegmentTelemetry, SEGMENT_METRICS_TELEMETRY_NAME } from './startSegmentTelemetry' diff --git a/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.spec.ts b/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.spec.ts index a312da277e..1403096411 100644 --- a/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.spec.ts +++ b/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.spec.ts @@ -2,10 +2,8 @@ import type { Telemetry, HttpRequestEvent, BandwidthStats } from '@datadog/brows import { Observable } from '@datadog/browser-core' import type { MockTelemetry } from '@datadog/browser-core/test' import { registerCleanupTask } from '@datadog/browser-core/test' -import type { RumConfiguration } from '@datadog/browser-rum-core' -import { mockRumConfiguration } from '@datadog/browser-rum-core/test' import { startMockTelemetry } from '../../../../core/test' -import { startSegmentTelemetry } from './startSegmentTelemetry' +import { startSegmentTelemetry, SEGMENT_METRICS_TELEMETRY_NAME } from './startSegmentTelemetry' import type { ReplayPayload } from './buildReplayPayload' describe('segmentTelemetry', () => { @@ -13,12 +11,6 @@ describe('segmentTelemetry', () => { let telemetry: MockTelemetry let stopSegmentTelemetry: (() => void) | undefined - const config: Partial = { - maxTelemetryEventsPerPage: 2, - replayTelemetrySampleRate: 100, - telemetrySampleRate: 100, - } - function generateReplayRequest({ result, isFullSnapshot, @@ -50,13 +42,11 @@ describe('segmentTelemetry', () => { requestObservable.notify({ type: result, bandwidth, payload }) } - function setupSegmentTelemetryCollection(partialConfig: Partial = config) { - const configuration = mockRumConfiguration(partialConfig) + function setupSegmentTelemetryCollection(telemetryEnabled: boolean = true) { requestObservable = new Observable() telemetry = startMockTelemetry() ;({ stop: stopSegmentTelemetry } = startSegmentTelemetry( - configuration, - { enabled: true } as Telemetry, + { enabledMetrics: { [SEGMENT_METRICS_TELEMETRY_NAME]: telemetryEnabled } } as unknown as Telemetry, requestObservable )) registerCleanupTask(stopSegmentTelemetry) @@ -151,10 +141,7 @@ describe('segmentTelemetry', () => { }) it('should not collect segment when telemetry disabled', async () => { - setupSegmentTelemetryCollection({ - replayTelemetrySampleRate: 0, - telemetrySampleRate: 100, - }) + setupSegmentTelemetryCollection(false) generateReplayRequest({ result: 'success', isFullSnapshot: true }) expect(await telemetry.hasEvents()).toBe(false) }) diff --git a/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.ts b/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.ts index 7241c94511..0c445e56c5 100644 --- a/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.ts +++ b/packages/rum/src/domain/segmentCollection/startSegmentTelemetry.ts @@ -1,9 +1,8 @@ import type { BandwidthStats, Context, HttpRequestEvent, Observable, Telemetry } from '@datadog/browser-core' -import { performDraw, addTelemetryMetrics, noop } from '@datadog/browser-core' -import type { RumConfiguration } from '@datadog/browser-rum-core' +import { addTelemetryMetrics, noop } from '@datadog/browser-core' import type { ReplayPayload } from './buildReplayPayload' -const SEGMENT_METRICS_TELEMETRY_NAME = 'Segment network request metrics' +export const SEGMENT_METRICS_TELEMETRY_NAME = 'Segment network request metrics' interface SegmentMetrics extends Context { cssText: { @@ -30,11 +29,10 @@ interface SegmentMetrics extends Context { } export function startSegmentTelemetry( - configuration: RumConfiguration, telemetry: Telemetry, requestObservable: Observable> ) { - const segmentTelemetryEnabled = telemetry.enabled && performDraw(configuration.replayTelemetrySampleRate) + const segmentTelemetryEnabled = telemetry.enabledMetrics[SEGMENT_METRICS_TELEMETRY_NAME] if (!segmentTelemetryEnabled) { return { stop: noop } } diff --git a/packages/rum/src/domain/startRecorderInitTelemetry.spec.ts b/packages/rum/src/domain/startRecorderInitTelemetry.spec.ts index 1ccef4c071..935d06abd8 100644 --- a/packages/rum/src/domain/startRecorderInitTelemetry.spec.ts +++ b/packages/rum/src/domain/startRecorderInitTelemetry.spec.ts @@ -2,28 +2,22 @@ import type { Telemetry, RawTelemetryEvent } from '@datadog/browser-core' import { Observable } from '@datadog/browser-core' import type { MockTelemetry } from '@datadog/browser-core/test' import { registerCleanupTask, startMockTelemetry } from '@datadog/browser-core/test' -import type { RumConfiguration } from '@datadog/browser-rum-core' -import { mockRumConfiguration } from '@datadog/browser-rum-core/test' import type { RecorderInitEvent } from '../boot/postStartStrategy' -import type { RecorderInitMetrics } from './startRecorderInitTelemetry' -import { startRecorderInitTelemetry } from './startRecorderInitTelemetry' +import { + type RecorderInitMetrics, + RECORDER_INIT_METRICS_TELEMETRY_NAME, + startRecorderInitTelemetry, +} from './startRecorderInitTelemetry' describe('startRecorderInitTelemetry', () => { let observable: Observable let telemetry: MockTelemetry - const config: Partial = { - replayTelemetrySampleRate: 100, - telemetrySampleRate: 100, - } - - function startRecorderInitTelemetryCollection(partialConfig: Partial = config) { - const configuration = mockRumConfiguration(partialConfig) + function startRecorderInitTelemetryCollection(telemetryEnabled: boolean = true) { observable = new Observable() telemetry = startMockTelemetry() const { stop: stopRecorderInitTelemetry } = startRecorderInitTelemetry( - configuration, - { enabled: true } as Telemetry, + { enabledMetrics: { [RECORDER_INIT_METRICS_TELEMETRY_NAME]: telemetryEnabled } } as unknown as Telemetry, observable ) registerCleanupTask(stopRecorderInitTelemetry) @@ -139,10 +133,7 @@ describe('startRecorderInitTelemetry', () => { }) it('should not collect recorder init metrics telemetry when telemetry is disabled', async () => { - startRecorderInitTelemetryCollection({ - telemetrySampleRate: 100, - initialViewMetricsTelemetrySampleRate: 0, - }) + startRecorderInitTelemetryCollection(false) observable.notify({ type: 'start', forced: false }) observable.notify({ type: 'recorder-settled' }) observable.notify({ type: 'document-ready' }) diff --git a/packages/rum/src/domain/startRecorderInitTelemetry.ts b/packages/rum/src/domain/startRecorderInitTelemetry.ts index 6281edde37..40cf09ca4c 100644 --- a/packages/rum/src/domain/startRecorderInitTelemetry.ts +++ b/packages/rum/src/domain/startRecorderInitTelemetry.ts @@ -1,9 +1,8 @@ import type { Context, Duration, Telemetry, Observable, TimeStamp } from '@datadog/browser-core' -import { performDraw, addTelemetryMetrics, noop, timeStampNow, elapsed } from '@datadog/browser-core' -import type { RumConfiguration } from '@datadog/browser-rum-core' +import { addTelemetryMetrics, noop, timeStampNow, elapsed } from '@datadog/browser-core' import type { RecorderInitEvent } from '../boot/postStartStrategy' -const RECORDER_INIT_METRICS_TELEMETRY_NAME = 'Recorder init metrics' +export const RECORDER_INIT_METRICS_TELEMETRY_NAME = 'Recorder init metrics' type RecorderInitResult = 'aborted' | 'deflate-encoder-load-failed' | 'recorder-load-failed' | 'succeeded' @@ -15,12 +14,8 @@ export interface RecorderInitMetrics extends Context { waitForDocReadyDuration: number | undefined } -export function startRecorderInitTelemetry( - configuration: RumConfiguration, - telemetry: Telemetry, - observable: Observable -) { - const recorderInitTelemetryEnabled = telemetry.enabled && performDraw(configuration.replayTelemetrySampleRate) +export function startRecorderInitTelemetry(telemetry: Telemetry, observable: Observable) { + const recorderInitTelemetryEnabled = telemetry.enabledMetrics[RECORDER_INIT_METRICS_TELEMETRY_NAME] if (!recorderInitTelemetryEnabled) { return { stop: noop } } From 47f5cedc4658d224e292876ee94aaa93ebc6d062 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 13 Aug 2025 15:32:48 +0200 Subject: [PATCH 2/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20share=20context=20to?= =?UTF-8?q?=20access=20remote=20configuration=20metrics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit preliminary refactoring, metrics introduced in next commit --- .../configuration/remoteConfiguration.ts | 196 +++++++++--------- 1 file changed, 100 insertions(+), 96 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 94c83a5e03..890678951e 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -64,30 +64,110 @@ export function applyRemoteConfiguration( } }) return appliedConfiguration -} -function resolveConfigurationProperty(property: unknown): unknown { - if (Array.isArray(property)) { - return property.map(resolveConfigurationProperty) - } - if (isObject(property)) { - if (isSerializedOption(property)) { - const type = property.rcSerializedType - switch (type) { - case 'string': - return property.value - case 'regex': - return resolveRegex(property.value) - case 'dynamic': - return resolveDynamicOption(property) - default: - display.error(`Unsupported remote configuration: "rcSerializedType": "${type as string}"`) - return + // share context to access metrics + + function resolveConfigurationProperty(property: unknown): unknown { + if (Array.isArray(property)) { + return property.map(resolveConfigurationProperty) + } + if (isObject(property)) { + if (isSerializedOption(property)) { + const type = property.rcSerializedType + switch (type) { + case 'string': + return property.value + case 'regex': + return resolveRegex(property.value) + case 'dynamic': + return resolveDynamicOption(property) + default: + display.error(`Unsupported remote configuration: "rcSerializedType": "${type as string}"`) + return + } } + return mapValues(property, resolveConfigurationProperty) + } + return property + } + + function resolveContextProperty( + contextManager: ReturnType, + contextItems: ContextItem[] + ) { + contextItems.forEach(({ key, value }) => { + contextManager.setContextProperty(key, resolveConfigurationProperty(value)) + }) + } + + function resolveDynamicOption(property: DynamicOption) { + const strategy = property.strategy + let resolvedValue: unknown + switch (strategy) { + case 'cookie': + resolvedValue = resolveCookieValue(property) + break + case 'dom': + resolvedValue = resolveDomValue(property) + break + case 'js': + resolvedValue = resolveJsValue(property) + break + default: + display.error(`Unsupported remote configuration: "strategy": "${strategy as string}"`) + return + } + const extractor = property.extractor + if (extractor !== undefined && typeof resolvedValue === 'string') { + return extractValue(extractor, resolvedValue) + } + return resolvedValue + } + + function resolveCookieValue({ name }: { name: string }) { + return getCookie(name) + } + + function resolveDomValue({ selector, attribute }: { selector: string; attribute?: string }) { + let element: Element | null + try { + element = document.querySelector(selector) + } catch { + element = null + display.error(`Invalid selector in the remote configuration: '${selector}'`) } - return mapValues(property, resolveConfigurationProperty) + if (element === null || isForbidden(element, attribute)) { + return + } + const domValue = attribute !== undefined ? element.getAttribute(attribute) : element.textContent + return domValue ?? undefined + } + + function isForbidden(element: Element, attribute: string | undefined) { + return element.getAttribute('type') === 'password' && attribute === 'value' + } + + function resolveJsValue({ path }: { path: string }): unknown { + let current = window as unknown as { [key: string]: unknown } + + const pathParts = parseJsonPath(path) + if (pathParts.length === 0) { + display.error(`Invalid JSON path in the remote configuration: '${path}'`) + return + } + for (const pathPart of pathParts) { + if (!(pathPart in current)) { + return + } + try { + current = current[pathPart] as { [key: string]: unknown } + } catch (e) { + display.error(`Error accessing: '${path}'`, e) + return + } + } + return current } - return property } function isObject(property: unknown): property is { [key: string]: unknown } { @@ -106,82 +186,6 @@ function resolveRegex(pattern: string): RegExp | undefined { } } -function resolveContextProperty(contextManager: ReturnType, contextItems: ContextItem[]) { - contextItems.forEach(({ key, value }) => { - contextManager.setContextProperty(key, resolveConfigurationProperty(value)) - }) -} - -function resolveDynamicOption(property: DynamicOption) { - const strategy = property.strategy - let resolvedValue: unknown - switch (strategy) { - case 'cookie': - resolvedValue = resolveCookieValue(property) - break - case 'dom': - resolvedValue = resolveDomValue(property) - break - case 'js': - resolvedValue = resolveJsValue(property) - break - default: - display.error(`Unsupported remote configuration: "strategy": "${strategy as string}"`) - return - } - const extractor = property.extractor - if (extractor !== undefined && typeof resolvedValue === 'string') { - return extractValue(extractor, resolvedValue) - } - return resolvedValue -} - -function resolveCookieValue({ name }: { name: string }) { - return getCookie(name) -} - -function resolveDomValue({ selector, attribute }: { selector: string; attribute?: string }) { - let element: Element | null - try { - element = document.querySelector(selector) - } catch { - element = null - display.error(`Invalid selector in the remote configuration: '${selector}'`) - } - if (element === null || isForbidden(element, attribute)) { - return - } - const domValue = attribute !== undefined ? element.getAttribute(attribute) : element.textContent - return domValue ?? undefined -} - -function isForbidden(element: Element, attribute: string | undefined) { - return element.getAttribute('type') === 'password' && attribute === 'value' -} - -function resolveJsValue({ path }: { path: string }): unknown { - let current = window as unknown as { [key: string]: unknown } - - const pathParts = parseJsonPath(path) - if (pathParts.length === 0) { - display.error(`Invalid JSON path in the remote configuration: '${path}'`) - return - } - for (const pathPart of pathParts) { - if (!(pathPart in current)) { - return - } - try { - current = current[pathPart] as { [key: string]: unknown } - } catch (e) { - display.error(`Error accessing: '${path}'`, e) - return - } - } - - return current -} - function extractValue(extractor: SerializedRegex, candidate: string) { const resolvedExtractor = resolveRegex(extractor.value) if (resolvedExtractor === undefined) { From d3a4b2320750c4119f419c1de6a38edc2759d605 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Mon, 15 Sep 2025 11:56:53 +0200 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=94=8Acollect=20remote=20configuratio?= =?UTF-8?q?n=20metrics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add new telemetry metric - collect data about results of rc fetch and dynamic options resolution --- packages/rum-core/src/boot/startRum.ts | 2 + .../src/domain/configuration/configuration.ts | 2 + .../configuration/remoteConfiguration.spec.ts | 89 ++++++++++++-- .../configuration/remoteConfiguration.ts | 116 ++++++++++++++---- 4 files changed, 176 insertions(+), 33 deletions(-) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index cdd1309691..fd9c20b49b 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -35,6 +35,7 @@ import { startRumEventBridge } from '../transport/startRumEventBridge' import { startUrlContexts } from '../domain/contexts/urlContexts' import { createLocationChangeObservable } from '../browser/locationChangeObservable' import type { RumConfiguration } from '../domain/configuration' +import { REMOTE_CONFIGURATION_METRIC_NAME } from '../domain/configuration' import type { ViewOptions } from '../domain/view/trackViews' import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' import { startCustomerDataTelemetry, CUSTOMER_DATA_METRIC_NAME } from '../domain/startCustomerDataTelemetry' @@ -96,6 +97,7 @@ export function startRum( const sampleRateByMetric = { [CUSTOMER_DATA_METRIC_NAME]: configuration.customerDataTelemetrySampleRate, + [REMOTE_CONFIGURATION_METRIC_NAME]: configuration.remoteConfigurationTelemetrySampleRate, ...recorderApi.getTelemetrySampleRateByMetric(configuration), } diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index b1a103b809..776ba502f0 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -249,6 +249,7 @@ export interface RumConfiguration extends Configuration { customerDataTelemetrySampleRate: number initialViewMetricsTelemetrySampleRate: number replayTelemetrySampleRate: number + remoteConfigurationTelemetrySampleRate: number traceContextInjection: TraceContextInjection plugins: RumPlugin[] trackFeatureFlagsForEvents: FeatureFlagsForEvents[] @@ -322,6 +323,7 @@ export function validateAndBuildRumConfiguration( customerDataTelemetrySampleRate: 1, initialViewMetricsTelemetrySampleRate: 1, replayTelemetrySampleRate: 1, + remoteConfigurationTelemetrySampleRate: 1, traceContextInjection: objectHasValue(TraceContextInjection, initConfiguration.traceContextInjection) ? initConfiguration.traceContextInjection : TraceContextInjection.SAMPLED, diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index 513f20716d..9d81ef5d68 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -10,7 +10,7 @@ import { import { interceptRequests, registerCleanupTask } from '@datadog/browser-core/test' import { appendElement } from '../../../test' import type { RumInitConfiguration } from './configuration' -import type { RumRemoteConfiguration } from './remoteConfiguration' +import type { RumRemoteConfiguration, RemoteConfigurationMetrics } from './remoteConfiguration' import { applyRemoteConfiguration, buildEndpoint, fetchRemoteConfiguration } from './remoteConfiguration' const DEFAULT_INIT_CONFIGURATION: RumInitConfiguration = { @@ -99,11 +99,15 @@ describe('remoteConfiguration', () => { }) describe('applyRemoteConfiguration', () => { + const COOKIE_NAME = 'unit_rc' + const root = window as any + let displaySpy: jasmine.Spy let supportedContextManagers: { user: ReturnType context: ReturnType } + let metrics: RemoteConfigurationMetrics function expectAppliedRemoteConfigurationToBe( actual: Partial, @@ -114,7 +118,7 @@ describe('remoteConfiguration', () => { ...actual, } expect( - applyRemoteConfiguration(DEFAULT_INIT_CONFIGURATION, rumRemoteConfiguration, supportedContextManagers) + applyRemoteConfiguration(DEFAULT_INIT_CONFIGURATION, rumRemoteConfiguration, supportedContextManagers, metrics) ).toEqual({ ...DEFAULT_INIT_CONFIGURATION, applicationId: 'yyy', @@ -125,6 +129,7 @@ describe('remoteConfiguration', () => { beforeEach(() => { displaySpy = spyOn(display, 'error') supportedContextManagers = { user: createContextManager(), context: createContextManager() } + metrics = { fetch: {} } }) it('should override the initConfiguration options with the ones from the remote configuration', () => { @@ -151,7 +156,7 @@ describe('remoteConfiguration', () => { defaultPrivacyLevel: DefaultPrivacyLevel.ALLOW, } expect( - applyRemoteConfiguration(DEFAULT_INIT_CONFIGURATION, rumRemoteConfiguration, supportedContextManagers) + applyRemoteConfiguration(DEFAULT_INIT_CONFIGURATION, rumRemoteConfiguration, supportedContextManagers, metrics) ).toEqual({ applicationId: 'yyy', clientToken: 'xxx', @@ -194,8 +199,6 @@ describe('remoteConfiguration', () => { }) describe('cookie strategy', () => { - const COOKIE_NAME = 'unit_rc' - it('should resolve a configuration value from a cookie', () => { setCookie(COOKIE_NAME, 'my-version', ONE_MINUTE) registerCleanupTask(() => deleteCookie(COOKIE_NAME)) @@ -203,6 +206,7 @@ describe('remoteConfiguration', () => { { version: { rcSerializedType: 'dynamic', strategy: 'cookie', name: COOKIE_NAME } }, { version: 'my-version' } ) + expect(metrics.cookie).toEqual({ success: 1 }) }) it('should resolve to undefined if the cookie is missing', () => { @@ -210,6 +214,7 @@ describe('remoteConfiguration', () => { { version: { rcSerializedType: 'dynamic', strategy: 'cookie', name: COOKIE_NAME } }, { version: undefined } ) + expect(metrics.cookie).toEqual({ missing: 1 }) }) }) @@ -226,6 +231,7 @@ describe('remoteConfiguration', () => { { version: { rcSerializedType: 'dynamic', strategy: 'dom', selector: '#version1' } }, { version: 'version-123' } ) + expect(metrics.dom).toEqual({ success: 1 }) }) it('should resolve a configuration value from an element text content and an extractor', () => { @@ -255,6 +261,7 @@ describe('remoteConfiguration', () => { { version: undefined } ) expect(displaySpy).toHaveBeenCalledWith("Invalid selector in the remote configuration: ''") + expect(metrics.dom).toEqual({ failure: 1 }) }) it('should resolve to undefined if the element is missing', () => { @@ -262,6 +269,7 @@ describe('remoteConfiguration', () => { { version: { rcSerializedType: 'dynamic', strategy: 'dom', selector: '#missing' } }, { version: undefined } ) + expect(metrics.dom).toEqual({ missing: 1 }) }) it('should resolve a configuration value from an element attribute', () => { @@ -278,6 +286,7 @@ describe('remoteConfiguration', () => { { version: { rcSerializedType: 'dynamic', strategy: 'dom', selector: '#version2', attribute: 'missing' } }, { version: undefined } ) + expect(metrics.dom).toEqual({ missing: 1 }) }) it('should resolve to undefined if trying to access a password input value attribute', () => { @@ -286,12 +295,11 @@ describe('remoteConfiguration', () => { { version: { rcSerializedType: 'dynamic', strategy: 'dom', selector: '#pwd', attribute: 'value' } }, { version: undefined } ) + expect(displaySpy).toHaveBeenCalledWith("Forbidden element selected by the remote configuration: '#pwd'") }) }) describe('js strategy', () => { - const root = window as any - it('should resolve a value from a variable content', () => { root.foo = 'bar' registerCleanupTask(() => { @@ -303,6 +311,7 @@ describe('remoteConfiguration', () => { }, { version: 'bar' } ) + expect(metrics.js).toEqual({ success: 1 }) }) it('should resolve a value from an object property', () => { @@ -401,6 +410,7 @@ describe('remoteConfiguration', () => { { version: undefined } ) expect(displaySpy).toHaveBeenCalledWith("Invalid JSON path in the remote configuration: '.'") + expect(metrics.js).toEqual({ failure: 1 }) }) it('should resolve to undefined and display an error if the variable access throws', () => { @@ -419,6 +429,7 @@ describe('remoteConfiguration', () => { { version: undefined } ) expect(displaySpy).toHaveBeenCalledWith("Error accessing: 'foo.bar'", new Error('foo')) + expect(metrics.js).toEqual({ failure: 1 }) }) it('should resolve to undefined if the variable does not exist', () => { @@ -428,6 +439,7 @@ describe('remoteConfiguration', () => { }, { version: undefined } ) + expect(metrics.js).toEqual({ missing: 1 }) }) it('should resolve to undefined if the property does not exist', () => { @@ -442,6 +454,7 @@ describe('remoteConfiguration', () => { }, { version: undefined } ) + expect(metrics.js).toEqual({ missing: 1 }) }) it('should resolve to undefined if the array index does not exist', () => { @@ -456,12 +469,11 @@ describe('remoteConfiguration', () => { }, { version: undefined } ) + expect(metrics.js).toEqual({ missing: 1 }) }) }) describe('with extractor', () => { - const COOKIE_NAME = 'unit_rc' - beforeEach(() => { setCookie(COOKIE_NAME, 'my-version-123', ONE_MINUTE) }) @@ -529,8 +541,6 @@ describe('remoteConfiguration', () => { }) describe('supported contexts', () => { - const COOKIE_NAME = 'unit_rc' - beforeEach(() => { setCookie(COOKIE_NAME, 'first.second', ONE_MINUTE) }) @@ -592,6 +602,63 @@ describe('remoteConfiguration', () => { }) }) }) + + describe('metrics', () => { + it('should report resolution stats', () => { + setCookie(COOKIE_NAME, 'my-version', ONE_MINUTE) + root.foo = '123' + registerCleanupTask(() => { + deleteCookie(COOKIE_NAME) + delete root.foo + }) + + expectAppliedRemoteConfigurationToBe( + { + context: [ + { + key: 'missing-cookie', + value: { + rcSerializedType: 'dynamic', + strategy: 'cookie', + name: 'missing-cookie', + }, + }, + { + key: 'existing-cookie', + value: { + rcSerializedType: 'dynamic', + strategy: 'cookie', + name: COOKIE_NAME, + }, + }, + { + key: 'existing-cookie2', + value: { + rcSerializedType: 'dynamic', + strategy: 'cookie', + name: COOKIE_NAME, + }, + }, + { + key: 'existing-js', + value: { + rcSerializedType: 'dynamic', + strategy: 'js', + path: 'foo', + }, + }, + ], + }, + {} + ) + expect(metrics).toEqual( + jasmine.objectContaining({ + cookie: { success: 2, missing: 1 }, + js: { success: 1 }, + }) + ) + }) + }) }) describe('buildEndpoint', () => { diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 890678951e..7f6902d732 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -1,11 +1,19 @@ -import type { createContextManager } from '@datadog/browser-core' -import { display, buildEndpointHost, mapValues, getCookie } from '@datadog/browser-core' +import type { createContextManager, Context } from '@datadog/browser-core' +import { + display, + buildEndpointHost, + mapValues, + getCookie, + addTelemetryMetrics, + Observable, +} from '@datadog/browser-core' import type { RumInitConfiguration } from './configuration' import type { RumSdkConfig, DynamicOption, ContextItem } from './remoteConfiguration.types' import { parseJsonPath } from './jsonPathParser' export type RemoteConfiguration = RumSdkConfig export type RumRemoteConfiguration = Exclude +export const REMOTE_CONFIGURATION_METRIC_NAME = 'remote configuration metrics' const REMOTE_CONFIGURATION_VERSION = 'v1' const SUPPORTED_FIELDS: Array = [ 'applicationId', @@ -32,23 +40,52 @@ interface SupportedContextManagers { context: ReturnType } +export interface RemoteConfigurationMetrics { + fetch: RemoteConfigurationMetricCounters + cookie?: RemoteConfigurationMetricCounters + dom?: RemoteConfigurationMetricCounters + js?: RemoteConfigurationMetricCounters +} + +interface RemoteConfigurationMetricCounters { + success?: number + missing?: number + failure?: number +} + +type ResolveEvent = [DynamicOption['strategy'], keyof RemoteConfigurationMetricCounters] + export async function fetchAndApplyRemoteConfiguration( initConfiguration: RumInitConfiguration, supportedContextManagers: SupportedContextManagers ) { + let rumInitConfiguration: RumInitConfiguration | undefined + const metrics: RemoteConfigurationMetrics = { fetch: {} } const fetchResult = await fetchRemoteConfiguration(initConfiguration) if (!fetchResult.ok) { + metrics.fetch.failure = 1 display.error(fetchResult.error) - return + } else { + metrics.fetch.success = 1 + rumInitConfiguration = applyRemoteConfiguration( + initConfiguration, + fetchResult.value, + supportedContextManagers, + metrics + ) } - return applyRemoteConfiguration(initConfiguration, fetchResult.value, supportedContextManagers) + addTelemetryMetrics(REMOTE_CONFIGURATION_METRIC_NAME, { metrics: metrics as unknown as Context }) + return rumInitConfiguration } export function applyRemoteConfiguration( initConfiguration: RumInitConfiguration, rumRemoteConfiguration: RumRemoteConfiguration & { [key: string]: unknown }, - supportedContextManagers: SupportedContextManagers + supportedContextManagers: SupportedContextManagers, + metrics: RemoteConfigurationMetrics ): RumInitConfiguration { + const resolveEventObservable = new Observable() + resolveEventObservable.subscribe(incrementMetrics) // intents: // - explicitly set each supported field to limit risk in case an attacker can create configurations // - check the existence in the remote config to avoid clearing a provided init field @@ -125,22 +162,41 @@ export function applyRemoteConfiguration( } function resolveCookieValue({ name }: { name: string }) { - return getCookie(name) + const value = getCookie(name) + resolveEventObservable.notify(['cookie', value !== undefined ? 'success' : 'missing']) + return value } function resolveDomValue({ selector, attribute }: { selector: string; attribute?: string }) { let element: Element | null + let missing = false + let failure = false + let value: string | undefined try { element = document.querySelector(selector) } catch { element = null + failure = true display.error(`Invalid selector in the remote configuration: '${selector}'`) } - if (element === null || isForbidden(element, attribute)) { - return + if (element === null && !failure) { + missing = true + } + if (element && isForbidden(element, attribute)) { + failure = true + element = null + display.error(`Forbidden element selected by the remote configuration: '${selector}'`) + } + if (element) { + const domValue = attribute !== undefined ? element.getAttribute(attribute) : element.textContent + if (domValue === null) { + missing = true + } else { + value = domValue + } } - const domValue = attribute !== undefined ? element.getAttribute(attribute) : element.textContent - return domValue ?? undefined + resolveEventObservable.notify(['dom', failure ? 'failure' : missing ? 'missing' : 'success']) + return value } function isForbidden(element: Element, attribute: string | undefined) { @@ -149,24 +205,40 @@ export function applyRemoteConfiguration( function resolveJsValue({ path }: { path: string }): unknown { let current = window as unknown as { [key: string]: unknown } + let missing = false + let failure = false const pathParts = parseJsonPath(path) if (pathParts.length === 0) { + failure = true display.error(`Invalid JSON path in the remote configuration: '${path}'`) - return - } - for (const pathPart of pathParts) { - if (!(pathPart in current)) { - return - } - try { - current = current[pathPart] as { [key: string]: unknown } - } catch (e) { - display.error(`Error accessing: '${path}'`, e) - return + } else { + for (const pathPart of pathParts) { + if (!(pathPart in current)) { + missing = true + break + } + try { + current = current[pathPart] as { [key: string]: unknown } + } catch (e) { + failure = true + display.error(`Error accessing: '${path}'`, e) + break + } } } - return current + resolveEventObservable.notify(['js', failure ? 'failure' : missing ? 'missing' : 'success']) + return !missing && !failure ? current : undefined + } + + function incrementMetrics([strategy, type]: ResolveEvent) { + if (!metrics[strategy]) { + metrics[strategy] = {} + } + if (!metrics[strategy][type]) { + metrics[strategy][type] = 0 + } + metrics[strategy][type] = metrics[strategy][type] + 1 } } From 211f65b24aec969b9f1e01a50e13ced24868d2e1 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Tue, 16 Sep 2025 17:53:07 +0200 Subject: [PATCH 4/5] =?UTF-8?q?=E2=9C=85add=20a=20test=20on=20performance?= =?UTF-8?q?=20PR=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../lib/reportAsAPrComment.spec.ts | 85 +++++++++++++++++++ scripts/performance/lib/reportAsAPrComment.ts | 10 +-- 2 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 scripts/performance/lib/reportAsAPrComment.spec.ts diff --git a/scripts/performance/lib/reportAsAPrComment.spec.ts b/scripts/performance/lib/reportAsAPrComment.spec.ts new file mode 100644 index 0000000000..e271ce8ef1 --- /dev/null +++ b/scripts/performance/lib/reportAsAPrComment.spec.ts @@ -0,0 +1,85 @@ +import assert from 'node:assert/strict' +import { describe, it } from 'node:test' +import { createMessage } from './reportAsAPrComment.ts' + +void describe('reportAsAPrComment', () => { + void describe('createMessage', () => { + const TEST_BUNDLE = 'test' + const PR_NUMBER = 123 + + const BASE_BUNDLE_SIZES = [{ name: TEST_BUNDLE, value: 100 }] + const MEMORY_BASE_PERFORMANCE = [{ name: TEST_BUNDLE, value: 100 }] + const CPU_BASE_PERFORMANCE = [{ name: TEST_BUNDLE, value: 100 }] + + const MEMORY_LOCAL_PERFORMANCE = [{ testProperty: TEST_BUNDLE, sdkMemoryBytes: 101, sdkMemoryPercentage: 10 }] + const CPU_LOCAL_PERFORMANCE = [{ name: TEST_BUNDLE, value: 101 }] + + void it('should generate a report with performance results', () => { + const localBundleSizes = { test: 101 } + + const message = createMessage( + BASE_BUNDLE_SIZES, + localBundleSizes, + MEMORY_BASE_PERFORMANCE, + MEMORY_LOCAL_PERFORMANCE, + CPU_BASE_PERFORMANCE, + CPU_LOCAL_PERFORMANCE, + PR_NUMBER + ) + + assert.equal( + message, + `| 📦 Bundle Name | Base Size | Local Size | 𝚫 | 𝚫% | Status | +| --- | --- | --- | --- | --- | --- | +| Test | 100 B | 101 B | 1 B | +1.00% | ✅ | + + +
+🚀 CPU Performance + +| Action Name | Base Average Cpu Time (ms) | Local Average Cpu Time (ms) | 𝚫 | +| --- | --- | --- | --- | +| test | 100.000 | 101.000 | 1.000 | + +
+ +
+🧠 Memory Performance + +| Action Name | Base Consumption Memory (bytes) | Local Consumption Memory (bytes) | 𝚫 (bytes) | +| --- | --- | --- | --- | +| test | 100 B | 101 B | 1 B | + +
+ +🔗 [RealWorld](https://datadoghq.dev/browser-sdk-test-playground/realworld-scenario/?prNumber=123) + +` + ) + }) + + void it('should add a warning when the size increase is above the threshold', () => { + const localBundleSizes = { test: 150 } + + const message = createMessage( + BASE_BUNDLE_SIZES, + localBundleSizes, + MEMORY_BASE_PERFORMANCE, + MEMORY_LOCAL_PERFORMANCE, + CPU_BASE_PERFORMANCE, + CPU_LOCAL_PERFORMANCE, + PR_NUMBER + ) + + assertContains(message, '| Test | 100 B | 150 B | 50 B | +50.00% | ⚠️ |') + assertContains(message, '⚠️ The increase is particularly high and exceeds 5%. Please check the changes.') + }) + }) +}) + +function assertContains(actual: string, expected: string) { + assert.ok( + actual.includes(expected), + ['Expected string to contain:', ` expected: "${expected}"`, ` actual: "${actual}"`].join('\n') + ) +} diff --git a/scripts/performance/lib/reportAsAPrComment.ts b/scripts/performance/lib/reportAsAPrComment.ts index a0ebed2fef..8cf91e7e4c 100644 --- a/scripts/performance/lib/reportAsAPrComment.ts +++ b/scripts/performance/lib/reportAsAPrComment.ts @@ -52,12 +52,8 @@ export async function reportAsPrComment( const cpuBasePerformance = await fetchPerformanceMetrics('cpu', testNames, lastCommonCommit) const cpuLocalPerformance = await fetchPerformanceMetrics('cpu', testNames, LOCAL_COMMIT_SHA || '') const memoryBasePerformance = await fetchPerformanceMetrics('memory', testNames, lastCommonCommit) - const differenceBundle = compare(baseBundleSizes, localBundleSizes) - const differenceCpu = compare(cpuBasePerformance, cpuLocalPerformance) const commentId = await retrieveExistingCommentId(pr.number) const message = createMessage( - differenceBundle, - differenceCpu, baseBundleSizes, localBundleSizes, memoryBasePerformance, @@ -129,9 +125,7 @@ async function updateOrAddComment(message: string, prNumber: number, commentId: }) } -function createMessage( - differenceBundle: PerformanceDifference[], - differenceCpu: PerformanceDifference[], +export function createMessage( baseBundleSizes: PerformanceMetric[], localBundleSizes: BundleSizes, memoryBasePerformance: PerformanceMetric[], @@ -140,6 +134,8 @@ function createMessage( cpuLocalPerformance: PerformanceMetric[], prNumber: number ): string { + const differenceBundle = compare(baseBundleSizes, localBundleSizes) + const differenceCpu = compare(cpuBasePerformance, cpuLocalPerformance) let highIncreaseDetected = false const bundleRows = differenceBundle.map((diff, index) => { const baseSize = formatSize(baseBundleSizes[index].value) From ad6ee2bb0e5e88d12d36fa73d3a22f6ede9f697a Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Tue, 16 Sep 2025 17:59:09 +0200 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=90=9Bremove=20number=20type=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/performance/lib/reportAsAPrComment.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/performance/lib/reportAsAPrComment.ts b/scripts/performance/lib/reportAsAPrComment.ts index 8cf91e7e4c..c8714ddfe3 100644 --- a/scripts/performance/lib/reportAsAPrComment.ts +++ b/scripts/performance/lib/reportAsAPrComment.ts @@ -141,9 +141,9 @@ export function createMessage( const baseSize = formatSize(baseBundleSizes[index].value) const localSize = formatSize(localBundleSizes[diff.name]) const diffSize = formatSize(diff.change) - const sign = typeof diff.percentageChange === 'number' && diff.percentageChange > 0 ? '+' : '' + const sign = (diff.percentageChange as number) > 0 ? '+' : '' let status = '✅' - if (typeof diff.percentageChange === 'number' && diff.percentageChange > SIZE_INCREASE_THRESHOLD) { + if ((diff.percentageChange as number) > SIZE_INCREASE_THRESHOLD) { status = '⚠️' highIncreaseDetected = true }