From 2374b6793fde82bd2235c1a8a13216fbe51e80d2 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 13 Jan 2023 16:05:45 +0100 Subject: [PATCH 01/18] Move computeBytesCount from Batch to utils --- packages/core/src/tools/utils.spec.ts | 11 +++++++++++ packages/core/src/tools/utils.ts | 16 ++++++++++++++++ packages/core/src/transport/batch.ts | 4 ++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tools/utils.spec.ts b/packages/core/src/tools/utils.spec.ts index 40548a5ef1..92d40dc874 100644 --- a/packages/core/src/tools/utils.spec.ts +++ b/packages/core/src/tools/utils.spec.ts @@ -4,6 +4,7 @@ import { display } from './display' import { arrayFrom, combine, + computeBytesCount, cssEscape, deepClone, elementMatches, @@ -689,3 +690,13 @@ describe('matchList', () => { expect(display.error).toHaveBeenCalled() }) }) + +describe('computeBytesCount', () => { + it('should count the bytes of a message composed of 1 byte characters', () => { + expect(computeBytesCount('1234')).toEqual(4) + }) + + it('should count the bytes of a message composed of multiple bytes characters', () => { + expect(computeBytesCount('🪐')).toEqual(4) + }) +}) diff --git a/packages/core/src/tools/utils.ts b/packages/core/src/tools/utils.ts index 438b33e867..c7dfd6d151 100644 --- a/packages/core/src/tools/utils.ts +++ b/packages/core/src/tools/utils.ts @@ -594,3 +594,19 @@ export function cssEscape(str: string) { return `\\${ch}` }) } + +// eslint-disable-next-line no-control-regex +const HAS_MULTI_BYTES_CHARACTERS = /[^\u0000-\u007F]/ + +export function computeBytesCount(candidate: string): number { + // Accurate bytes count computations can degrade performances when there is a lot of events to process + if (!HAS_MULTI_BYTES_CHARACTERS.test(candidate)) { + return candidate.length + } + + if (window.TextEncoder !== undefined) { + return new TextEncoder().encode(candidate).length + } + + return new Blob([candidate]).size +} diff --git a/packages/core/src/transport/batch.ts b/packages/core/src/transport/batch.ts index b8fb985cc5..6009671bc6 100644 --- a/packages/core/src/transport/batch.ts +++ b/packages/core/src/transport/batch.ts @@ -86,7 +86,7 @@ export class Batch { private process(message: Context) { const processedMessage = jsonStringify(message)! - const messageBytesCount = this.computeBytesCount(processedMessage) + const messageBytesCount = computeBytesCount(processedMessage) return { processedMessage, messageBytesCount } } @@ -107,7 +107,7 @@ export class Batch { private remove(key: string) { const removedMessage = this.upsertBuffer[key] delete this.upsertBuffer[key] - const messageBytesCount = this.computeBytesCount(removedMessage) + const messageBytesCount = computeBytesCount(removedMessage) this.bufferBytesCount -= messageBytesCount this.bufferMessagesCount -= 1 if (this.bufferMessagesCount > 0) { From ca7d3ab77536be4ca5efa1d7c0946866cac0f901 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 13 Jan 2023 16:02:24 +0100 Subject: [PATCH 02/18] Count context bytes in context manager --- .../core/src/tools/contextManager.spec.ts | 56 ++++++++++++++++++- packages/core/src/tools/contextManager.ts | 33 ++++++++++- packages/core/test/specHelper.ts | 8 +++ packages/rum-core/test/specHelper.ts | 13 ++++- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/packages/core/src/tools/contextManager.spec.ts b/packages/core/src/tools/contextManager.spec.ts index 6444768ca0..c00035e3a7 100644 --- a/packages/core/src/tools/contextManager.spec.ts +++ b/packages/core/src/tools/contextManager.spec.ts @@ -1,4 +1,6 @@ -import { createContextManager } from './contextManager' +import { contextBytesCounterStub } from '../../test/specHelper' +import { contextBytesCounter, createContextManager } from './contextManager' +import { computeBytesCount } from './utils' describe('createContextManager', () => { it('starts with an empty context', () => { @@ -73,4 +75,56 @@ describe('createContextManager', () => { manager.clearContext() expect(manager.getContext()).toEqual({}) }) + + it('should invalidate the context bytes counter at each mutation', () => { + const bytesCountStub = contextBytesCounterStub() + const manager = createContextManager(bytesCountStub) + manager.add('foo', 'bar') + manager.remove('foo') + manager.set({ foo: 'bar' }) + manager.removeContextProperty('foo') + manager.setContext({ foo: 'bar' }) + manager.clearContext() + + expect(bytesCountStub.invalidate).toHaveBeenCalledTimes(6) + }) + + it('should get the context bytes count', () => { + const bytesCountStub = contextBytesCounterStub() + const manager = createContextManager(bytesCountStub) + const contextBytesCount = manager.getBytesCount() + + expect(contextBytesCount).toEqual(1) + }) +}) + +describe('contextBytesCounter', () => { + let computeBytesCountSpy: jasmine.Spy + let counter: ReturnType + + beforeEach(() => { + computeBytesCountSpy = jasmine.createSpy('computeBytesCount', computeBytesCount).and.callThrough() + counter = contextBytesCounter(computeBytesCountSpy) + }) + + it('should compute the batch count when the cache is invalidate', () => { + const bytesCount1 = counter.compute({ a: 'b' }) + counter.invalidate() + const bytesCount2 = counter.compute({ foo: 'bar' }) + + expect(computeBytesCountSpy).toHaveBeenCalledTimes(2) + expect(bytesCount1).not.toEqual(bytesCount2) + }) + + it('should use the cached bytes count when the cache is not invalidate', () => { + const bytesCount1 = counter.compute({ a: 'b' }) + const bytesCount2 = counter.compute({ foo: 'bar' }) + expect(computeBytesCountSpy).toHaveBeenCalledTimes(1) + expect(bytesCount1).toEqual(bytesCount2) + }) + + it('should return a bytes count at 0 when the object is empty', () => { + const bytesCount = counter.compute({}) + expect(bytesCount).toEqual(0) + }) }) diff --git a/packages/core/src/tools/contextManager.ts b/packages/core/src/tools/contextManager.ts index 3dea87f169..abd54932b2 100644 --- a/packages/core/src/tools/contextManager.ts +++ b/packages/core/src/tools/contextManager.ts @@ -1,45 +1,72 @@ -import { deepClone } from './utils' - +import { computeBytesCount, deepClone, isEmptyObject, jsonStringify } from './utils' import type { Context, ContextValue } from './context' -export function createContextManager() { +export type ContextManager = ReturnType + +export function createContextManager(bytesCounter = contextBytesCounter()) { let context: Context = {} return { + getBytesCount: () => bytesCounter.compute(context), /** @deprecated use getContext instead */ get: () => context, /** @deprecated use setContextProperty instead */ add: (key: string, value: any) => { context[key] = value as ContextValue + bytesCounter.invalidate() }, /** @deprecated renamed to removeContextProperty */ remove: (key: string) => { delete context[key] + bytesCounter.invalidate() }, /** @deprecated use setContext instead */ set: (newContext: object) => { context = newContext as Context + bytesCounter.invalidate() }, getContext: () => deepClone(context), setContext: (newContext: Context) => { context = deepClone(newContext) + bytesCounter.invalidate() }, setContextProperty: (key: string, property: any) => { context[key] = deepClone(property) + bytesCounter.invalidate() }, removeContextProperty: (key: string) => { delete context[key] + bytesCounter.invalidate() }, clearContext: () => { context = {} + bytesCounter.invalidate() + }, + } +} + +export type ContextBytesCounter = ReturnType + +export function contextBytesCounter(computeBytesCountImpl = computeBytesCount) { + let bytesCount: number | undefined + + return { + compute: (context: Context) => { + if (bytesCount === undefined) { + bytesCount = !isEmptyObject(context) ? computeBytesCountImpl(jsonStringify(context)!) : 0 + } + return bytesCount + }, + invalidate: () => { + bytesCount = undefined }, } } diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 879558c12e..7ffe34a45a 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -1,4 +1,5 @@ import type { EndpointBuilder } from '../src/domain/configuration' +import type { ContextBytesCounter } from '../src/tools/contextManager' import { instrumentMethod } from '../src/tools/instrumentMethod' import { resetNavigationStart } from '../src/tools/timeUtils' import { buildUrl } from '../src/tools/urlPolyfill' @@ -498,3 +499,10 @@ export function interceptRequests() { }, } } + +export function contextBytesCounterStub(): ContextBytesCounter { + return { + compute: () => 1, + invalidate: jasmine.createSpy('invalidate'), + } +} diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index 1b01e3747d..4818e442de 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -1,5 +1,5 @@ -import type { Context, TimeStamp } from '@datadog/browser-core' -import { assign, combine, Observable, noop } from '@datadog/browser-core' +import type { Context, ContextManager, TimeStamp } from '@datadog/browser-core' +import { createContextManager, assign, combine, Observable, noop } from '@datadog/browser-core' import type { Clock } from '../../core/test/specHelper' import { SPEC_ENDPOINTS, mockClock, buildLocation } from '../../core/test/specHelper' import type { RecorderApi } from '../src/boot/rumPublicApi' @@ -53,6 +53,8 @@ export interface BuildContext { foregroundContexts: ForegroundContexts featureFlagContexts: FeatureFlagContexts urlContexts: UrlContexts + globalContextManager: ContextManager + userContextManager: ContextManager } export interface TestIO { @@ -87,6 +89,7 @@ export function setup(): TestSetupBuilder { let featureFlagContexts: FeatureFlagContexts = { findFeatureFlagEvaluations: () => undefined, addFeatureFlagEvaluation: noop, + getFeatureFlagBytesCount: () => 0, } let actionContexts: ActionContexts = { findActionId: noop as () => undefined, @@ -96,6 +99,10 @@ export function setup(): TestSetupBuilder { selectInForegroundPeriodsFor: () => undefined, stop: noop, } + + const globalContextManager = createContextManager() + const userContextManager = createContextManager() + const FAKE_APP_ID = 'appId' const configuration: RumConfiguration = { ...validateAndBuildRumConfiguration({ clientToken: 'xxx', applicationId: FAKE_APP_ID })!, @@ -170,6 +177,8 @@ export function setup(): TestSetupBuilder { applicationId: FAKE_APP_ID, configuration, location: fakeLocation as Location, + globalContextManager, + userContextManager, }) if (result && result.stop) { cleanupTasks.push(result.stop) From 075b8867194bd3c962a1fcacaf05c951e4503d1e Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 13 Jan 2023 16:03:26 +0100 Subject: [PATCH 03/18] Count feature flags context bytes --- .../contexts/featureFlagContext.spec.ts | 35 ++++++++++++++++++- .../src/domain/contexts/featureFlagContext.ts | 18 +++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts index ec5a1d595f..762524823b 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts @@ -1,5 +1,7 @@ import type { RelativeTime } from '@datadog/browser-core' import { resetExperimentalFeatures, updateExperimentalFeatures, relativeToClocks } from '@datadog/browser-core' +import type { ContextBytesCounter } from '../../../../core/src/tools/contextManager' +import { contextBytesCounterStub } from '../../../../core/test/specHelper' import type { TestSetupBuilder } from '../../../test/specHelper' import { setup } from '../../../test/specHelper' import { LifeCycleEventType } from '../lifeCycle' @@ -10,10 +12,12 @@ import { startFeatureFlagContexts } from './featureFlagContext' describe('featureFlagContexts', () => { let setupBuilder: TestSetupBuilder let featureFlagContexts: FeatureFlagContexts + let bytesCounterStub: ContextBytesCounter beforeEach(() => { setupBuilder = setup().beforeBuild(({ lifeCycle }) => { - featureFlagContexts = startFeatureFlagContexts(lifeCycle) + bytesCounterStub = contextBytesCounterStub() + featureFlagContexts = startFeatureFlagContexts(lifeCycle, bytesCounterStub) }) }) @@ -84,6 +88,18 @@ describe('featureFlagContexts', () => { expect(featureFlagContext).toBeUndefined() }) + + it('should invalidate the context bytes count when a new feature flag is added or a view is created', () => { + updateExperimentalFeatures(['feature_flags']) + + const { lifeCycle } = setupBuilder.withFakeClock().build() + lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { + startClocks: relativeToClocks(0 as RelativeTime), + } as ViewCreatedEvent) + + featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo') + expect(bytesCounterStub.invalidate).toHaveBeenCalledTimes(2) + }) }) describe('findFeatureFlagEvaluations', () => { @@ -144,4 +160,21 @@ describe('featureFlagContexts', () => { expect(featureFlagContexts.findFeatureFlagEvaluations(15 as RelativeTime)).toEqual({ feature: 'two' }) }) }) + + describe('getFeatureFlagBytesCount', () => { + it('should get the context bytes count', () => { + updateExperimentalFeatures(['feature_flags']) + + const { lifeCycle } = setupBuilder.withFakeClock().build() + lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { + startClocks: relativeToClocks(0 as RelativeTime), + } as ViewCreatedEvent) + + featureFlagContexts.addFeatureFlagEvaluation('foo', 'bar') + + const bytesCount = featureFlagContexts.getFeatureFlagBytesCount() + + expect(bytesCount).toEqual(1) + }) + }) }) diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.ts index 23f90d6c0d..0428e10188 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.ts @@ -1,5 +1,6 @@ import type { RelativeTime, ContextValue, Context } from '@datadog/browser-core' import { + contextBytesCounter, deepClone, noop, isExperimentalFeatureEnabled, @@ -15,6 +16,7 @@ export type FeatureFlagContext = Context export interface FeatureFlagContexts { findFeatureFlagEvaluations: (startTime?: RelativeTime) => FeatureFlagContext | undefined + getFeatureFlagBytesCount: () => number addFeatureFlagEvaluation: (key: string, value: ContextValue) => void } @@ -26,10 +28,14 @@ export interface FeatureFlagContexts { * * Note: we choose not to add a new context at each evaluation to save memory */ -export function startFeatureFlagContexts(lifeCycle: LifeCycle): FeatureFlagContexts { +export function startFeatureFlagContexts( + lifeCycle: LifeCycle, + bytesCounter = contextBytesCounter() +): FeatureFlagContexts { if (!isExperimentalFeatureEnabled('feature_flags')) { return { findFeatureFlagEvaluations: () => undefined, + getFeatureFlagBytesCount: () => 0, addFeatureFlagEvaluation: noop, } } @@ -42,14 +48,24 @@ export function startFeatureFlagContexts(lifeCycle: LifeCycle): FeatureFlagConte lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => { featureFlagContexts.add({}, startClocks.relative) + bytesCounter.invalidate() }) return { findFeatureFlagEvaluations: (startTime?: RelativeTime) => featureFlagContexts.find(startTime), + getFeatureFlagBytesCount: () => { + const currentContext = featureFlagContexts.find() + if (!currentContext) { + return 0 + } + + return bytesCounter.compute(currentContext) + }, addFeatureFlagEvaluation: (key: string, value: ContextValue) => { const currentContext = featureFlagContexts.find() if (currentContext) { currentContext[key] = deepClone(value) + bytesCounter.invalidate() } }, } From 46d4c138458523f05c0b32e581530e8573707f16 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 13 Jan 2023 16:07:43 +0100 Subject: [PATCH 04/18] Make the batch flush observable --- packages/core/src/index.ts | 3 +- packages/core/src/transport/batch.spec.ts | 17 +++++----- packages/core/src/transport/batch.ts | 31 ++++++++----------- packages/core/src/transport/index.ts | 2 +- .../rum-core/src/transport/startRumBatch.ts | 8 +++-- 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8b1c1f2fbf..91869e0c64 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -47,6 +47,7 @@ export { Payload, createHttpRequest, Batch, + BatchFlushEvent, canUseEventBridge, getEventBridge, startBatchWithReplica, @@ -81,7 +82,7 @@ export * from './browser/addEventListener' export { initConsoleObservable, ConsoleLog } from './domain/console/consoleObservable' export { BoundedBuffer } from './tools/boundedBuffer' export { catchUserErrors } from './tools/catchUserErrors' -export { createContextManager } from './tools/contextManager' +export { createContextManager, contextBytesCounter, ContextManager } from './tools/contextManager' export { limitModification } from './tools/limitModification' export { ContextHistory, ContextHistoryEntry, CLEAR_OLD_CONTEXTS_INTERVAL } from './tools/contextHistory' export { readBytesFromStream } from './tools/readBytesFromStream' diff --git a/packages/core/src/transport/batch.spec.ts b/packages/core/src/transport/batch.spec.ts index e89350287a..2a9fec70c5 100644 --- a/packages/core/src/transport/batch.spec.ts +++ b/packages/core/src/transport/batch.spec.ts @@ -3,6 +3,7 @@ import type { PageExitEvent } from '../browser/pageExitObservable' import { PageExitReason } from '../browser/pageExitObservable' import { Observable } from '../tools/observable' import { noop } from '../tools/utils' +import type { BatchFlushEvent } from './batch' import { Batch } from './batch' import type { HttpRequest } from './httpRequest' @@ -15,6 +16,7 @@ describe('batch', () => { let transport: HttpRequest let sendSpy: jasmine.Spy let pageExitObservable: Observable + let flushNotifySpy: jasmine.Spy<() => BatchFlushEvent> beforeEach(() => { transport = { send: noop } as unknown as HttpRequest @@ -28,6 +30,7 @@ describe('batch', () => { FLUSH_TIMEOUT, pageExitObservable ) + flushNotifySpy = spyOn(batch.flushObservable, 'notify') }) it('should add context to message', () => { @@ -51,14 +54,6 @@ describe('batch', () => { expect(transport.send).not.toHaveBeenCalled() }) - it('should count the bytes of a message composed of 1 byte characters', () => { - expect(batch.computeBytesCount('1234')).toEqual(4) - }) - - it('should count the bytes of a message composed of multiple bytes characters', () => { - expect(batch.computeBytesCount('🪐')).toEqual(4) - }) - it('should flush when the message count limit is reached', () => { batch.add({ message: '1' }) batch.add({ message: '2' }) @@ -189,4 +184,10 @@ describe('batch', () => { batch.flush() expect(sendSpy).toHaveBeenCalledTimes(2) }) + + it('should notify when the batch is flushed', () => { + batch.add({}) + batch.flush() + expect(flushNotifySpy).toHaveBeenCalledOnceWith({ bufferBytesCount: 2, bufferMessagesCount: 1 }) + }) }) diff --git a/packages/core/src/transport/batch.ts b/packages/core/src/transport/batch.ts index 6009671bc6..b741694a64 100644 --- a/packages/core/src/transport/batch.ts +++ b/packages/core/src/transport/batch.ts @@ -1,16 +1,19 @@ import { display } from '../tools/display' import type { Context } from '../tools/context' -import { jsonStringify, objectValues } from '../tools/utils' +import { computeBytesCount, jsonStringify, objectValues } from '../tools/utils' import { monitor } from '../tools/monitor' -import type { Observable } from '../tools/observable' +import { Observable } from '../tools/observable' import type { PageExitEvent } from '../browser/pageExitObservable' import type { HttpRequest } from './httpRequest' -// https://en.wikipedia.org/wiki/UTF-8 -// eslint-disable-next-line no-control-regex -const HAS_MULTI_BYTES_CHARACTERS = /[^\u0000-\u007F]/ +export interface BatchFlushEvent { + bufferBytesCount: number + bufferMessagesCount: number +} export class Batch { + flushObservable = new Observable() + private pushOnlyBuffer: string[] = [] private upsertBuffer: { [key: string]: string } = {} private bufferBytesCount = 0 @@ -41,6 +44,11 @@ export class Batch { const messages = this.pushOnlyBuffer.concat(objectValues(this.upsertBuffer)) const bytesCount = this.bufferBytesCount + this.flushObservable.notify({ + bufferBytesCount: this.bufferBytesCount, + bufferMessagesCount: this.bufferMessagesCount, + }) + this.pushOnlyBuffer = [] this.upsertBuffer = {} this.bufferBytesCount = 0 @@ -50,19 +58,6 @@ export class Batch { } } - computeBytesCount(candidate: string) { - // Accurate bytes count computations can degrade performances when there is a lot of events to process - if (!HAS_MULTI_BYTES_CHARACTERS.test(candidate)) { - return candidate.length - } - - if (window.TextEncoder !== undefined) { - return new TextEncoder().encode(candidate).length - } - - return new Blob([candidate]).size - } - private addOrUpdate(message: Context, key?: string) { const { processedMessage, messageBytesCount } = this.process(message) if (messageBytesCount >= this.messageBytesLimit) { diff --git a/packages/core/src/transport/index.ts b/packages/core/src/transport/index.ts index 595f2fccd0..ce16439d4e 100644 --- a/packages/core/src/transport/index.ts +++ b/packages/core/src/transport/index.ts @@ -1,4 +1,4 @@ export { HttpRequest, createHttpRequest, Payload, RetryInfo } from './httpRequest' -export { Batch } from './batch' +export { Batch, BatchFlushEvent } from './batch' export { canUseEventBridge, getEventBridge, BrowserWindowWithEventBridge } from './eventBridge' export { startBatchWithReplica } from './startBatchWithReplica' diff --git a/packages/rum-core/src/transport/startRumBatch.ts b/packages/rum-core/src/transport/startRumBatch.ts index 6fcd801b99..0694f0396e 100644 --- a/packages/rum-core/src/transport/startRumBatch.ts +++ b/packages/rum-core/src/transport/startRumBatch.ts @@ -5,6 +5,7 @@ import type { Observable, RawError, PageExitEvent, + BatchFlushEvent, } from '@datadog/browser-core' import { Batch, combine, createHttpRequest, isTelemetryReplicationAllowed } from '@datadog/browser-core' import type { RumConfiguration } from '../domain/configuration' @@ -31,9 +32,12 @@ export function startRumBatch( }) telemetryEventObservable.subscribe((event) => batch.add(event, isTelemetryReplicationAllowed(configuration))) + + return batch } -interface RumBatch { +export interface RumBatch { + flushObservable: Observable add: (message: Context, replicated?: boolean) => void upsert: (message: Context, key: string) => void } @@ -44,7 +48,6 @@ function makeRumBatch( pageExitObservable: Observable ): RumBatch { const primaryBatch = createRumBatch(configuration.rumEndpointBuilder) - let replicaBatch: Batch | undefined const replica = configuration.replica if (replica !== undefined) { @@ -67,6 +70,7 @@ function makeRumBatch( } return { + flushObservable: primaryBatch.flushObservable, add: (message: Context, replicated = true) => { primaryBatch.add(message) if (replicaBatch && replicated) { From 67ed862ebf9d77cf24471bb8599afa107dc205da Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 13 Jan 2023 16:16:27 +0100 Subject: [PATCH 05/18] Collect user data telemetry --- .../core/src/domain/telemetry/telemetry.ts | 2 + .../rum-core/src/boot/rumPublicApi.spec.ts | 6 +- packages/rum-core/src/boot/rumPublicApi.ts | 2 + packages/rum-core/src/boot/startRum.spec.ts | 10 +- packages/rum-core/src/boot/startRum.ts | 20 ++- packages/rum-core/src/domain/configuration.ts | 2 + .../src/domain/startUserDataTelemetry.spec.ts | 144 ++++++++++++++++++ .../src/domain/startUserDataTelemetry.ts | 119 +++++++++++++++ 8 files changed, 299 insertions(+), 6 deletions(-) create mode 100644 packages/rum-core/src/domain/startUserDataTelemetry.spec.ts create mode 100644 packages/rum-core/src/domain/startUserDataTelemetry.ts diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index c406285faa..4d5334fa8e 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -32,6 +32,7 @@ export const enum TelemetryService { export interface Telemetry { setContextProvider: (provider: () => Context) => void observable: Observable + enabled: boolean } const TELEMETRY_EXCLUDED_SITES: string[] = [INTAKE_SITE_US1_FED] @@ -90,6 +91,7 @@ export function startTelemetry(telemetryService: TelemetryService, configuration contextProvider = provider }, observable, + enabled: telemetryConfiguration.telemetryEnabled, } } diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index 2259b44308..e5cd856610 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -749,7 +749,7 @@ describe('rum public api', () => { rumPublicApi.init(MANUAL_CONFIGURATION) expect(startRumSpy).toHaveBeenCalled() - expect(startRumSpy.calls.argsFor(0)[4]).toEqual({ name: 'foo' }) + expect(startRumSpy.calls.argsFor(0)[6]).toEqual({ name: 'foo' }) expect(recorderApiOnRumStartSpy).toHaveBeenCalled() expect(startViewSpy).not.toHaveBeenCalled() }) @@ -761,7 +761,7 @@ describe('rum public api', () => { rumPublicApi.startView('foo') expect(startRumSpy).toHaveBeenCalled() - expect(startRumSpy.calls.argsFor(0)[4]).toEqual({ name: 'foo' }) + expect(startRumSpy.calls.argsFor(0)[6]).toEqual({ name: 'foo' }) expect(recorderApiOnRumStartSpy).toHaveBeenCalled() expect(startViewSpy).not.toHaveBeenCalled() }) @@ -772,7 +772,7 @@ describe('rum public api', () => { rumPublicApi.startView('bar') expect(startRumSpy).toHaveBeenCalled() - expect(startRumSpy.calls.argsFor(0)[4]).toEqual({ name: 'foo' }) + expect(startRumSpy.calls.argsFor(0)[6]).toEqual({ name: 'foo' }) expect(recorderApiOnRumStartSpy).toHaveBeenCalled() expect(startViewSpy).toHaveBeenCalled() expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'bar' }) diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 49ee143157..31695f667a 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -157,6 +157,8 @@ export function makeRumPublicApi( hasReplay: recorderApi.isRecording() ? true : undefined, }), recorderApi, + globalContextManager, + userContextManager, initialViewOptions ) diff --git a/packages/rum-core/src/boot/startRum.spec.ts b/packages/rum-core/src/boot/startRum.spec.ts index 63203d46db..12788fff86 100644 --- a/packages/rum-core/src/boot/startRum.spec.ts +++ b/packages/rum-core/src/boot/startRum.spec.ts @@ -1,5 +1,6 @@ import type { RelativeTime, Observable, RawError, Duration } from '@datadog/browser-core' import { + createContextManager, stopSessionManager, toServerDuration, ONE_SECOND, @@ -301,7 +302,14 @@ describe('view events', () => { beforeEach(() => { setupBuilder = setup().beforeBuild(({ configuration }) => { - startRum({} as RumInitConfiguration, configuration, () => ({ context: {}, user: {} }), noopRecorderApi) + startRum( + {} as RumInitConfiguration, + configuration, + () => ({ context: {}, user: {} }), + noopRecorderApi, + createContextManager(), + createContextManager() + ) }) interceptor = interceptRequests() }) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 1a0c4151fa..ba43ee2659 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -1,4 +1,4 @@ -import type { Observable, TelemetryEvent, RawError } from '@datadog/browser-core' +import type { Observable, TelemetryEvent, RawError, ContextManager } from '@datadog/browser-core' import { sendToExtension, createPageExitObservable, @@ -33,6 +33,7 @@ import type { RumConfiguration, RumInitConfiguration } from '../domain/configura import { serializeRumConfiguration } from '../domain/configuration' import type { ViewOptions } from '../domain/rumEventsCollection/view/trackViews' import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' +import { startUserDataTelemetry } from '../domain/startUserDataTelemetry' import type { RecorderApi } from './rumPublicApi' export function startRum( @@ -40,6 +41,8 @@ export function startRum( configuration: RumConfiguration, getCommonContext: () => CommonContext, recorderApi: RecorderApi, + globalContextManager: ContextManager, + userContextManager: ContextManager, initialViewOptions?: ViewOptions ) { const lifeCycle = new LifeCycle() @@ -65,12 +68,13 @@ export function startRum( const reportError = (error: RawError) => { lifeCycle.notify(LifeCycleEventType.RAW_ERROR_COLLECTED, { error }) } + let batch if (!canUseEventBridge()) { const pageExitObservable = createPageExitObservable() pageExitObservable.subscribe((event) => { lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, event) }) - startRumBatch(configuration, lifeCycle, telemetry.observable, reportError, pageExitObservable) + batch = startRumBatch(configuration, lifeCycle, telemetry.observable, reportError, pageExitObservable) } else { startRumEventBridge(lifeCycle) } @@ -90,6 +94,18 @@ export function startRum( getCommonContext, reportError ) + + if (batch) { + startUserDataTelemetry( + configuration, + telemetry, + lifeCycle, + globalContextManager, + userContextManager, + featureFlagContexts, + batch.flushObservable + ) + } addTelemetryConfiguration(serializeRumConfiguration(initConfiguration)) startLongTaskCollection(lifeCycle, session) diff --git a/packages/rum-core/src/domain/configuration.ts b/packages/rum-core/src/domain/configuration.ts index bd3d23c1a9..4d689fbaff 100644 --- a/packages/rum-core/src/domain/configuration.ts +++ b/packages/rum-core/src/domain/configuration.ts @@ -80,6 +80,7 @@ export interface RumConfiguration extends Configuration { trackResources: boolean | undefined trackLongTasks: boolean | undefined version?: string + userDataTelemetrySampleRate: number } export function validateAndBuildRumConfiguration( @@ -152,6 +153,7 @@ export function validateAndBuildRumConfiguration( defaultPrivacyLevel: objectHasValue(DefaultPrivacyLevel, initConfiguration.defaultPrivacyLevel) ? initConfiguration.defaultPrivacyLevel : DefaultPrivacyLevel.MASK_USER_INPUT, + userDataTelemetrySampleRate: 1, }, baseConfiguration ) diff --git a/packages/rum-core/src/domain/startUserDataTelemetry.spec.ts b/packages/rum-core/src/domain/startUserDataTelemetry.spec.ts new file mode 100644 index 0000000000..c3f9147024 --- /dev/null +++ b/packages/rum-core/src/domain/startUserDataTelemetry.spec.ts @@ -0,0 +1,144 @@ +import type { BatchFlushEvent, Context, TelemetryEvent } from '@datadog/browser-core' +import { + resetExperimentalFeatures, + updateExperimentalFeatures, + TelemetryService, + startTelemetry, + Observable, +} from '@datadog/browser-core' +import type { TestSetupBuilder } from '../../test/specHelper' +import { setup } from '../../test/specHelper' +import { RumEventType } from '../rawRumEvent.types' +import type { RumEvent } from '../rumEvent.types' +import { LifeCycle, LifeCycleEventType } from './lifeCycle' +import { MEASURES_FLUSH_INTERVAL, startUserDataTelemetry } from './startUserDataTelemetry' + +describe('userDataTelemetry', () => { + let setupBuilder: TestSetupBuilder + let batchFlushObservable: Observable + let telemetryEvents: TelemetryEvent[] + let fakeContextBytesCount: number + let lifeCycle: LifeCycle + + function generateBatch({ + eventNumber, + contextBytesCount, + batchBytesCount, + }: { + eventNumber: number + contextBytesCount: number + batchBytesCount: number + }) { + fakeContextBytesCount = contextBytesCount + for (let index = 0; index < eventNumber; index++) { + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.VIEW } as RumEvent & Context) + } + batchFlushObservable.notify({ bufferBytesCount: batchBytesCount, bufferMessagesCount: eventNumber }) + } + + beforeEach(() => { + updateExperimentalFeatures(['user_data_telemetry']) + setupBuilder = setup() + .withFakeClock() + .withConfiguration({ telemetrySampleRate: 100, userDataTelemetrySampleRate: 100, maxTelemetryEventsPerPage: 2 }) + .beforeBuild(({ globalContextManager, userContextManager, featureFlagContexts, configuration }) => { + batchFlushObservable = new Observable() + lifeCycle = new LifeCycle() + fakeContextBytesCount = 1 + + spyOn(globalContextManager, 'getBytesCount').and.callFake(() => fakeContextBytesCount) + spyOn(userContextManager, 'getBytesCount').and.callFake(() => fakeContextBytesCount) + spyOn(featureFlagContexts, 'getFeatureFlagBytesCount').and.callFake(() => fakeContextBytesCount) + + telemetryEvents = [] + const telemetry = startTelemetry(TelemetryService.RUM, configuration) + telemetry.observable.subscribe((telemetryEvent) => telemetryEvents.push(telemetryEvent)) + + startUserDataTelemetry( + configuration, + telemetry, + lifeCycle, + globalContextManager, + userContextManager, + featureFlagContexts, + batchFlushObservable + ) + }) + }) + + afterEach(() => { + setupBuilder.cleanup() + resetExperimentalFeatures() + }) + + it('should collect user data telemetry', () => { + const { clock } = setupBuilder.build() + + generateBatch({ eventNumber: 2, contextBytesCount: 2, batchBytesCount: 2 }) + generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) + clock.tick(MEASURES_FLUSH_INTERVAL) + + expect(telemetryEvents[0].telemetry).toEqual({ + type: 'log', + status: 'debug', + message: 'User data measures', + batchCount: 2, + batchBytesCount: { min: 1, max: 2, sum: 3 }, + batchMessagesCount: { min: 1, max: 2, sum: 3 }, + globalContextBytes: { min: 1, max: 2, sum: 5 }, + userContextBytes: { min: 1, max: 2, sum: 5 }, + featureFlagBytes: { min: 1, max: 2, sum: 5 }, + }) + }) + + it('should not collect empty contexts telemetry', () => { + const { clock } = setupBuilder.build() + + generateBatch({ eventNumber: 1, contextBytesCount: 0, batchBytesCount: 1 }) + clock.tick(MEASURES_FLUSH_INTERVAL) + + expect(telemetryEvents[0].telemetry.globalContextBytes).not.toBeDefined() + expect(telemetryEvents[0].telemetry.userContextBytes).not.toBeDefined() + expect(telemetryEvents[0].telemetry.featureFlagBytes).not.toBeDefined() + }) + + it('should not collect contexts telemetry of a unfinished batches', () => { + const { clock } = setupBuilder.build() + + generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.VIEW } as RumEvent & Context) + clock.tick(MEASURES_FLUSH_INTERVAL) + + expect(telemetryEvents[0].telemetry).toEqual( + jasmine.objectContaining({ + batchCount: 1, + batchBytesCount: { min: 1, max: 1, sum: 1 }, + batchMessagesCount: { min: 1, max: 1, sum: 1 }, + globalContextBytes: { min: 1, max: 1, sum: 1 }, + userContextBytes: { min: 1, max: 1, sum: 1 }, + featureFlagBytes: { min: 1, max: 1, sum: 1 }, + }) + ) + }) + + it('should not collect user data telemetry when telemetry disabled', () => { + const { clock } = setupBuilder + .withConfiguration({ telemetrySampleRate: 100, userDataTelemetrySampleRate: 0 }) + .build() + + generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) + clock.tick(MEASURES_FLUSH_INTERVAL) + + expect(telemetryEvents.length).toEqual(0) + }) + + it('should not collect user data telemetry when user_data_telemetry ff is disabled', () => { + resetExperimentalFeatures() + const { clock } = setupBuilder.build() + + generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) + clock.tick(MEASURES_FLUSH_INTERVAL) + + expect(telemetryEvents.length).toEqual(0) + }) +}) diff --git a/packages/rum-core/src/domain/startUserDataTelemetry.ts b/packages/rum-core/src/domain/startUserDataTelemetry.ts new file mode 100644 index 0000000000..25e1de3f48 --- /dev/null +++ b/packages/rum-core/src/domain/startUserDataTelemetry.ts @@ -0,0 +1,119 @@ +import type { BatchFlushEvent, Context, ContextManager, Observable, Telemetry } from '@datadog/browser-core' +import { + isExperimentalFeatureEnabled, + performDraw, + ONE_SECOND, + addTelemetryDebug, + monitor, +} from '@datadog/browser-core' +import { RumEventType } from '../rawRumEvent.types' +import type { RumEvent } from '../rumEvent.types' +import type { RumConfiguration } from './configuration' +import type { FeatureFlagContexts } from './contexts/featureFlagContext' +import type { LifeCycle } from './lifeCycle' +import { LifeCycleEventType } from './lifeCycle' + +export const MEASURES_FLUSH_INTERVAL = 10 * ONE_SECOND + +type Measure = { + min: number + max: number + sum: number +} + +let batchCount: number +let batchBytesCount: Measure +let batchMessagesCount: Measure +let globalContextBytes: Measure +let userContextBytes: Measure +let featureFlagBytes: Measure + +let currentBatchGlobalContextBytes: Measure +let currentBatchUserContextBytes: Measure +let currentBatchFeatureFlagBytes: Measure + +export function startUserDataTelemetry( + configuration: RumConfiguration, + telemetry: Telemetry, + lifeCycle: LifeCycle, + globalContextManager: ContextManager, + userContextManager: ContextManager, + featureFlagContexts: FeatureFlagContexts, + batchFlushObservable: Observable +) { + const userDataTelemetryEnabled = telemetry.enabled && performDraw(configuration.userDataTelemetrySampleRate) + if (!userDataTelemetryEnabled || !isExperimentalFeatureEnabled('user_data_telemetry')) { + return + } + + initMeasures() + initCurrentBatchMeasures() + + lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event: RumEvent & Context) => { + updateMeasure(currentBatchGlobalContextBytes, globalContextManager.getBytesCount()) + updateMeasure(currentBatchUserContextBytes, userContextManager.getBytesCount()) + + if ([RumEventType.VIEW, RumEventType.ERROR].includes(event.type as RumEventType)) { + updateMeasure(currentBatchFeatureFlagBytes, featureFlagContexts.getFeatureFlagBytesCount()) + } + }) + + batchFlushObservable.subscribe(({ bufferBytesCount, bufferMessagesCount }) => { + batchCount += 1 + updateMeasure(batchBytesCount, bufferBytesCount) + updateMeasure(batchMessagesCount, bufferMessagesCount) + mergeMeasure(globalContextBytes, currentBatchGlobalContextBytes) + mergeMeasure(userContextBytes, currentBatchUserContextBytes) + mergeMeasure(featureFlagBytes, currentBatchFeatureFlagBytes) + initCurrentBatchMeasures() + }) + + setInterval(monitor(sendMeasures), MEASURES_FLUSH_INTERVAL) +} + +function sendMeasures() { + if (batchCount === 0) { + return + } + + addTelemetryDebug('User data measures', { + batchCount, + batchBytesCount, + batchMessagesCount, + globalContextBytes: globalContextBytes.sum ? globalContextBytes : undefined, + userContextBytes: globalContextBytes.sum ? userContextBytes : undefined, + featureFlagBytes: globalContextBytes.sum ? featureFlagBytes : undefined, + }) + initMeasures() +} + +function createMeasure(): Measure { + return { min: Infinity, max: 0, sum: 0 } +} + +export function updateMeasure(measure: Measure, value: number) { + measure.sum += value + measure.min = Math.min(measure.min, value) + measure.max = Math.max(measure.max, value) +} + +export function mergeMeasure(target: Measure, source: Measure) { + target.sum += source.sum + target.min = Math.min(target.min, source.min) + target.max = Math.max(target.max, source.max) +} + +function initMeasures() { + batchCount = 0 + batchBytesCount = createMeasure() + batchMessagesCount = createMeasure() + globalContextBytes = createMeasure() + userContextBytes = createMeasure() + featureFlagBytes = createMeasure() +} + +function initCurrentBatchMeasures() { + currentBatchGlobalContextBytes = createMeasure() + currentBatchUserContextBytes = createMeasure() + currentBatchFeatureFlagBytes = createMeasure() +} From 0cdb7118af3d1a2c8456d8f0762b139ef4c941ec Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 13 Jan 2023 16:56:29 +0100 Subject: [PATCH 06/18] Fix ie includes compatibility --- packages/rum-core/src/domain/startUserDataTelemetry.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/rum-core/src/domain/startUserDataTelemetry.ts b/packages/rum-core/src/domain/startUserDataTelemetry.ts index 25e1de3f48..67a9e5f408 100644 --- a/packages/rum-core/src/domain/startUserDataTelemetry.ts +++ b/packages/rum-core/src/domain/startUserDataTelemetry.ts @@ -1,5 +1,6 @@ import type { BatchFlushEvent, Context, ContextManager, Observable, Telemetry } from '@datadog/browser-core' import { + includes, isExperimentalFeatureEnabled, performDraw, ONE_SECOND, @@ -53,7 +54,7 @@ export function startUserDataTelemetry( updateMeasure(currentBatchGlobalContextBytes, globalContextManager.getBytesCount()) updateMeasure(currentBatchUserContextBytes, userContextManager.getBytesCount()) - if ([RumEventType.VIEW, RumEventType.ERROR].includes(event.type as RumEventType)) { + if (includes([RumEventType.VIEW, RumEventType.ERROR], event.type)) { updateMeasure(currentBatchFeatureFlagBytes, featureFlagContexts.getFeatureFlagBytesCount()) } }) From 14cfaa4c201fc1bd28f1c3f10968236f9177446a Mon Sep 17 00:00:00 2001 From: Aymeric Date: Tue, 17 Jan 2023 14:18:15 +0100 Subject: [PATCH 07/18] =?UTF-8?q?=F0=9F=91=8CSimplify=20bytes=20count=20ca?= =?UTF-8?q?che=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/index.ts | 2 +- .../core/src/tools/contextManager.spec.ts | 63 +++++-------------- packages/core/src/tools/contextManager.ts | 44 +++++-------- packages/core/test/specHelper.ts | 13 ++-- .../contexts/featureFlagContext.spec.ts | 39 +++++------- .../src/domain/contexts/featureFlagContext.ts | 15 +++-- 6 files changed, 66 insertions(+), 110 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 91869e0c64..cec5375267 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -82,7 +82,7 @@ export * from './browser/addEventListener' export { initConsoleObservable, ConsoleLog } from './domain/console/consoleObservable' export { BoundedBuffer } from './tools/boundedBuffer' export { catchUserErrors } from './tools/catchUserErrors' -export { createContextManager, contextBytesCounter, ContextManager } from './tools/contextManager' +export { createContextManager, ContextManager } from './tools/contextManager' export { limitModification } from './tools/limitModification' export { ContextHistory, ContextHistoryEntry, CLEAR_OLD_CONTEXTS_INTERVAL } from './tools/contextHistory' export { readBytesFromStream } from './tools/readBytesFromStream' diff --git a/packages/core/src/tools/contextManager.spec.ts b/packages/core/src/tools/contextManager.spec.ts index c00035e3a7..5729dad0fa 100644 --- a/packages/core/src/tools/contextManager.spec.ts +++ b/packages/core/src/tools/contextManager.spec.ts @@ -1,6 +1,4 @@ -import { contextBytesCounterStub } from '../../test/specHelper' -import { contextBytesCounter, createContextManager } from './contextManager' -import { computeBytesCount } from './utils' +import { createContextManager } from './contextManager' describe('createContextManager', () => { it('starts with an empty context', () => { @@ -76,55 +74,28 @@ describe('createContextManager', () => { expect(manager.getContext()).toEqual({}) }) - it('should invalidate the context bytes counter at each mutation', () => { - const bytesCountStub = contextBytesCounterStub() - const manager = createContextManager(bytesCountStub) + it('should compute the bytes count only if a the context has been updated', () => { + const computeBytesCountStub = jasmine.createSpy('computeBytesCountStub').and.returnValue(1) + const manager = createContextManager(computeBytesCountStub) manager.add('foo', 'bar') - manager.remove('foo') - manager.set({ foo: 'bar' }) - manager.removeContextProperty('foo') - manager.setContext({ foo: 'bar' }) - manager.clearContext() + manager.getBytesCount() - expect(bytesCountStub.invalidate).toHaveBeenCalledTimes(6) - }) - - it('should get the context bytes count', () => { - const bytesCountStub = contextBytesCounterStub() - const manager = createContextManager(bytesCountStub) - const contextBytesCount = manager.getBytesCount() - - expect(contextBytesCount).toEqual(1) - }) -}) - -describe('contextBytesCounter', () => { - let computeBytesCountSpy: jasmine.Spy - let counter: ReturnType + manager.remove('foo') + manager.getBytesCount() - beforeEach(() => { - computeBytesCountSpy = jasmine.createSpy('computeBytesCount', computeBytesCount).and.callThrough() - counter = contextBytesCounter(computeBytesCountSpy) - }) + manager.set({ foo: 'bar' }) + manager.getBytesCount() - it('should compute the batch count when the cache is invalidate', () => { - const bytesCount1 = counter.compute({ a: 'b' }) - counter.invalidate() - const bytesCount2 = counter.compute({ foo: 'bar' }) + manager.removeContextProperty('foo') + manager.getBytesCount() - expect(computeBytesCountSpy).toHaveBeenCalledTimes(2) - expect(bytesCount1).not.toEqual(bytesCount2) - }) + manager.setContext({ foo: 'bar' }) + manager.getBytesCount() - it('should use the cached bytes count when the cache is not invalidate', () => { - const bytesCount1 = counter.compute({ a: 'b' }) - const bytesCount2 = counter.compute({ foo: 'bar' }) - expect(computeBytesCountSpy).toHaveBeenCalledTimes(1) - expect(bytesCount1).toEqual(bytesCount2) - }) + manager.clearContext() + manager.getBytesCount() + manager.getBytesCount() - it('should return a bytes count at 0 when the object is empty', () => { - const bytesCount = counter.compute({}) - expect(bytesCount).toEqual(0) + expect(computeBytesCountStub).toHaveBeenCalledTimes(6) }) }) diff --git a/packages/core/src/tools/contextManager.ts b/packages/core/src/tools/contextManager.ts index abd54932b2..3755025dc6 100644 --- a/packages/core/src/tools/contextManager.ts +++ b/packages/core/src/tools/contextManager.ts @@ -1,72 +1,60 @@ -import { computeBytesCount, deepClone, isEmptyObject, jsonStringify } from './utils' +import { computeBytesCount, deepClone, jsonStringify } from './utils' import type { Context, ContextValue } from './context' export type ContextManager = ReturnType -export function createContextManager(bytesCounter = contextBytesCounter()) { +export function createContextManager(computeBytesCountImpl = computeBytesCount) { let context: Context = {} + let bytesCountCache: number | undefined return { - getBytesCount: () => bytesCounter.compute(context), + getBytesCount: () => { + if (bytesCountCache === undefined) { + bytesCountCache = computeBytesCountImpl(jsonStringify(context)!) + } + return bytesCountCache + }, /** @deprecated use getContext instead */ get: () => context, /** @deprecated use setContextProperty instead */ add: (key: string, value: any) => { context[key] = value as ContextValue - bytesCounter.invalidate() + bytesCountCache = undefined }, /** @deprecated renamed to removeContextProperty */ remove: (key: string) => { delete context[key] - bytesCounter.invalidate() + bytesCountCache = undefined }, /** @deprecated use setContext instead */ set: (newContext: object) => { context = newContext as Context - bytesCounter.invalidate() + bytesCountCache = undefined }, getContext: () => deepClone(context), setContext: (newContext: Context) => { context = deepClone(newContext) - bytesCounter.invalidate() + bytesCountCache = undefined }, setContextProperty: (key: string, property: any) => { context[key] = deepClone(property) - bytesCounter.invalidate() + bytesCountCache = undefined }, removeContextProperty: (key: string) => { delete context[key] - bytesCounter.invalidate() + bytesCountCache = undefined }, clearContext: () => { context = {} - bytesCounter.invalidate() - }, - } -} - -export type ContextBytesCounter = ReturnType - -export function contextBytesCounter(computeBytesCountImpl = computeBytesCount) { - let bytesCount: number | undefined - - return { - compute: (context: Context) => { - if (bytesCount === undefined) { - bytesCount = !isEmptyObject(context) ? computeBytesCountImpl(jsonStringify(context)!) : 0 - } - return bytesCount - }, - invalidate: () => { - bytesCount = undefined + bytesCountCache = undefined }, } } diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 444ba533f8..b900250315 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -1,5 +1,4 @@ import type { EndpointBuilder } from '../src/domain/configuration' -import type { ContextBytesCounter } from '../src/tools/contextManager' import { instrumentMethod } from '../src/tools/instrumentMethod' import { resetNavigationStart } from '../src/tools/timeUtils' import { buildUrl } from '../src/tools/urlPolyfill' @@ -500,12 +499,12 @@ export function interceptRequests() { } } -export function contextBytesCounterStub(): ContextBytesCounter { - return { - compute: () => 1, - invalidate: jasmine.createSpy('invalidate'), - } -} +// export function contextBytesCounterStub(): ContextBytesCounter { +// return { +// compute: () => 1, +// invalidate: jasmine.createSpy('invalidate'), +// } +// } export function isAdoptedStyleSheetsSupported() { return Boolean((document as any).adoptedStyleSheets) diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts index 762524823b..9628ba011d 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts @@ -1,7 +1,5 @@ import type { RelativeTime } from '@datadog/browser-core' import { resetExperimentalFeatures, updateExperimentalFeatures, relativeToClocks } from '@datadog/browser-core' -import type { ContextBytesCounter } from '../../../../core/src/tools/contextManager' -import { contextBytesCounterStub } from '../../../../core/test/specHelper' import type { TestSetupBuilder } from '../../../test/specHelper' import { setup } from '../../../test/specHelper' import { LifeCycleEventType } from '../lifeCycle' @@ -12,12 +10,12 @@ import { startFeatureFlagContexts } from './featureFlagContext' describe('featureFlagContexts', () => { let setupBuilder: TestSetupBuilder let featureFlagContexts: FeatureFlagContexts - let bytesCounterStub: ContextBytesCounter + let computeBytesCountStub: jasmine.Spy beforeEach(() => { setupBuilder = setup().beforeBuild(({ lifeCycle }) => { - bytesCounterStub = contextBytesCounterStub() - featureFlagContexts = startFeatureFlagContexts(lifeCycle, bytesCounterStub) + computeBytesCountStub = jasmine.createSpy('computeBytesCountStub').and.returnValue(1) + featureFlagContexts = startFeatureFlagContexts(lifeCycle, computeBytesCountStub) }) }) @@ -88,18 +86,6 @@ describe('featureFlagContexts', () => { expect(featureFlagContext).toBeUndefined() }) - - it('should invalidate the context bytes count when a new feature flag is added or a view is created', () => { - updateExperimentalFeatures(['feature_flags']) - - const { lifeCycle } = setupBuilder.withFakeClock().build() - lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { - startClocks: relativeToClocks(0 as RelativeTime), - } as ViewCreatedEvent) - - featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo') - expect(bytesCounterStub.invalidate).toHaveBeenCalledTimes(2) - }) }) describe('findFeatureFlagEvaluations', () => { @@ -162,19 +148,26 @@ describe('featureFlagContexts', () => { }) describe('getFeatureFlagBytesCount', () => { - it('should get the context bytes count', () => { + it('should compute the bytes count only if a the context has been updated', () => { updateExperimentalFeatures(['feature_flags']) - const { lifeCycle } = setupBuilder.withFakeClock().build() + lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { startClocks: relativeToClocks(0 as RelativeTime), } as ViewCreatedEvent) + featureFlagContexts.addFeatureFlagEvaluation('feature1', 'foo') + featureFlagContexts.getFeatureFlagBytesCount() + featureFlagContexts.addFeatureFlagEvaluation('feature2', 'bar') + featureFlagContexts.getFeatureFlagBytesCount() - featureFlagContexts.addFeatureFlagEvaluation('foo', 'bar') - - const bytesCount = featureFlagContexts.getFeatureFlagBytesCount() + // feature flags are cleared when a view is created + lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { + startClocks: relativeToClocks(10 as RelativeTime), + } as ViewCreatedEvent) + featureFlagContexts.getFeatureFlagBytesCount() + featureFlagContexts.getFeatureFlagBytesCount() - expect(bytesCount).toEqual(1) + expect(computeBytesCountStub).toHaveBeenCalledTimes(3) }) }) }) diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.ts index 0428e10188..9869a2ec2f 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.ts @@ -1,6 +1,7 @@ import type { RelativeTime, ContextValue, Context } from '@datadog/browser-core' import { - contextBytesCounter, + jsonStringify, + computeBytesCount, deepClone, noop, isExperimentalFeatureEnabled, @@ -30,7 +31,7 @@ export interface FeatureFlagContexts { */ export function startFeatureFlagContexts( lifeCycle: LifeCycle, - bytesCounter = contextBytesCounter() + computeBytesCountImpl = computeBytesCount ): FeatureFlagContexts { if (!isExperimentalFeatureEnabled('feature_flags')) { return { @@ -41,6 +42,7 @@ export function startFeatureFlagContexts( } const featureFlagContexts = new ContextHistory(FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY) + let bytesCountCache: number | undefined lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => { featureFlagContexts.closeActive(endClocks.relative) @@ -48,7 +50,7 @@ export function startFeatureFlagContexts( lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => { featureFlagContexts.add({}, startClocks.relative) - bytesCounter.invalidate() + bytesCountCache = undefined }) return { @@ -59,13 +61,16 @@ export function startFeatureFlagContexts( return 0 } - return bytesCounter.compute(currentContext) + if (bytesCountCache === undefined) { + bytesCountCache = computeBytesCountImpl(jsonStringify(currentContext)!) + } + return bytesCountCache }, addFeatureFlagEvaluation: (key: string, value: ContextValue) => { const currentContext = featureFlagContexts.find() if (currentContext) { currentContext[key] = deepClone(value) - bytesCounter.invalidate() + bytesCountCache = undefined } }, } From 2050070f09a32c09efda765a455dcfb7a960d862 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Tue, 17 Jan 2023 14:18:49 +0100 Subject: [PATCH 08/18] =?UTF-8?q?=F0=9F=91=8CCollocate=20startUserDataTele?= =?UTF-8?q?metry=20and=20startRumBatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum-core/src/boot/startRum.spec.ts | 9 ++-- packages/rum-core/src/boot/startRum.ts | 46 ++++++++++----------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/packages/rum-core/src/boot/startRum.spec.ts b/packages/rum-core/src/boot/startRum.spec.ts index 12788fff86..45885af6f2 100644 --- a/packages/rum-core/src/boot/startRum.spec.ts +++ b/packages/rum-core/src/boot/startRum.spec.ts @@ -25,6 +25,7 @@ import { startLongTaskCollection } from '../domain/rumEventsCollection/longTask/ import type { RumSessionManager } from '..' import type { RumConfiguration, RumInitConfiguration } from '../domain/configuration' import { RumEventType } from '../rawRumEvent.types' +import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' import { startRum, startRumEventCollection } from './startRum' function collectServerEvents(lifeCycle: LifeCycle) { @@ -44,11 +45,7 @@ function startRumStub( locationChangeObservable: Observable, reportError: (error: RawError) => void ) { - const { - stop: rumEventCollectionStop, - foregroundContexts, - featureFlagContexts, - } = startRumEventCollection( + const { stop: rumEventCollectionStop, foregroundContexts } = startRumEventCollection( lifeCycle, configuration, location, @@ -68,7 +65,7 @@ function startRumStub( domMutationObservable, locationChangeObservable, foregroundContexts, - featureFlagContexts, + startFeatureFlagContexts(lifeCycle), noopRecorderApi ) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index ef21cef9bd..2855d4fe96 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -69,34 +69,14 @@ export function startRum( const reportError = (error: RawError) => { lifeCycle.notify(LifeCycleEventType.RAW_ERROR_COLLECTED, { error }) } - let batch + const featureFlagContexts = startFeatureFlagContexts(lifeCycle) + if (!canUseEventBridge()) { const pageExitObservable = createPageExitObservable() pageExitObservable.subscribe((event) => { lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, event) }) - batch = startRumBatch(configuration, lifeCycle, telemetry.observable, reportError, pageExitObservable) - } else { - startRumEventBridge(lifeCycle) - } - - const session = !canUseEventBridge() ? startRumSessionManager(configuration, lifeCycle) : startRumSessionManagerStub() - const domMutationObservable = createDOMMutationObservable() - const locationChangeObservable = createLocationChangeObservable(location) - - const { viewContexts, foregroundContexts, featureFlagContexts, urlContexts, actionContexts, addAction } = - startRumEventCollection( - lifeCycle, - configuration, - location, - session, - locationChangeObservable, - domMutationObservable, - getCommonContext, - reportError - ) - - if (batch) { + const batch = startRumBatch(configuration, lifeCycle, telemetry.observable, reportError, pageExitObservable) startUserDataTelemetry( configuration, telemetry, @@ -106,7 +86,25 @@ export function startRum( featureFlagContexts, batch.flushObservable ) + } else { + startRumEventBridge(lifeCycle) } + + const session = !canUseEventBridge() ? startRumSessionManager(configuration, lifeCycle) : startRumSessionManagerStub() + const domMutationObservable = createDOMMutationObservable() + const locationChangeObservable = createLocationChangeObservable(location) + + const { viewContexts, foregroundContexts, urlContexts, actionContexts, addAction } = startRumEventCollection( + lifeCycle, + configuration, + location, + session, + locationChangeObservable, + domMutationObservable, + getCommonContext, + reportError + ) + addTelemetryConfiguration(serializeRumConfiguration(initConfiguration)) startLongTaskCollection(lifeCycle, session) @@ -170,7 +168,6 @@ export function startRumEventCollection( ) { const viewContexts = startViewContexts(lifeCycle) const urlContexts = startUrlContexts(lifeCycle, locationChangeObservable, location) - const featureFlagContexts = startFeatureFlagContexts(lifeCycle) const foregroundContexts = startForegroundContexts() const { addAction, actionContexts } = startActionCollection( @@ -195,7 +192,6 @@ export function startRumEventCollection( viewContexts, foregroundContexts, urlContexts, - featureFlagContexts, addAction, actionContexts, stop: () => { From 5dd94b349f38ec3992224041233f46c9414c2d0e Mon Sep 17 00:00:00 2001 From: Aymeric Date: Tue, 17 Jan 2023 14:37:02 +0100 Subject: [PATCH 09/18] =?UTF-8?q?=F0=9F=91=8CRename=20userData=20to=20cust?= =?UTF-8?q?omerData?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum-core/src/boot/startRum.ts | 4 ++-- packages/rum-core/src/domain/configuration.ts | 4 ++-- ....ts => startCustomerDataTelemetry.spec.ts} | 24 +++++++++++-------- ...metry.ts => startCustomerDataTelemetry.ts} | 9 +++---- 4 files changed, 23 insertions(+), 18 deletions(-) rename packages/rum-core/src/domain/{startUserDataTelemetry.spec.ts => startCustomerDataTelemetry.spec.ts} (85%) rename packages/rum-core/src/domain/{startUserDataTelemetry.ts => startCustomerDataTelemetry.ts} (91%) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 2855d4fe96..242daacb1b 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -33,7 +33,7 @@ import type { RumConfiguration, RumInitConfiguration } from '../domain/configura import { serializeRumConfiguration } from '../domain/configuration' import type { ViewOptions } from '../domain/rumEventsCollection/view/trackViews' import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' -import { startUserDataTelemetry } from '../domain/startUserDataTelemetry' +import { startCustomerDataTelemetry } from '../domain/startCustomerDataTelemetry' import { startPageStateHistory } from '../domain/contexts/pageStateHistory' import type { RecorderApi } from './rumPublicApi' @@ -77,7 +77,7 @@ export function startRum( lifeCycle.notify(LifeCycleEventType.PAGE_EXITED, event) }) const batch = startRumBatch(configuration, lifeCycle, telemetry.observable, reportError, pageExitObservable) - startUserDataTelemetry( + startCustomerDataTelemetry( configuration, telemetry, lifeCycle, diff --git a/packages/rum-core/src/domain/configuration.ts b/packages/rum-core/src/domain/configuration.ts index 4d689fbaff..556cfa8d82 100644 --- a/packages/rum-core/src/domain/configuration.ts +++ b/packages/rum-core/src/domain/configuration.ts @@ -80,7 +80,7 @@ export interface RumConfiguration extends Configuration { trackResources: boolean | undefined trackLongTasks: boolean | undefined version?: string - userDataTelemetrySampleRate: number + customerDataTelemetrySampleRate: number } export function validateAndBuildRumConfiguration( @@ -153,7 +153,7 @@ export function validateAndBuildRumConfiguration( defaultPrivacyLevel: objectHasValue(DefaultPrivacyLevel, initConfiguration.defaultPrivacyLevel) ? initConfiguration.defaultPrivacyLevel : DefaultPrivacyLevel.MASK_USER_INPUT, - userDataTelemetrySampleRate: 1, + customerDataTelemetrySampleRate: 1, }, baseConfiguration ) diff --git a/packages/rum-core/src/domain/startUserDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts similarity index 85% rename from packages/rum-core/src/domain/startUserDataTelemetry.spec.ts rename to packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index c3f9147024..0229f087f7 100644 --- a/packages/rum-core/src/domain/startUserDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -11,9 +11,9 @@ import { setup } from '../../test/specHelper' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' import { LifeCycle, LifeCycleEventType } from './lifeCycle' -import { MEASURES_FLUSH_INTERVAL, startUserDataTelemetry } from './startUserDataTelemetry' +import { MEASURES_FLUSH_INTERVAL, startCustomerDataTelemetry } from './startCustomerDataTelemetry' -describe('userDataTelemetry', () => { +describe('customerDataTelemetry', () => { let setupBuilder: TestSetupBuilder let batchFlushObservable: Observable let telemetryEvents: TelemetryEvent[] @@ -37,10 +37,14 @@ describe('userDataTelemetry', () => { } beforeEach(() => { - updateExperimentalFeatures(['user_data_telemetry']) + updateExperimentalFeatures(['customer_data_telemetry']) setupBuilder = setup() .withFakeClock() - .withConfiguration({ telemetrySampleRate: 100, userDataTelemetrySampleRate: 100, maxTelemetryEventsPerPage: 2 }) + .withConfiguration({ + telemetrySampleRate: 100, + customerDataTelemetrySampleRate: 100, + maxTelemetryEventsPerPage: 2, + }) .beforeBuild(({ globalContextManager, userContextManager, featureFlagContexts, configuration }) => { batchFlushObservable = new Observable() lifeCycle = new LifeCycle() @@ -54,7 +58,7 @@ describe('userDataTelemetry', () => { const telemetry = startTelemetry(TelemetryService.RUM, configuration) telemetry.observable.subscribe((telemetryEvent) => telemetryEvents.push(telemetryEvent)) - startUserDataTelemetry( + startCustomerDataTelemetry( configuration, telemetry, lifeCycle, @@ -71,7 +75,7 @@ describe('userDataTelemetry', () => { resetExperimentalFeatures() }) - it('should collect user data telemetry', () => { + it('should collect customer data telemetry', () => { const { clock } = setupBuilder.build() generateBatch({ eventNumber: 2, contextBytesCount: 2, batchBytesCount: 2 }) @@ -81,7 +85,7 @@ describe('userDataTelemetry', () => { expect(telemetryEvents[0].telemetry).toEqual({ type: 'log', status: 'debug', - message: 'User data measures', + message: 'Customer data measures', batchCount: 2, batchBytesCount: { min: 1, max: 2, sum: 3 }, batchMessagesCount: { min: 1, max: 2, sum: 3 }, @@ -121,9 +125,9 @@ describe('userDataTelemetry', () => { ) }) - it('should not collect user data telemetry when telemetry disabled', () => { + it('should not collect customer data telemetry when telemetry disabled', () => { const { clock } = setupBuilder - .withConfiguration({ telemetrySampleRate: 100, userDataTelemetrySampleRate: 0 }) + .withConfiguration({ telemetrySampleRate: 100, customerDataTelemetrySampleRate: 0 }) .build() generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) @@ -132,7 +136,7 @@ describe('userDataTelemetry', () => { expect(telemetryEvents.length).toEqual(0) }) - it('should not collect user data telemetry when user_data_telemetry ff is disabled', () => { + it('should not collect customer data telemetry when customer_data_telemetry ff is disabled', () => { resetExperimentalFeatures() const { clock } = setupBuilder.build() diff --git a/packages/rum-core/src/domain/startUserDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts similarity index 91% rename from packages/rum-core/src/domain/startUserDataTelemetry.ts rename to packages/rum-core/src/domain/startCustomerDataTelemetry.ts index 67a9e5f408..510d9fca60 100644 --- a/packages/rum-core/src/domain/startUserDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -33,7 +33,7 @@ let currentBatchGlobalContextBytes: Measure let currentBatchUserContextBytes: Measure let currentBatchFeatureFlagBytes: Measure -export function startUserDataTelemetry( +export function startCustomerDataTelemetry( configuration: RumConfiguration, telemetry: Telemetry, lifeCycle: LifeCycle, @@ -42,8 +42,9 @@ export function startUserDataTelemetry( featureFlagContexts: FeatureFlagContexts, batchFlushObservable: Observable ) { - const userDataTelemetryEnabled = telemetry.enabled && performDraw(configuration.userDataTelemetrySampleRate) - if (!userDataTelemetryEnabled || !isExperimentalFeatureEnabled('user_data_telemetry')) { + const customerDataTelemetrySampleRate = + telemetry.enabled && performDraw(configuration.customerDataTelemetrySampleRate) + if (!customerDataTelemetrySampleRate || !isExperimentalFeatureEnabled('customer_data_telemetry')) { return } @@ -77,7 +78,7 @@ function sendMeasures() { return } - addTelemetryDebug('User data measures', { + addTelemetryDebug('Customer data measures', { batchCount, batchBytesCount, batchMessagesCount, From 23fb2198df8b7b4828768a9eba3dab417ce5b83b Mon Sep 17 00:00:00 2001 From: Aymeric Date: Tue, 17 Jan 2023 15:48:26 +0100 Subject: [PATCH 10/18] =?UTF-8?q?=F0=9F=91=8C=20Reinforce=20period=20conce?= =?UTF-8?q?pt=20in=20customer=20data=20telemetry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/startCustomerDataTelemetry.spec.ts | 12 +-- .../src/domain/startCustomerDataTelemetry.ts | 90 +++++++++++-------- 2 files changed, 57 insertions(+), 45 deletions(-) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index 0229f087f7..92dbf48efd 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -11,7 +11,7 @@ import { setup } from '../../test/specHelper' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' import { LifeCycle, LifeCycleEventType } from './lifeCycle' -import { MEASURES_FLUSH_INTERVAL, startCustomerDataTelemetry } from './startCustomerDataTelemetry' +import { MEASURES_PERIOD_DURATION, startCustomerDataTelemetry } from './startCustomerDataTelemetry' describe('customerDataTelemetry', () => { let setupBuilder: TestSetupBuilder @@ -80,7 +80,7 @@ describe('customerDataTelemetry', () => { generateBatch({ eventNumber: 2, contextBytesCount: 2, batchBytesCount: 2 }) generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) - clock.tick(MEASURES_FLUSH_INTERVAL) + clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents[0].telemetry).toEqual({ type: 'log', @@ -99,7 +99,7 @@ describe('customerDataTelemetry', () => { const { clock } = setupBuilder.build() generateBatch({ eventNumber: 1, contextBytesCount: 0, batchBytesCount: 1 }) - clock.tick(MEASURES_FLUSH_INTERVAL) + clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents[0].telemetry.globalContextBytes).not.toBeDefined() expect(telemetryEvents[0].telemetry.userContextBytes).not.toBeDefined() @@ -111,7 +111,7 @@ describe('customerDataTelemetry', () => { generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.VIEW } as RumEvent & Context) - clock.tick(MEASURES_FLUSH_INTERVAL) + clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents[0].telemetry).toEqual( jasmine.objectContaining({ @@ -131,7 +131,7 @@ describe('customerDataTelemetry', () => { .build() generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) - clock.tick(MEASURES_FLUSH_INTERVAL) + clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents.length).toEqual(0) }) @@ -141,7 +141,7 @@ describe('customerDataTelemetry', () => { const { clock } = setupBuilder.build() generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) - clock.tick(MEASURES_FLUSH_INTERVAL) + clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents.length).toEqual(0) }) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index 510d9fca60..9bb6f95925 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -14,7 +14,7 @@ import type { FeatureFlagContexts } from './contexts/featureFlagContext' import type { LifeCycle } from './lifeCycle' import { LifeCycleEventType } from './lifeCycle' -export const MEASURES_FLUSH_INTERVAL = 10 * ONE_SECOND +export const MEASURES_PERIOD_DURATION = 10 * ONE_SECOND type Measure = { min: number @@ -22,16 +22,23 @@ type Measure = { sum: number } -let batchCount: number -let batchBytesCount: Measure -let batchMessagesCount: Measure -let globalContextBytes: Measure -let userContextBytes: Measure -let featureFlagBytes: Measure +type CurrentPeriodMeasures = { + batchCount: number + batchBytesCount: Measure + batchMessagesCount: Measure + globalContextBytes: Measure + userContextBytes: Measure + featureFlagBytes: Measure +} + +type CurrentBatchMeasures = { + globalContextBytes: Measure + userContextBytes: Measure + featureFlagBytes: Measure +} -let currentBatchGlobalContextBytes: Measure -let currentBatchUserContextBytes: Measure -let currentBatchFeatureFlagBytes: Measure +let currentPeriodMeasures: CurrentPeriodMeasures +let currentBatchMeasures: CurrentBatchMeasures export function startCustomerDataTelemetry( configuration: RumConfiguration, @@ -48,74 +55,79 @@ export function startCustomerDataTelemetry( return } - initMeasures() + initCurrentPeriodMeasures() initCurrentBatchMeasures() lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event: RumEvent & Context) => { - updateMeasure(currentBatchGlobalContextBytes, globalContextManager.getBytesCount()) - updateMeasure(currentBatchUserContextBytes, userContextManager.getBytesCount()) + updateMeasure(currentBatchMeasures.globalContextBytes, globalContextManager.getBytesCount()) + updateMeasure(currentBatchMeasures.userContextBytes, userContextManager.getBytesCount()) if (includes([RumEventType.VIEW, RumEventType.ERROR], event.type)) { - updateMeasure(currentBatchFeatureFlagBytes, featureFlagContexts.getFeatureFlagBytesCount()) + updateMeasure(currentBatchMeasures.featureFlagBytes, featureFlagContexts.getFeatureFlagBytesCount()) } }) batchFlushObservable.subscribe(({ bufferBytesCount, bufferMessagesCount }) => { - batchCount += 1 - updateMeasure(batchBytesCount, bufferBytesCount) - updateMeasure(batchMessagesCount, bufferMessagesCount) - mergeMeasure(globalContextBytes, currentBatchGlobalContextBytes) - mergeMeasure(userContextBytes, currentBatchUserContextBytes) - mergeMeasure(featureFlagBytes, currentBatchFeatureFlagBytes) + currentPeriodMeasures.batchCount += 1 + updateMeasure(currentPeriodMeasures.batchBytesCount, bufferBytesCount) + updateMeasure(currentPeriodMeasures.batchMessagesCount, bufferMessagesCount) + mergeMeasure(currentPeriodMeasures.globalContextBytes, currentBatchMeasures.globalContextBytes) + mergeMeasure(currentPeriodMeasures.userContextBytes, currentBatchMeasures.userContextBytes) + mergeMeasure(currentPeriodMeasures.featureFlagBytes, currentBatchMeasures.featureFlagBytes) initCurrentBatchMeasures() }) - setInterval(monitor(sendMeasures), MEASURES_FLUSH_INTERVAL) + setInterval(monitor(sendCurrentPeriodMeasures), MEASURES_PERIOD_DURATION) } -function sendMeasures() { - if (batchCount === 0) { +function sendCurrentPeriodMeasures() { + if (currentPeriodMeasures.batchCount === 0) { return } - + const { batchCount, batchBytesCount, batchMessagesCount, globalContextBytes, userContextBytes, featureFlagBytes } = + currentPeriodMeasures addTelemetryDebug('Customer data measures', { batchCount, batchBytesCount, batchMessagesCount, globalContextBytes: globalContextBytes.sum ? globalContextBytes : undefined, - userContextBytes: globalContextBytes.sum ? userContextBytes : undefined, - featureFlagBytes: globalContextBytes.sum ? featureFlagBytes : undefined, + userContextBytes: userContextBytes.sum ? userContextBytes : undefined, + featureFlagBytes: featureFlagBytes.sum ? featureFlagBytes : undefined, }) - initMeasures() + initCurrentPeriodMeasures() } function createMeasure(): Measure { return { min: Infinity, max: 0, sum: 0 } } -export function updateMeasure(measure: Measure, value: number) { +function updateMeasure(measure: Measure, value: number) { measure.sum += value measure.min = Math.min(measure.min, value) measure.max = Math.max(measure.max, value) } -export function mergeMeasure(target: Measure, source: Measure) { +function mergeMeasure(target: Measure, source: Measure) { target.sum += source.sum target.min = Math.min(target.min, source.min) target.max = Math.max(target.max, source.max) } -function initMeasures() { - batchCount = 0 - batchBytesCount = createMeasure() - batchMessagesCount = createMeasure() - globalContextBytes = createMeasure() - userContextBytes = createMeasure() - featureFlagBytes = createMeasure() +function initCurrentPeriodMeasures() { + currentPeriodMeasures = { + batchCount: 0, + batchBytesCount: createMeasure(), + batchMessagesCount: createMeasure(), + globalContextBytes: createMeasure(), + userContextBytes: createMeasure(), + featureFlagBytes: createMeasure(), + } } function initCurrentBatchMeasures() { - currentBatchGlobalContextBytes = createMeasure() - currentBatchUserContextBytes = createMeasure() - currentBatchFeatureFlagBytes = createMeasure() + currentBatchMeasures = { + globalContextBytes: createMeasure(), + userContextBytes: createMeasure(), + featureFlagBytes: createMeasure(), + } } From 07ebbc84c566a41caf598be3e48e1e83f6ab3db2 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Tue, 17 Jan 2023 15:54:58 +0100 Subject: [PATCH 11/18] =?UTF-8?q?=F0=9F=91=8CAlso=20check=20TELEMETRY=5FEX?= =?UTF-8?q?CLUDED=5FSITES?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/domain/telemetry/telemetry.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index 4d5334fa8e..8e10f27144 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -50,12 +50,13 @@ export function startTelemetry(telemetryService: TelemetryService, configuration let contextProvider: () => Context const observable = new Observable() - telemetryConfiguration.telemetryEnabled = performDraw(configuration.telemetrySampleRate) + telemetryConfiguration.telemetryEnabled = + !includes(TELEMETRY_EXCLUDED_SITES, configuration.site) && performDraw(configuration.telemetrySampleRate) telemetryConfiguration.telemetryConfigurationEnabled = telemetryConfiguration.telemetryEnabled && performDraw(configuration.telemetryConfigurationSampleRate) onRawTelemetryEventCollected = (rawEvent: RawTelemetryEvent) => { - if (!includes(TELEMETRY_EXCLUDED_SITES, configuration.site) && telemetryConfiguration.telemetryEnabled) { + if (telemetryConfiguration.telemetryEnabled) { const event = toTelemetryEvent(telemetryService, rawEvent) observable.notify(event) sendToExtension('telemetry', event) From 99322783bc4e45699246f49f2f42c71dff7c9500 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Wed, 18 Jan 2023 09:44:35 +0100 Subject: [PATCH 12/18] =?UTF-8?q?=F0=9F=91=8CCheck=20if=20context=20are=20?= =?UTF-8?q?empty=20in=20customerDataTelemetry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/startCustomerDataTelemetry.spec.ts | 41 +++++++++++++------ .../src/domain/startCustomerDataTelemetry.ts | 16 ++++++-- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index 92dbf48efd..acbe0dd8f1 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -1,4 +1,4 @@ -import type { BatchFlushEvent, Context, TelemetryEvent } from '@datadog/browser-core' +import type { BatchFlushEvent, Context, ContextManager, TelemetryEvent } from '@datadog/browser-core' import { resetExperimentalFeatures, updateExperimentalFeatures, @@ -10,6 +10,7 @@ import type { TestSetupBuilder } from '../../test/specHelper' import { setup } from '../../test/specHelper' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' +import type { FeatureFlagContexts } from './contexts/featureFlagContext' import { LifeCycle, LifeCycleEventType } from './lifeCycle' import { MEASURES_PERIOD_DURATION, startCustomerDataTelemetry } from './startCustomerDataTelemetry' @@ -17,25 +18,40 @@ describe('customerDataTelemetry', () => { let setupBuilder: TestSetupBuilder let batchFlushObservable: Observable let telemetryEvents: TelemetryEvent[] + let fakeContext: Context let fakeContextBytesCount: number let lifeCycle: LifeCycle function generateBatch({ eventNumber, - contextBytesCount, - batchBytesCount, + batchBytesCount = 1, + contextBytesCount = fakeContextBytesCount, + context = fakeContext, }: { eventNumber: number - contextBytesCount: number - batchBytesCount: number + batchBytesCount?: number + contextBytesCount?: number + context?: Context }) { fakeContextBytesCount = contextBytesCount + fakeContext = context + for (let index = 0; index < eventNumber; index++) { lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.VIEW } as RumEvent & Context) } batchFlushObservable.notify({ bufferBytesCount: batchBytesCount, bufferMessagesCount: eventNumber }) } + function spyOnContextManager(contextManager: ContextManager) { + spyOn(contextManager, 'getContext').and.callFake(() => fakeContext) + spyOn(contextManager, 'getBytesCount').and.callFake(() => fakeContextBytesCount) + } + + function spyOnFeatureFlagContexts(featureFlagContexts: FeatureFlagContexts) { + spyOn(featureFlagContexts, 'findFeatureFlagEvaluations').and.callFake(() => fakeContext) + spyOn(featureFlagContexts, 'getFeatureFlagBytesCount').and.callFake(() => fakeContextBytesCount) + } + beforeEach(() => { updateExperimentalFeatures(['customer_data_telemetry']) setupBuilder = setup() @@ -49,10 +65,10 @@ describe('customerDataTelemetry', () => { batchFlushObservable = new Observable() lifeCycle = new LifeCycle() fakeContextBytesCount = 1 - - spyOn(globalContextManager, 'getBytesCount').and.callFake(() => fakeContextBytesCount) - spyOn(userContextManager, 'getBytesCount').and.callFake(() => fakeContextBytesCount) - spyOn(featureFlagContexts, 'getFeatureFlagBytesCount').and.callFake(() => fakeContextBytesCount) + fakeContext = { foo: 'bar' } + spyOnContextManager(globalContextManager) + spyOnContextManager(userContextManager) + spyOnFeatureFlagContexts(featureFlagContexts) telemetryEvents = [] const telemetry = startTelemetry(TelemetryService.RUM, configuration) @@ -98,7 +114,8 @@ describe('customerDataTelemetry', () => { it('should not collect empty contexts telemetry', () => { const { clock } = setupBuilder.build() - generateBatch({ eventNumber: 1, contextBytesCount: 0, batchBytesCount: 1 }) + generateBatch({ eventNumber: 1, context: {} }) + clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents[0].telemetry.globalContextBytes).not.toBeDefined() @@ -130,7 +147,7 @@ describe('customerDataTelemetry', () => { .withConfiguration({ telemetrySampleRate: 100, customerDataTelemetrySampleRate: 0 }) .build() - generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) + generateBatch({ eventNumber: 1 }) clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents.length).toEqual(0) @@ -140,7 +157,7 @@ describe('customerDataTelemetry', () => { resetExperimentalFeatures() const { clock } = setupBuilder.build() - generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) + generateBatch({ eventNumber: 1 }) clock.tick(MEASURES_PERIOD_DURATION) expect(telemetryEvents.length).toEqual(0) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index 9bb6f95925..3dbfde32b3 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -1,5 +1,6 @@ import type { BatchFlushEvent, Context, ContextManager, Observable, Telemetry } from '@datadog/browser-core' import { + isEmptyObject, includes, isExperimentalFeatureEnabled, performDraw, @@ -59,10 +60,19 @@ export function startCustomerDataTelemetry( initCurrentBatchMeasures() lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event: RumEvent & Context) => { - updateMeasure(currentBatchMeasures.globalContextBytes, globalContextManager.getBytesCount()) - updateMeasure(currentBatchMeasures.userContextBytes, userContextManager.getBytesCount()) + if (!isEmptyObject(globalContextManager.getContext())) { + updateMeasure(currentBatchMeasures.globalContextBytes, globalContextManager.getBytesCount()) + } + if (!isEmptyObject(userContextManager.getContext())) { + updateMeasure(currentBatchMeasures.userContextBytes, userContextManager.getBytesCount()) + } - if (includes([RumEventType.VIEW, RumEventType.ERROR], event.type)) { + const featureFlagContext = featureFlagContexts.findFeatureFlagEvaluations() + if ( + includes([RumEventType.VIEW, RumEventType.ERROR], event.type) && + featureFlagContext && + !isEmptyObject(featureFlagContext) + ) { updateMeasure(currentBatchMeasures.featureFlagBytes, featureFlagContexts.getFeatureFlagBytesCount()) } }) From 4a1f5ab02de180f2314ae0b429c872236f88050b Mon Sep 17 00:00:00 2001 From: Aymeric Date: Wed, 18 Jan 2023 12:07:02 +0100 Subject: [PATCH 13/18] =?UTF-8?q?=F0=9F=91=8CSimplify=20common=20context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/test/specHelper.ts | 27 ++++++++---- .../rum-core/src/boot/rumPublicApi.spec.ts | 36 ++-------------- packages/rum-core/src/boot/rumPublicApi.ts | 16 ++------ packages/rum-core/src/boot/startRum.spec.ts | 1 - packages/rum-core/src/boot/startRum.ts | 6 +-- packages/rum-core/src/domain/assembly.spec.ts | 3 +- packages/rum-core/src/domain/assembly.ts | 2 +- .../src/domain/contexts/commonContext.spec.ts | 41 +++++++++++++++++++ .../src/domain/contexts/commonContext.ts | 25 +++++++++++ packages/rum-core/src/domain/lifeCycle.ts | 3 +- .../action/actionCollection.ts | 3 +- .../error/errorCollection.spec.ts | 4 +- .../error/errorCollection.ts | 3 +- packages/rum-core/src/index.ts | 9 +--- packages/rum-core/src/rawRumEvent.types.ts | 7 ---- packages/rum-core/test/specHelper.ts | 8 ++-- 16 files changed, 111 insertions(+), 83 deletions(-) create mode 100644 packages/rum-core/src/domain/contexts/commonContext.spec.ts create mode 100644 packages/rum-core/src/domain/contexts/commonContext.ts diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index b900250315..548f6091ad 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -1,4 +1,6 @@ +import type { Context } from '@datadog/browser-core' import type { EndpointBuilder } from '../src/domain/configuration' +import type { ContextManager } from '../src/tools/contextManager' import { instrumentMethod } from '../src/tools/instrumentMethod' import { resetNavigationStart } from '../src/tools/timeUtils' import { buildUrl } from '../src/tools/urlPolyfill' @@ -499,13 +501,24 @@ export function interceptRequests() { } } -// export function contextBytesCounterStub(): ContextBytesCounter { -// return { -// compute: () => 1, -// invalidate: jasmine.createSpy('invalidate'), -// } -// } - export function isAdoptedStyleSheetsSupported() { return Boolean((document as any).adoptedStyleSheets) } + +export function createContextManagerStub(initialContext: Context = {}): ContextManager { + let context = initialContext + return { + getBytesCount: () => 0, + get: () => context, + add: noop, + remove: noop, + set: noop, + getContext: () => context, + setContext: (newContext: Context) => { + context = newContext + }, + setContextProperty: noop, + removeContextProperty: noop, + clearContext: noop, + } +} diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index ef1cf35b03..c9bb14f96f 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -765,7 +765,7 @@ describe('rum public api', () => { rumPublicApi.init(MANUAL_CONFIGURATION) expect(startRumSpy).toHaveBeenCalled() - expect(startRumSpy.calls.argsFor(0)[6]).toEqual({ name: 'foo' }) + expect(startRumSpy.calls.argsFor(0)[5]).toEqual({ name: 'foo' }) expect(recorderApiOnRumStartSpy).toHaveBeenCalled() expect(startViewSpy).not.toHaveBeenCalled() }) @@ -777,7 +777,7 @@ describe('rum public api', () => { rumPublicApi.startView('foo') expect(startRumSpy).toHaveBeenCalled() - expect(startRumSpy.calls.argsFor(0)[6]).toEqual({ name: 'foo' }) + expect(startRumSpy.calls.argsFor(0)[5]).toEqual({ name: 'foo' }) expect(recorderApiOnRumStartSpy).toHaveBeenCalled() expect(startViewSpy).not.toHaveBeenCalled() }) @@ -788,7 +788,7 @@ describe('rum public api', () => { rumPublicApi.startView('bar') expect(startRumSpy).toHaveBeenCalled() - expect(startRumSpy.calls.argsFor(0)[6]).toEqual({ name: 'foo' }) + expect(startRumSpy.calls.argsFor(0)[5]).toEqual({ name: 'foo' }) expect(recorderApiOnRumStartSpy).toHaveBeenCalled() expect(startViewSpy).toHaveBeenCalled() expect(startViewSpy.calls.argsFor(0)[0]).toEqual({ name: 'bar' }) @@ -827,36 +827,6 @@ describe('rum public api', () => { }) }) - describe('common context', () => { - let isRecording: boolean - let rumPublicApi: RumPublicApi - let startRumSpy: jasmine.Spy - - function getCommonContext() { - return startRumSpy.calls.argsFor(0)[2]() - } - - beforeEach(() => { - isRecording = false - startRumSpy = jasmine.createSpy('startRum') - rumPublicApi = makeRumPublicApi(startRumSpy, { ...noopRecorderApi, isRecording: () => isRecording }) - }) - - describe('hasReplay', () => { - it('should be undefined if it is not recording', () => { - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - isRecording = false - expect(getCommonContext().hasReplay).toBeUndefined() - }) - - it('should be true if it is recording', () => { - rumPublicApi.init(DEFAULT_INIT_CONFIGURATION) - isRecording = true - expect(getCommonContext().hasReplay).toBeTrue() - }) - }) - }) - describe('recording', () => { let recorderApiOnRumStartSpy: jasmine.Spy let setupBuilder: TestSetupBuilder diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index 1348b26e85..c761be2b6f 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -27,6 +27,7 @@ import { ActionType } from '../rawRumEvent.types' import type { RumConfiguration, RumInitConfiguration } from '../domain/configuration' import { validateAndBuildRumConfiguration } from '../domain/configuration' import type { ViewOptions } from '../domain/rumEventsCollection/view/trackViews' +import { getCommonContext } from '../domain/contexts/commonContext' import type { startRum } from './startRum' export type RumPublicApi = ReturnType @@ -73,19 +74,13 @@ export function makeRumPublicApi( } let addActionStrategy: StartRumResult['addAction'] = ( action, - commonContext = { - context: globalContextManager.getContext(), - user: userContextManager.getContext(), - } + commonContext = getCommonContext(globalContextManager, userContextManager) ) => { bufferApiCalls.add(() => addActionStrategy(action, commonContext)) } let addErrorStrategy: StartRumResult['addError'] = ( providedError, - commonContext = { - context: globalContextManager.getContext(), - user: userContextManager.getContext(), - } + commonContext = getCommonContext(globalContextManager, userContextManager) ) => { bufferApiCalls.add(() => addErrorStrategy(providedError, commonContext)) } @@ -153,11 +148,6 @@ export function makeRumPublicApi( const startRumResults = startRumImpl( initConfiguration, configuration, - () => ({ - user: userContextManager.getContext(), - context: globalContextManager.getContext(), - hasReplay: recorderApi.isRecording() ? true : undefined, - }), recorderApi, globalContextManager, userContextManager, diff --git a/packages/rum-core/src/boot/startRum.spec.ts b/packages/rum-core/src/boot/startRum.spec.ts index 45885af6f2..54129141a6 100644 --- a/packages/rum-core/src/boot/startRum.spec.ts +++ b/packages/rum-core/src/boot/startRum.spec.ts @@ -302,7 +302,6 @@ describe('view events', () => { startRum( {} as RumInitConfiguration, configuration, - () => ({ context: {}, user: {} }), noopRecorderApi, createContextManager(), createContextManager() diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 242daacb1b..86834501de 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -23,7 +23,6 @@ import { startResourceCollection } from '../domain/rumEventsCollection/resource/ import { startViewCollection } from '../domain/rumEventsCollection/view/viewCollection' import type { RumSessionManager } from '../domain/rumSessionManager' import { startRumSessionManager, startRumSessionManagerStub } from '../domain/rumSessionManager' -import type { CommonContext } from '../rawRumEvent.types' import { startRumBatch } from '../transport/startRumBatch' import { startRumEventBridge } from '../transport/startRumEventBridge' import { startUrlContexts } from '../domain/contexts/urlContexts' @@ -35,12 +34,13 @@ import type { ViewOptions } from '../domain/rumEventsCollection/view/trackViews' import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' import { startCustomerDataTelemetry } from '../domain/startCustomerDataTelemetry' import { startPageStateHistory } from '../domain/contexts/pageStateHistory' +import type { CommonContext } from '../domain/contexts/commonContext' +import { getCommonContext } from '../domain/contexts/commonContext' import type { RecorderApi } from './rumPublicApi' export function startRum( initConfiguration: RumInitConfiguration, configuration: RumConfiguration, - getCommonContext: () => CommonContext, recorderApi: RecorderApi, globalContextManager: ContextManager, userContextManager: ContextManager, @@ -101,7 +101,7 @@ export function startRum( session, locationChangeObservable, domMutationObservable, - getCommonContext, + () => getCommonContext(globalContextManager, userContextManager, recorderApi), reportError ) diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index a6667c1c31..a09aaa652b 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -11,7 +11,7 @@ import { createRawRumEvent } from '../../test/fixtures' import type { TestSetupBuilder } from '../../test/specHelper' import { mockCiVisibilityWindowValues, cleanupCiVisibilityWindowValues, setup } from '../../test/specHelper' import type { RumEventDomainContext } from '../domainContext.types' -import type { CommonContext, RawRumActionEvent, RawRumErrorEvent, RawRumEvent } from '../rawRumEvent.types' +import type { RawRumActionEvent, RawRumErrorEvent, RawRumEvent } from '../rawRumEvent.types' import { RumEventType } from '../rawRumEvent.types' import type { RumActionEvent, RumErrorEvent, RumEvent } from '../rumEvent.types' import { initEventBridgeStub, deleteEventBridgeStub } from '../../../core/test/specHelper' @@ -22,6 +22,7 @@ import { LifeCycleEventType } from './lifeCycle' import { RumSessionPlan } from './rumSessionManager' import type { RumConfiguration } from './configuration' import type { ViewContext } from './contexts/viewContexts' +import type { CommonContext } from './contexts/commonContext' describe('rum assembly', () => { let setupBuilder: TestSetupBuilder diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index 913eb38314..ae98f6eae0 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -12,7 +12,6 @@ import { } from '@datadog/browser-core' import type { RumEventDomainContext } from '../domainContext.types' import type { - CommonContext, RawRumErrorEvent, RawRumEvent, RawRumLongTaskEvent, @@ -31,6 +30,7 @@ import type { UrlContexts } from './contexts/urlContexts' import type { RumConfiguration } from './configuration' import type { ActionContexts } from './rumEventsCollection/action/actionCollection' import { getDisplayContext } from './contexts/displayContext' +import type { CommonContext } from './contexts/commonContext' // replaced at build time declare const __BUILD_ENV__SDK_VERSION__: string diff --git a/packages/rum-core/src/domain/contexts/commonContext.spec.ts b/packages/rum-core/src/domain/contexts/commonContext.spec.ts new file mode 100644 index 0000000000..9e2f81ae40 --- /dev/null +++ b/packages/rum-core/src/domain/contexts/commonContext.spec.ts @@ -0,0 +1,41 @@ +import type { Context, ContextManager } from '@datadog/browser-core' +import { createContextManagerStub } from '../../../../core/test/specHelper' +import { noopRecorderApi } from '../../../test/specHelper' +import type { RecorderApi } from '../../boot/rumPublicApi' +import type { CommonContext } from './commonContext' +import { getCommonContext as getCommonContextImpl } from './commonContext' + +describe('commonContext', () => { + let isRecording: boolean + let fakeContext: Context + let getCommonContext: () => CommonContext + + beforeEach(() => { + isRecording = false + fakeContext = { foo: 'bar' } + const globalContextManager: ContextManager = createContextManagerStub(fakeContext) + const userContextManager: ContextManager = createContextManagerStub(fakeContext) + const recorderApi: RecorderApi = { ...noopRecorderApi, isRecording: () => isRecording } + getCommonContext = (): CommonContext => getCommonContextImpl(globalContextManager, userContextManager, recorderApi) + }) + + it('should return common context', () => { + expect(getCommonContext()).toEqual({ + context: fakeContext, + user: fakeContext, + hasReplay: undefined, + }) + }) + + describe('hasReplay', () => { + it('should be undefined if it is not recording', () => { + isRecording = false + expect(getCommonContext().hasReplay).toBeUndefined() + }) + + it('should be true if it is recording', () => { + isRecording = true + expect(getCommonContext().hasReplay).toBeTrue() + }) + }) +}) diff --git a/packages/rum-core/src/domain/contexts/commonContext.ts b/packages/rum-core/src/domain/contexts/commonContext.ts new file mode 100644 index 0000000000..bd0904b7e0 --- /dev/null +++ b/packages/rum-core/src/domain/contexts/commonContext.ts @@ -0,0 +1,25 @@ +import type { Context, ContextManager, User } from '@datadog/browser-core' +import type { RecorderApi } from '@datadog/browser-rum-core' + +export interface CommonContext { + user: User + context: Context + hasReplay?: true +} + +export function getCommonContext( + globalContextManager: ContextManager, + userContextManager: ContextManager, + recorderApi?: RecorderApi +): CommonContext { + const commonContext: CommonContext = { + context: globalContextManager.getContext(), + user: userContextManager.getContext(), + } + + if (recorderApi) { + commonContext.hasReplay = recorderApi.isRecording() ? true : undefined + } + + return commonContext +} diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index 7ff3e5fd96..f973cfe7ff 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -1,8 +1,9 @@ import type { Context, PageExitEvent, RawError, RelativeTime, Subscription } from '@datadog/browser-core' import type { RumPerformanceEntry } from '../browser/performanceCollection' import type { RumEventDomainContext } from '../domainContext.types' -import type { CommonContext, RawRumEvent } from '../rawRumEvent.types' +import type { RawRumEvent } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' +import type { CommonContext } from './contexts/commonContext' import type { RequestCompleteEvent, RequestStartEvent } from './requestCollection' import type { AutoAction } from './rumEventsCollection/action/actionCollection' import type { ViewEvent, ViewCreatedEvent, ViewEndedEvent } from './rumEventsCollection/view/trackViews' diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts index e0cb0c86e6..ac0208f383 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts @@ -1,12 +1,13 @@ import type { ClocksState, Context, Observable } from '@datadog/browser-core' import { noop, assign, combine, toServerDuration, generateUUID } from '@datadog/browser-core' -import type { CommonContext, RawRumActionEvent } from '../../../rawRumEvent.types' +import type { RawRumActionEvent } from '../../../rawRumEvent.types' import { ActionType, RumEventType } from '../../../rawRumEvent.types' import type { LifeCycle, RawRumEventCollectedData } from '../../lifeCycle' import { LifeCycleEventType } from '../../lifeCycle' import type { ForegroundContexts } from '../../contexts/foregroundContexts' import type { RumConfiguration } from '../../configuration' +import type { CommonContext } from '../../contexts/commonContext' import type { ActionContexts, ClickAction } from './trackClickActions' import { trackClickActions } from './trackClickActions' diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts index 46eb34449c..dfefc89332 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts @@ -118,7 +118,7 @@ describe('error collection', () => { }, { context: { foo: 'bar' }, user: {} } ) - expect(rawRumEvents[0].savedCommonContext!.context).toEqual({ + expect(rawRumEvents[0].savedCommonContext.context).toEqual({ foo: 'bar', }) }) @@ -133,7 +133,7 @@ describe('error collection', () => { }, { context: {}, user: { id: 'foo' } } ) - expect(rawRumEvents[0].savedCommonContext!.user).toEqual({ + expect(rawRumEvents[0].savedCommonContext.user).toEqual({ id: 'foo', }) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts index 60bb19e9c6..4196c9a012 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts @@ -10,12 +10,13 @@ import { Observable, trackRuntimeError, } from '@datadog/browser-core' -import type { CommonContext, RawRumErrorEvent } from '../../../rawRumEvent.types' +import type { RawRumErrorEvent } from '../../../rawRumEvent.types' import { RumEventType } from '../../../rawRumEvent.types' import type { LifeCycle, RawRumEventCollectedData } from '../../lifeCycle' import { LifeCycleEventType } from '../../lifeCycle' import type { ForegroundContexts } from '../../contexts/foregroundContexts' import type { FeatureFlagContexts } from '../../contexts/featureFlagContext' +import type { CommonContext } from '../../contexts/commonContext' import { trackConsoleError } from './trackConsoleError' import { trackReportError } from './trackReportError' diff --git a/packages/rum-core/src/index.ts b/packages/rum-core/src/index.ts index e796689c25..f6acdec597 100644 --- a/packages/rum-core/src/index.ts +++ b/packages/rum-core/src/index.ts @@ -18,14 +18,7 @@ export { RumViewEventDomainContext, RumEventDomainContext, } from './domainContext.types' -export { - CommonContext, - ReplayStats, - ActionType, - RumEventType, - FrustrationType, - RawRumActionEvent, -} from './rawRumEvent.types' +export { ReplayStats, ActionType, RumEventType, FrustrationType, RawRumActionEvent } from './rawRumEvent.types' export { startRum } from './boot/startRum' export { LifeCycle, LifeCycleEventType } from './domain/lifeCycle' export { ViewCreatedEvent } from './domain/rumEventsCollection/view/trackViews' diff --git a/packages/rum-core/src/rawRumEvent.types.ts b/packages/rum-core/src/rawRumEvent.types.ts index 4f46c123c3..289a6783da 100644 --- a/packages/rum-core/src/rawRumEvent.types.ts +++ b/packages/rum-core/src/rawRumEvent.types.ts @@ -7,7 +7,6 @@ import type { ServerDuration, TimeStamp, RawErrorCause, - User, } from '@datadog/browser-core' import type { PageStateEntry } from './domain/contexts/pageStateHistory' import type { RumSessionPlan } from './domain/rumSessionManager' @@ -248,9 +247,3 @@ export interface RumContext { browser_sdk_version?: string } } - -export interface CommonContext { - user: User - context: Context - hasReplay?: true -} diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index 0ea6140e54..d258cfc9c4 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -1,7 +1,7 @@ import type { Context, ContextManager, TimeStamp } from '@datadog/browser-core' -import { createContextManager, assign, combine, Observable, noop } from '@datadog/browser-core' +import { assign, combine, Observable, noop } from '@datadog/browser-core' import type { Clock } from '../../core/test/specHelper' -import { SPEC_ENDPOINTS, mockClock, buildLocation } from '../../core/test/specHelper' +import { createContextManagerStub, SPEC_ENDPOINTS, mockClock, buildLocation } from '../../core/test/specHelper' import type { RecorderApi } from '../src/boot/rumPublicApi' import type { ForegroundContexts } from '../src/domain/contexts/foregroundContexts' import type { RawRumEventCollectedData } from '../src/domain/lifeCycle' @@ -101,8 +101,8 @@ export function setup(): TestSetupBuilder { selectInForegroundPeriodsFor: () => undefined, stop: noop, } - const globalContextManager = createContextManager() - const userContextManager = createContextManager() + const globalContextManager = createContextManagerStub() + const userContextManager = createContextManagerStub() const pageStateHistory: PageStateHistory = { findAll: () => undefined, stop: noop, From d46b7d32ce9fa08abd60ad807048859bbe0343f1 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Thu, 19 Jan 2023 09:36:25 +0100 Subject: [PATCH 14/18] =?UTF-8?q?=F0=9F=91=8CUse=20get=20instead=20of=20ge?= =?UTF-8?q?tContext=20(avoid=20deepclone)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum-core/src/domain/contexts/commonContext.ts | 2 +- .../domain/rumEventsCollection/error/errorCollection.spec.ts | 4 ++-- .../rum-core/src/domain/startCustomerDataTelemetry.spec.ts | 2 +- packages/rum-core/src/domain/startCustomerDataTelemetry.ts | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/rum-core/src/domain/contexts/commonContext.ts b/packages/rum-core/src/domain/contexts/commonContext.ts index bd0904b7e0..2891a40e89 100644 --- a/packages/rum-core/src/domain/contexts/commonContext.ts +++ b/packages/rum-core/src/domain/contexts/commonContext.ts @@ -1,5 +1,5 @@ import type { Context, ContextManager, User } from '@datadog/browser-core' -import type { RecorderApi } from '@datadog/browser-rum-core' +import type { RecorderApi } from '../../boot/rumPublicApi' export interface CommonContext { user: User diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts index dfefc89332..46eb34449c 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts @@ -118,7 +118,7 @@ describe('error collection', () => { }, { context: { foo: 'bar' }, user: {} } ) - expect(rawRumEvents[0].savedCommonContext.context).toEqual({ + expect(rawRumEvents[0].savedCommonContext!.context).toEqual({ foo: 'bar', }) }) @@ -133,7 +133,7 @@ describe('error collection', () => { }, { context: {}, user: { id: 'foo' } } ) - expect(rawRumEvents[0].savedCommonContext.user).toEqual({ + expect(rawRumEvents[0].savedCommonContext!.user).toEqual({ id: 'foo', }) }) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index acbe0dd8f1..11f3a3b793 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -43,7 +43,7 @@ describe('customerDataTelemetry', () => { } function spyOnContextManager(contextManager: ContextManager) { - spyOn(contextManager, 'getContext').and.callFake(() => fakeContext) + spyOn(contextManager, 'get').and.callFake(() => fakeContext) spyOn(contextManager, 'getBytesCount').and.callFake(() => fakeContextBytesCount) } diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index 3dbfde32b3..2e8ce21e85 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -60,10 +60,10 @@ export function startCustomerDataTelemetry( initCurrentBatchMeasures() lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event: RumEvent & Context) => { - if (!isEmptyObject(globalContextManager.getContext())) { + if (!isEmptyObject(globalContextManager.get())) { updateMeasure(currentBatchMeasures.globalContextBytes, globalContextManager.getBytesCount()) } - if (!isEmptyObject(userContextManager.getContext())) { + if (!isEmptyObject(userContextManager.get())) { updateMeasure(currentBatchMeasures.userContextBytes, userContextManager.getBytesCount()) } From 81d2864f77d6b8c86af72a166587b85c3de2787b Mon Sep 17 00:00:00 2001 From: Aymeric Date: Thu, 19 Jan 2023 10:35:08 +0100 Subject: [PATCH 15/18] =?UTF-8?q?=F0=9F=91=8C=20Clarifying=20view=20update?= =?UTF-8?q?=20behavior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum-core/src/domain/startCustomerDataTelemetry.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index 2e8ce21e85..c02367b022 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -59,6 +59,8 @@ export function startCustomerDataTelemetry( initCurrentPeriodMeasures() initCurrentBatchMeasures() + // We measure the data of every view updates even if there could only be one per batch due to the upsert + // It means that contexts bytes count sums can be higher than it really is lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event: RumEvent & Context) => { if (!isEmptyObject(globalContextManager.get())) { updateMeasure(currentBatchMeasures.globalContextBytes, globalContextManager.getBytesCount()) From 6b81a45099af4bf53f6aa468f3cc82f1d7fafeba Mon Sep 17 00:00:00 2001 From: Aymeric Date: Thu, 19 Jan 2023 17:38:07 +0100 Subject: [PATCH 16/18] =?UTF-8?q?=F0=9F=91=8CTest=20that=20getBytesCount?= =?UTF-8?q?=20return=20the=20bytes=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/tools/contextManager.spec.ts | 7 ++++--- .../src/domain/contexts/featureFlagContext.spec.ts | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core/src/tools/contextManager.spec.ts b/packages/core/src/tools/contextManager.spec.ts index 5729dad0fa..08ec150525 100644 --- a/packages/core/src/tools/contextManager.spec.ts +++ b/packages/core/src/tools/contextManager.spec.ts @@ -74,10 +74,10 @@ describe('createContextManager', () => { expect(manager.getContext()).toEqual({}) }) - it('should compute the bytes count only if a the context has been updated', () => { + it('should compute the bytes count only if the context has been updated', () => { const computeBytesCountStub = jasmine.createSpy('computeBytesCountStub').and.returnValue(1) const manager = createContextManager(computeBytesCountStub) - manager.add('foo', 'bar') + manager.getBytesCount() manager.remove('foo') @@ -94,8 +94,9 @@ describe('createContextManager', () => { manager.clearContext() manager.getBytesCount() - manager.getBytesCount() + const bytesCount = manager.getBytesCount() + expect(bytesCount).toEqual(1) expect(computeBytesCountStub).toHaveBeenCalledTimes(6) }) }) diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts index 9628ba011d..e6f66c97d8 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts @@ -148,7 +148,7 @@ describe('featureFlagContexts', () => { }) describe('getFeatureFlagBytesCount', () => { - it('should compute the bytes count only if a the context has been updated', () => { + it('should compute the bytes count only if the context has been updated', () => { updateExperimentalFeatures(['feature_flags']) const { lifeCycle } = setupBuilder.withFakeClock().build() @@ -165,8 +165,9 @@ describe('featureFlagContexts', () => { startClocks: relativeToClocks(10 as RelativeTime), } as ViewCreatedEvent) featureFlagContexts.getFeatureFlagBytesCount() - featureFlagContexts.getFeatureFlagBytesCount() + const bytesCount = featureFlagContexts.getFeatureFlagBytesCount() + expect(bytesCount).toEqual(1) expect(computeBytesCountStub).toHaveBeenCalledTimes(3) }) }) From 242b255816e8931513998981d6f7b4e0e84ba628 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 20 Jan 2023 10:12:41 +0100 Subject: [PATCH 17/18] =?UTF-8?q?=F0=9F=91=8CFixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/test/specHelper.ts | 20 ----------- packages/logs/src/boot/logsPublicApi.spec.ts | 16 ++++----- packages/logs/src/boot/logsPublicApi.ts | 6 ++-- packages/logs/src/boot/startLogs.ts | 4 +-- packages/logs/src/domain/assembly.ts | 4 +-- .../rum-core/src/boot/rumPublicApi.spec.ts | 4 +-- packages/rum-core/src/boot/rumPublicApi.ts | 6 ++-- packages/rum-core/src/boot/startRum.spec.ts | 1 + packages/rum-core/src/boot/startRum.ts | 8 ++--- packages/rum-core/src/domain/assembly.spec.ts | 3 ++ packages/rum-core/src/domain/assembly.ts | 4 +-- .../src/domain/contexts/commonContext.spec.ts | 22 +++++++----- .../src/domain/contexts/commonContext.ts | 15 +++----- .../error/errorCollection.spec.ts | 4 +-- .../domain/startCustomerDataTelemetry.spec.ts | 34 ++++++++----------- .../src/domain/startCustomerDataTelemetry.ts | 8 +++-- packages/rum-core/test/specHelper.ts | 8 ++--- 17 files changed, 74 insertions(+), 93 deletions(-) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index 548f6091ad..42ea99e1cf 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -1,6 +1,4 @@ -import type { Context } from '@datadog/browser-core' import type { EndpointBuilder } from '../src/domain/configuration' -import type { ContextManager } from '../src/tools/contextManager' import { instrumentMethod } from '../src/tools/instrumentMethod' import { resetNavigationStart } from '../src/tools/timeUtils' import { buildUrl } from '../src/tools/urlPolyfill' @@ -504,21 +502,3 @@ export function interceptRequests() { export function isAdoptedStyleSheetsSupported() { return Boolean((document as any).adoptedStyleSheets) } - -export function createContextManagerStub(initialContext: Context = {}): ContextManager { - let context = initialContext - return { - getBytesCount: () => 0, - get: () => context, - add: noop, - remove: noop, - set: noop, - getContext: () => context, - setContext: (newContext: Context) => { - context = newContext - }, - setContextProperty: noop, - removeContextProperty: noop, - clearContext: noop, - } -} diff --git a/packages/logs/src/boot/logsPublicApi.spec.ts b/packages/logs/src/boot/logsPublicApi.spec.ts index 82b816bfc7..86b29ee22f 100644 --- a/packages/logs/src/boot/logsPublicApi.spec.ts +++ b/packages/logs/src/boot/logsPublicApi.spec.ts @@ -145,8 +145,8 @@ describe('logs entry', () => { it('should have the current date, view and global context', () => { LOGS.setGlobalContextProperty('foo', 'bar') - const getCommonContext = startLogs.calls.mostRecent().args[2] - expect(getCommonContext()).toEqual({ + const buildCommonContext = startLogs.calls.mostRecent().args[2] + expect(buildCommonContext()).toEqual({ view: { referrer: document.referrer, url: window.location.href, @@ -346,8 +346,8 @@ describe('logs entry', () => { const user = { id: 'foo', name: 'bar', email: 'qux', foo: { bar: 'qux' } } logsPublicApi.setUser(user) - const getCommonContext = startLogs.calls.mostRecent().args[2] - expect(getCommonContext().user).toEqual({ + const buildCommonContext = startLogs.calls.mostRecent().args[2] + expect(buildCommonContext().user).toEqual({ email: 'qux', foo: { bar: 'qux' }, id: 'foo', @@ -358,8 +358,8 @@ describe('logs entry', () => { it('should sanitize predefined properties', () => { const user = { id: null, name: 2, email: { bar: 'qux' } } logsPublicApi.setUser(user as any) - const getCommonContext = startLogs.calls.mostRecent().args[2] - expect(getCommonContext().user).toEqual({ + const buildCommonContext = startLogs.calls.mostRecent().args[2] + expect(buildCommonContext().user).toEqual({ email: '[object Object]', id: 'null', name: '2', @@ -371,8 +371,8 @@ describe('logs entry', () => { logsPublicApi.setUser(user) logsPublicApi.clearUser() - const getCommonContext = startLogs.calls.mostRecent().args[2] - expect(getCommonContext().user).toEqual({}) + const buildCommonContext = startLogs.calls.mostRecent().args[2] + expect(buildCommonContext().user).toEqual({}) }) it('should reject non object input', () => { diff --git a/packages/logs/src/boot/logsPublicApi.ts b/packages/logs/src/boot/logsPublicApi.ts index f4cd056891..1ff3c1c1af 100644 --- a/packages/logs/src/boot/logsPublicApi.ts +++ b/packages/logs/src/boot/logsPublicApi.ts @@ -45,7 +45,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { let handleLogStrategy: StartLogsResult['handleLog'] = ( logsMessage: LogsMessage, logger: Logger, - savedCommonContext = deepClone(getCommonContext()), + savedCommonContext = deepClone(buildCommonContext()), date = timeStampNow() ) => { beforeInitLoggerLog.add(() => handleLogStrategy(logsMessage, logger, savedCommonContext, date)) @@ -54,7 +54,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { let getInitConfigurationStrategy = (): InitConfiguration | undefined => undefined const mainLogger = new Logger((...params) => handleLogStrategy(...params)) - function getCommonContext(): CommonContext { + function buildCommonContext(): CommonContext { return { view: { referrer: document.referrer, @@ -88,7 +88,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) { ;({ handleLog: handleLogStrategy, getInternalContext: getInternalContextStrategy } = startLogsImpl( initConfiguration, configuration, - getCommonContext, + buildCommonContext, mainLogger )) diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index cd404f23a5..7406f35dbf 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -33,7 +33,7 @@ import { startInternalContext } from '../domain/internalContext' export function startLogs( initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration, - getCommonContext: () => CommonContext, + buildCommonContext: () => CommonContext, mainLogger: Logger ) { const lifeCycle = new LifeCycle() @@ -80,7 +80,7 @@ export function startLogs( ? startLogsSessionManager(configuration) : startLogsSessionManagerStub(configuration) - startLogsAssembly(session, configuration, lifeCycle, getCommonContext, mainLogger, reportError) + startLogsAssembly(session, configuration, lifeCycle, buildCommonContext, mainLogger, reportError) if (!canUseEventBridge()) { startLogsBatch(configuration, lifeCycle, reportError, pageExitObservable) diff --git a/packages/logs/src/domain/assembly.ts b/packages/logs/src/domain/assembly.ts index ba9467a136..16f7ff21a1 100644 --- a/packages/logs/src/domain/assembly.ts +++ b/packages/logs/src/domain/assembly.ts @@ -23,7 +23,7 @@ export function startLogsAssembly( sessionManager: LogsSessionManager, configuration: LogsConfiguration, lifeCycle: LifeCycle, - getCommonContext: () => CommonContext, + buildCommonContext: () => CommonContext, mainLogger: Logger, // Todo: [RUMF-1230] Remove this parameter in the next major release reportError: (error: RawError) => void ) { @@ -43,7 +43,7 @@ export function startLogsAssembly( return } - const commonContext = savedCommonContext || getCommonContext() + const commonContext = savedCommonContext || buildCommonContext() const log = combine( { service: configuration.service, diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index c9bb14f96f..9b6e17c3fd 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -247,7 +247,7 @@ describe('rum public api', () => { startClocks: jasmine.any(Object), type: ActionType.CUSTOM, }, - { context: {}, user: {} }, + { context: {}, user: {}, hasReplay: undefined }, ]) }) @@ -338,7 +338,7 @@ describe('rum public api', () => { handlingStack: jasmine.any(String), startClocks: jasmine.any(Object), }, - { context: {}, user: {} }, + { context: {}, user: {}, hasReplay: undefined }, ]) }) diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index c761be2b6f..075555b5a6 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -27,7 +27,7 @@ import { ActionType } from '../rawRumEvent.types' import type { RumConfiguration, RumInitConfiguration } from '../domain/configuration' import { validateAndBuildRumConfiguration } from '../domain/configuration' import type { ViewOptions } from '../domain/rumEventsCollection/view/trackViews' -import { getCommonContext } from '../domain/contexts/commonContext' +import { buildCommonContext } from '../domain/contexts/commonContext' import type { startRum } from './startRum' export type RumPublicApi = ReturnType @@ -74,13 +74,13 @@ export function makeRumPublicApi( } let addActionStrategy: StartRumResult['addAction'] = ( action, - commonContext = getCommonContext(globalContextManager, userContextManager) + commonContext = buildCommonContext(globalContextManager, userContextManager, recorderApi) ) => { bufferApiCalls.add(() => addActionStrategy(action, commonContext)) } let addErrorStrategy: StartRumResult['addError'] = ( providedError, - commonContext = getCommonContext(globalContextManager, userContextManager) + commonContext = buildCommonContext(globalContextManager, userContextManager, recorderApi) ) => { bufferApiCalls.add(() => addErrorStrategy(providedError, commonContext)) } diff --git a/packages/rum-core/src/boot/startRum.spec.ts b/packages/rum-core/src/boot/startRum.spec.ts index 54129141a6..8060ffe3ef 100644 --- a/packages/rum-core/src/boot/startRum.spec.ts +++ b/packages/rum-core/src/boot/startRum.spec.ts @@ -55,6 +55,7 @@ function startRumStub( () => ({ context: {}, user: {}, + hasReplay: undefined, }), reportError ) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 86834501de..ebec51e1b2 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -35,7 +35,7 @@ import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext' import { startCustomerDataTelemetry } from '../domain/startCustomerDataTelemetry' import { startPageStateHistory } from '../domain/contexts/pageStateHistory' import type { CommonContext } from '../domain/contexts/commonContext' -import { getCommonContext } from '../domain/contexts/commonContext' +import { buildCommonContext } from '../domain/contexts/commonContext' import type { RecorderApi } from './rumPublicApi' export function startRum( @@ -101,7 +101,7 @@ export function startRum( session, locationChangeObservable, domMutationObservable, - () => getCommonContext(globalContextManager, userContextManager, recorderApi), + () => buildCommonContext(globalContextManager, userContextManager, recorderApi), reportError ) @@ -163,7 +163,7 @@ export function startRumEventCollection( sessionManager: RumSessionManager, locationChangeObservable: Observable, domMutationObservable: Observable, - getCommonContext: () => CommonContext, + buildCommonContext: () => CommonContext, reportError: (error: RawError) => void ) { const viewContexts = startViewContexts(lifeCycle) @@ -184,7 +184,7 @@ export function startRumEventCollection( viewContexts, urlContexts, actionContexts, - getCommonContext, + buildCommonContext, reportError ) diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index a09aaa652b..2b90f38f92 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -41,6 +41,7 @@ describe('rum assembly', () => { commonContext = { context: {}, user: {}, + hasReplay: undefined, } setupBuilder = setup() .withViewContexts({ @@ -364,6 +365,7 @@ describe('rum assembly', () => { savedCommonContext: { context: { replacedContext: 'a' }, user: {}, + hasReplay: undefined, }, }) @@ -402,6 +404,7 @@ describe('rum assembly', () => { savedCommonContext: { context: {}, user: { replacedAttribute: 'a' }, + hasReplay: undefined, }, }) diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index ae98f6eae0..58b3204cc4 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -66,7 +66,7 @@ export function startRumAssembly( viewContexts: ViewContexts, urlContexts: UrlContexts, actionContexts: ActionContexts, - getCommonContext: () => CommonContext, + buildCommonContext: () => CommonContext, reportError: (error: RawError) => void ) { const eventRateLimiters = { @@ -95,7 +95,7 @@ export function startRumAssembly( // TODO: stop sending view updates when session is expired const session = sessionManager.findTrackedSession(rawRumEvent.type !== RumEventType.VIEW ? startTime : undefined) if (session && viewContext && urlContext) { - const commonContext = savedCommonContext || getCommonContext() + const commonContext = savedCommonContext || buildCommonContext() const actionId = actionContexts.findActionId(startTime) const rumContext: RumContext = { diff --git a/packages/rum-core/src/domain/contexts/commonContext.spec.ts b/packages/rum-core/src/domain/contexts/commonContext.spec.ts index 9e2f81ae40..4b3c68a82d 100644 --- a/packages/rum-core/src/domain/contexts/commonContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/commonContext.spec.ts @@ -1,26 +1,30 @@ import type { Context, ContextManager } from '@datadog/browser-core' -import { createContextManagerStub } from '../../../../core/test/specHelper' +import { createContextManager } from '@datadog/browser-core' import { noopRecorderApi } from '../../../test/specHelper' import type { RecorderApi } from '../../boot/rumPublicApi' import type { CommonContext } from './commonContext' -import { getCommonContext as getCommonContextImpl } from './commonContext' +import { buildCommonContext as buildCommonContextImpl } from './commonContext' describe('commonContext', () => { let isRecording: boolean let fakeContext: Context - let getCommonContext: () => CommonContext + let buildCommonContext: () => CommonContext beforeEach(() => { isRecording = false fakeContext = { foo: 'bar' } - const globalContextManager: ContextManager = createContextManagerStub(fakeContext) - const userContextManager: ContextManager = createContextManagerStub(fakeContext) + const globalContextManager: ContextManager = createContextManager() + const userContextManager: ContextManager = createContextManager() + spyOn(globalContextManager, 'getContext').and.callFake(() => fakeContext) + spyOn(userContextManager, 'getContext').and.callFake(() => fakeContext) + const recorderApi: RecorderApi = { ...noopRecorderApi, isRecording: () => isRecording } - getCommonContext = (): CommonContext => getCommonContextImpl(globalContextManager, userContextManager, recorderApi) + buildCommonContext = (): CommonContext => + buildCommonContextImpl(globalContextManager, userContextManager, recorderApi) }) it('should return common context', () => { - expect(getCommonContext()).toEqual({ + expect(buildCommonContext()).toEqual({ context: fakeContext, user: fakeContext, hasReplay: undefined, @@ -30,12 +34,12 @@ describe('commonContext', () => { describe('hasReplay', () => { it('should be undefined if it is not recording', () => { isRecording = false - expect(getCommonContext().hasReplay).toBeUndefined() + expect(buildCommonContext().hasReplay).toBeUndefined() }) it('should be true if it is recording', () => { isRecording = true - expect(getCommonContext().hasReplay).toBeTrue() + expect(buildCommonContext().hasReplay).toBeTrue() }) }) }) diff --git a/packages/rum-core/src/domain/contexts/commonContext.ts b/packages/rum-core/src/domain/contexts/commonContext.ts index 2891a40e89..76a268df42 100644 --- a/packages/rum-core/src/domain/contexts/commonContext.ts +++ b/packages/rum-core/src/domain/contexts/commonContext.ts @@ -4,22 +4,17 @@ import type { RecorderApi } from '../../boot/rumPublicApi' export interface CommonContext { user: User context: Context - hasReplay?: true + hasReplay: true | undefined } -export function getCommonContext( +export function buildCommonContext( globalContextManager: ContextManager, userContextManager: ContextManager, - recorderApi?: RecorderApi + recorderApi: RecorderApi ): CommonContext { - const commonContext: CommonContext = { + return { context: globalContextManager.getContext(), user: userContextManager.getContext(), + hasReplay: recorderApi.isRecording() ? true : undefined, } - - if (recorderApi) { - commonContext.hasReplay = recorderApi.isRecording() ? true : undefined - } - - return commonContext } diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts index 46eb34449c..4abac35c09 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.spec.ts @@ -116,7 +116,7 @@ describe('error collection', () => { handlingStack: 'Error: handling foo', startClocks: { relative: 1234 as RelativeTime, timeStamp: 123456789 as TimeStamp }, }, - { context: { foo: 'bar' }, user: {} } + { context: { foo: 'bar' }, user: {}, hasReplay: undefined } ) expect(rawRumEvents[0].savedCommonContext!.context).toEqual({ foo: 'bar', @@ -131,7 +131,7 @@ describe('error collection', () => { handlingStack: 'Error: handling foo', startClocks: { relative: 1234 as RelativeTime, timeStamp: 123456789 as TimeStamp }, }, - { context: {}, user: { id: 'foo' } } + { context: {}, user: { id: 'foo' }, hasReplay: undefined } ) expect(rawRumEvents[0].savedCommonContext!.user).toEqual({ id: 'foo', diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index 11f3a3b793..46f051fabf 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -21,6 +21,7 @@ describe('customerDataTelemetry', () => { let fakeContext: Context let fakeContextBytesCount: number let lifeCycle: LifeCycle + const viewEvent = { type: RumEventType.VIEW } as RumEvent & Context function generateBatch({ eventNumber, @@ -37,7 +38,7 @@ describe('customerDataTelemetry', () => { fakeContext = context for (let index = 0; index < eventNumber; index++) { - lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.VIEW } as RumEvent & Context) + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, viewEvent) } batchFlushObservable.notify({ bufferBytesCount: batchBytesCount, bufferMessagesCount: eventNumber }) } @@ -94,7 +95,7 @@ describe('customerDataTelemetry', () => { it('should collect customer data telemetry', () => { const { clock } = setupBuilder.build() - generateBatch({ eventNumber: 2, contextBytesCount: 2, batchBytesCount: 2 }) + generateBatch({ eventNumber: 10, contextBytesCount: 10, batchBytesCount: 10 }) generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) clock.tick(MEASURES_PERIOD_DURATION) @@ -103,11 +104,11 @@ describe('customerDataTelemetry', () => { status: 'debug', message: 'Customer data measures', batchCount: 2, - batchBytesCount: { min: 1, max: 2, sum: 3 }, - batchMessagesCount: { min: 1, max: 2, sum: 3 }, - globalContextBytes: { min: 1, max: 2, sum: 5 }, - userContextBytes: { min: 1, max: 2, sum: 5 }, - featureFlagBytes: { min: 1, max: 2, sum: 5 }, + batchBytesCount: { min: 1, max: 10, sum: 11 }, + batchMessagesCount: { min: 1, max: 10, sum: 11 }, + globalContextBytes: { min: 1, max: 10, sum: 101 }, + userContextBytes: { min: 1, max: 10, sum: 101 }, + featureFlagBytes: { min: 1, max: 10, sum: 101 }, }) }) @@ -126,20 +127,15 @@ describe('customerDataTelemetry', () => { it('should not collect contexts telemetry of a unfinished batches', () => { const { clock } = setupBuilder.build() - generateBatch({ eventNumber: 1, contextBytesCount: 1, batchBytesCount: 1 }) - lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.VIEW } as RumEvent & Context) + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, viewEvent) + batchFlushObservable.notify({ bufferBytesCount: 1, bufferMessagesCount: 1 }) + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, viewEvent) clock.tick(MEASURES_PERIOD_DURATION) - expect(telemetryEvents[0].telemetry).toEqual( - jasmine.objectContaining({ - batchCount: 1, - batchBytesCount: { min: 1, max: 1, sum: 1 }, - batchMessagesCount: { min: 1, max: 1, sum: 1 }, - globalContextBytes: { min: 1, max: 1, sum: 1 }, - userContextBytes: { min: 1, max: 1, sum: 1 }, - featureFlagBytes: { min: 1, max: 1, sum: 1 }, - }) - ) + expect(telemetryEvents[0].telemetry.batchMessagesCount).toEqual(jasmine.objectContaining({ sum: 1 })) + expect(telemetryEvents[0].telemetry.globalContextBytes).toEqual(jasmine.objectContaining({ sum: 1 })) + expect(telemetryEvents[0].telemetry.userContextBytes).toEqual(jasmine.objectContaining({ sum: 1 })) + expect(telemetryEvents[0].telemetry.featureFlagBytes).toEqual(jasmine.objectContaining({ sum: 1 })) }) it('should not collect customer data telemetry when telemetry disabled', () => { diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index c02367b022..3ab1cdacb6 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -50,9 +50,11 @@ export function startCustomerDataTelemetry( featureFlagContexts: FeatureFlagContexts, batchFlushObservable: Observable ) { - const customerDataTelemetrySampleRate = - telemetry.enabled && performDraw(configuration.customerDataTelemetrySampleRate) - if (!customerDataTelemetrySampleRate || !isExperimentalFeatureEnabled('customer_data_telemetry')) { + const customerDataTelemetryEnabled = + telemetry.enabled && + isExperimentalFeatureEnabled('customer_data_telemetry') && + performDraw(configuration.customerDataTelemetrySampleRate) + if (!customerDataTelemetryEnabled) { return } diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index d258cfc9c4..0ea6140e54 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -1,7 +1,7 @@ import type { Context, ContextManager, TimeStamp } from '@datadog/browser-core' -import { assign, combine, Observable, noop } from '@datadog/browser-core' +import { createContextManager, assign, combine, Observable, noop } from '@datadog/browser-core' import type { Clock } from '../../core/test/specHelper' -import { createContextManagerStub, SPEC_ENDPOINTS, mockClock, buildLocation } from '../../core/test/specHelper' +import { SPEC_ENDPOINTS, mockClock, buildLocation } from '../../core/test/specHelper' import type { RecorderApi } from '../src/boot/rumPublicApi' import type { ForegroundContexts } from '../src/domain/contexts/foregroundContexts' import type { RawRumEventCollectedData } from '../src/domain/lifeCycle' @@ -101,8 +101,8 @@ export function setup(): TestSetupBuilder { selectInForegroundPeriodsFor: () => undefined, stop: noop, } - const globalContextManager = createContextManagerStub() - const userContextManager = createContextManagerStub() + const globalContextManager = createContextManager() + const userContextManager = createContextManager() const pageStateHistory: PageStateHistory = { findAll: () => undefined, stop: noop, From 8bed3390d5f95f319fdd6f8821ca638534f9616c Mon Sep 17 00:00:00 2001 From: Aymeric Date: Fri, 20 Jan 2023 16:53:35 +0100 Subject: [PATCH 18/18] =?UTF-8?q?=F0=9F=91=8CMeasure=20empty=20contexts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/startCustomerDataTelemetry.spec.ts | 23 ++++++++-- .../src/domain/startCustomerDataTelemetry.ts | 44 ++++++++++--------- 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts index 46f051fabf..3e9862ca5f 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.spec.ts @@ -30,6 +30,7 @@ describe('customerDataTelemetry', () => { context = fakeContext, }: { eventNumber: number + eventType?: RumEventType | 'Telemetry' batchBytesCount?: number contextBytesCount?: number context?: Context @@ -112,16 +113,30 @@ describe('customerDataTelemetry', () => { }) }) - it('should not collect empty contexts telemetry', () => { + it('should collect empty contexts telemetry', () => { const { clock } = setupBuilder.build() generateBatch({ eventNumber: 1, context: {} }) clock.tick(MEASURES_PERIOD_DURATION) - expect(telemetryEvents[0].telemetry.globalContextBytes).not.toBeDefined() - expect(telemetryEvents[0].telemetry.userContextBytes).not.toBeDefined() - expect(telemetryEvents[0].telemetry.featureFlagBytes).not.toBeDefined() + expect(telemetryEvents[0].telemetry).toEqual( + jasmine.objectContaining({ + globalContextBytes: { min: 0, max: 0, sum: 0 }, + userContextBytes: { min: 0, max: 0, sum: 0 }, + featureFlagBytes: { min: 0, max: 0, sum: 0 }, + }) + ) + }) + + it('should collect customer data only if batches contains rum events, no just telemetry', () => { + const { clock } = setupBuilder.build() + + batchFlushObservable.notify({ bufferBytesCount: 1, bufferMessagesCount: 1 }) + + clock.tick(MEASURES_PERIOD_DURATION) + + expect(telemetryEvents.length).toEqual(0) }) it('should not collect contexts telemetry of a unfinished batches', () => { diff --git a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts index 3ab1cdacb6..add3e31b25 100644 --- a/packages/rum-core/src/domain/startCustomerDataTelemetry.ts +++ b/packages/rum-core/src/domain/startCustomerDataTelemetry.ts @@ -40,6 +40,7 @@ type CurrentBatchMeasures = { let currentPeriodMeasures: CurrentPeriodMeasures let currentBatchMeasures: CurrentBatchMeasures +let batchHasRumEvent: boolean export function startCustomerDataTelemetry( configuration: RumConfiguration, @@ -64,24 +65,34 @@ export function startCustomerDataTelemetry( // We measure the data of every view updates even if there could only be one per batch due to the upsert // It means that contexts bytes count sums can be higher than it really is lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event: RumEvent & Context) => { - if (!isEmptyObject(globalContextManager.get())) { - updateMeasure(currentBatchMeasures.globalContextBytes, globalContextManager.getBytesCount()) - } - if (!isEmptyObject(userContextManager.get())) { - updateMeasure(currentBatchMeasures.userContextBytes, userContextManager.getBytesCount()) - } + batchHasRumEvent = true + updateMeasure( + currentBatchMeasures.globalContextBytes, + !isEmptyObject(globalContextManager.get()) ? globalContextManager.getBytesCount() : 0 + ) + + updateMeasure( + currentBatchMeasures.userContextBytes, + !isEmptyObject(userContextManager.get()) ? userContextManager.getBytesCount() : 0 + ) const featureFlagContext = featureFlagContexts.findFeatureFlagEvaluations() - if ( + const hasFeatureFlagContext = includes([RumEventType.VIEW, RumEventType.ERROR], event.type) && featureFlagContext && !isEmptyObject(featureFlagContext) - ) { - updateMeasure(currentBatchMeasures.featureFlagBytes, featureFlagContexts.getFeatureFlagBytesCount()) - } + updateMeasure( + currentBatchMeasures.featureFlagBytes, + hasFeatureFlagContext ? featureFlagContexts.getFeatureFlagBytesCount() : 0 + ) }) batchFlushObservable.subscribe(({ bufferBytesCount, bufferMessagesCount }) => { + // Don't measure batch that only contains telemetry events to avoid batch sending loop + // It could happen because after each batch we are adding a customer data measures telemetry event to the next one + if (!batchHasRumEvent) { + return + } currentPeriodMeasures.batchCount += 1 updateMeasure(currentPeriodMeasures.batchBytesCount, bufferBytesCount) updateMeasure(currentPeriodMeasures.batchMessagesCount, bufferMessagesCount) @@ -98,16 +109,8 @@ function sendCurrentPeriodMeasures() { if (currentPeriodMeasures.batchCount === 0) { return } - const { batchCount, batchBytesCount, batchMessagesCount, globalContextBytes, userContextBytes, featureFlagBytes } = - currentPeriodMeasures - addTelemetryDebug('Customer data measures', { - batchCount, - batchBytesCount, - batchMessagesCount, - globalContextBytes: globalContextBytes.sum ? globalContextBytes : undefined, - userContextBytes: userContextBytes.sum ? userContextBytes : undefined, - featureFlagBytes: featureFlagBytes.sum ? featureFlagBytes : undefined, - }) + + addTelemetryDebug('Customer data measures', currentPeriodMeasures) initCurrentPeriodMeasures() } @@ -139,6 +142,7 @@ function initCurrentPeriodMeasures() { } function initCurrentBatchMeasures() { + batchHasRumEvent = false currentBatchMeasures = { globalContextBytes: createMeasure(), userContextBytes: createMeasure(),