From e13a603749f6fefa3e137af88ac83c2ccd0f6b96 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 22 Jul 2024 15:51:25 +0200 Subject: [PATCH 01/12] Discard TraceIdentifier class in favour of function --- .../src/domain/requestCollection.spec.ts | 10 ++--- .../resource/resourceCollection.spec.ts | 26 +++++------ .../src/domain/resource/resourceCollection.ts | 4 +- .../src/domain/tracing/tracer.spec.ts | 26 +++++------ .../rum-core/src/domain/tracing/tracer.ts | 44 ++++++++++--------- 5 files changed, 56 insertions(+), 54 deletions(-) diff --git a/packages/rum-core/src/domain/requestCollection.spec.ts b/packages/rum-core/src/domain/requestCollection.spec.ts index 8ff58b3e94..578cb27d06 100644 --- a/packages/rum-core/src/domain/requestCollection.spec.ts +++ b/packages/rum-core/src/domain/requestCollection.spec.ts @@ -8,7 +8,7 @@ import { LifeCycle, LifeCycleEventType } from './lifeCycle' import type { RequestCompleteEvent, RequestStartEvent } from './requestCollection' import { trackFetch, trackXhr } from './requestCollection' import type { Tracer } from './tracing/tracer' -import { clearTracingIfNeeded, TraceIdentifier } from './tracing/tracer' +import { clearTracingIfNeeded, traceIdentifier } from './tracing/tracer' const DEFAULT_PAYLOAD = {} as Payload @@ -40,8 +40,8 @@ describe('collect fetch', () => { const tracerStub: Partial = { clearTracingIfNeeded, traceFetch: (context) => { - context.traceId = new TraceIdentifier() - context.spanId = new TraceIdentifier() + context.traceId = traceIdentifier() + context.spanId = traceIdentifier() }, } ;({ stop: stopFetchTracking } = trackFetch(lifeCycle, configuration, tracerStub as Tracer)) @@ -214,8 +214,8 @@ describe('collect xhr', () => { const tracerStub: Partial = { clearTracingIfNeeded, traceXhr: (context) => { - context.traceId = new TraceIdentifier() - context.spanId = new TraceIdentifier() + context.traceId = traceIdentifier() + context.spanId = traceIdentifier() }, } ;({ stop: stopXhrTracking } = trackXhr(lifeCycle, configuration, tracerStub as Tracer)) diff --git a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts index ea445c5c31..5a90c2555a 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts @@ -7,7 +7,7 @@ import type { RawRumResourceEvent } from '../../rawRumEvent.types' import { RumEventType } from '../../rawRumEvent.types' import { LifeCycleEventType } from '../lifeCycle' import type { RequestCompleteEvent } from '../requestCollection' -import { TraceIdentifier } from '../tracing/tracer' +import { traceIdentifier } from '../tracing/tracer' import { validateAndBuildRumConfiguration } from '../configuration' import type { RumPerformanceEntry } from '../../browser/performanceObservable' import { RumPerformanceEntryType } from '../../browser/performanceObservable' @@ -163,8 +163,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ type: RequestType.XHR, - traceId: new TraceIdentifier(), - spanId: new TraceIdentifier(), + traceId: traceIdentifier(), + spanId: traceIdentifier(), traceSampled: true, }) ) @@ -292,8 +292,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -307,8 +307,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: false, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -334,8 +334,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -359,8 +359,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -385,8 +385,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd diff --git a/packages/rum-core/src/domain/resource/resourceCollection.ts b/packages/rum-core/src/domain/resource/resourceCollection.ts index 1a11fbfc73..19d38e150c 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.ts @@ -23,7 +23,7 @@ import type { RawRumEventCollectedData, LifeCycle } from '../lifeCycle' import type { RequestCompleteEvent } from '../requestCollection' import type { PageStateHistory } from '../contexts/pageStateHistory' import { PageState } from '../contexts/pageStateHistory' -import { TraceIdentifier } from '../tracing/tracer' +import { traceIdentifier } from '../tracing/tracer' import { matchRequestTiming } from './matchRequestTiming' import { computePerformanceResourceDetails, @@ -205,7 +205,7 @@ function computeEntryTracingInfo(entry: RumPerformanceResourceTiming, configurat return { _dd: { trace_id: entry.traceId, - span_id: new TraceIdentifier().toDecimalString(), + span_id: traceIdentifier().toDecimalString(), rule_psr: getRulePsr(configuration), }, } diff --git a/packages/rum-core/src/domain/tracing/tracer.spec.ts b/packages/rum-core/src/domain/tracing/tracer.spec.ts index d8375633b2..c8a7036a80 100644 --- a/packages/rum-core/src/domain/tracing/tracer.spec.ts +++ b/packages/rum-core/src/domain/tracing/tracer.spec.ts @@ -4,7 +4,7 @@ import { createRumSessionManagerMock } from '../../../test' import type { RumFetchResolveContext, RumFetchStartContext, RumXhrStartContext } from '../requestCollection' import type { RumConfiguration, RumInitConfiguration } from '../configuration' import { validateAndBuildRumConfiguration } from '../configuration' -import { startTracer, TraceIdentifier } from './tracer' +import { startTracer, traceIdentifier, type TraceIdentifier } from './tracer' describe('tracer', () => { let configuration: RumConfiguration @@ -622,8 +622,8 @@ describe('tracer', () => { const context: RumFetchResolveContext = { status: 0, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), } as any tracer.clearTracingIfNeeded(context) @@ -636,8 +636,8 @@ describe('tracer', () => { const context: RumFetchResolveContext = { status: 200, - spanId: new TraceIdentifier(), - traceId: new TraceIdentifier(), + spanId: traceIdentifier(), + traceId: traceIdentifier(), } as any tracer.clearTracingIfNeeded(context) @@ -649,18 +649,18 @@ describe('tracer', () => { describe('TraceIdentifier', () => { it('should generate id', () => { - const traceIdentifier = new TraceIdentifier() + const identifier = traceIdentifier() - expect(traceIdentifier.toDecimalString()).toMatch(/^\d+$/) + expect(identifier.toDecimalString()).toMatch(/^\d+$/) }) - it('should pad the string to 16 characters', () => { - const traceIdentifier = new TraceIdentifier() - // Forcing as any to access private member: buffer - ;(traceIdentifier as any).buffer = new Uint8Array([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]) + // it('should pad the string to 16 characters', () => { + // const identifier = traceIdentifier() + // // Forcing as any to access private member: buffer + // ;(identifier as any).buffer = new Uint8Array([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]) - expect(traceIdentifier.toPaddedHexadecimalString()).toEqual('0001020304050607') - }) + // expect(identifier.toPaddedHexadecimalString()).toEqual('0001020304050607') + // }) }) function toPlainObject(headers: Headers) { diff --git a/packages/rum-core/src/domain/tracing/tracer.ts b/packages/rum-core/src/domain/tracing/tracer.ts index 239d7a0627..47cd730f35 100644 --- a/packages/rum-core/src/domain/tracing/tracer.ts +++ b/packages/rum-core/src/domain/tracing/tracer.ts @@ -124,8 +124,8 @@ function injectHeadersIfTracingAllowed( return } - context.traceId = new TraceIdentifier() - context.spanId = new TraceIdentifier() + context.traceId = traceIdentifier() + context.spanId = traceIdentifier() inject(makeTracingHeaders(context.traceId, context.spanId, context.traceSampled, tracingOption.propagatorTypes)) } @@ -193,17 +193,23 @@ function makeTracingHeaders( } /* eslint-disable no-bitwise */ -export class TraceIdentifier { - private buffer: Uint8Array = new Uint8Array(8) +export interface TraceIdentifier { + toDecimalString: () => string + toPaddedHexadecimalString: () => string +} + +export function traceIdentifier(): TraceIdentifier { + const buffer: Uint8Array = new Uint8Array(8) + getCrypto().getRandomValues(buffer) + buffer[0] = buffer[0] & 0x7f // force 63-bit - constructor() { - getCrypto().getRandomValues(this.buffer) - this.buffer[0] = this.buffer[0] & 0x7f // force 63-bit + function readInt32(offset: number) { + return buffer[offset] * 16777216 + (buffer[offset + 1] << 16) + (buffer[offset + 2] << 8) + buffer[offset + 3] } - toString(radix: number) { - let high = this.readInt32(0) - let low = this.readInt32(4) + function toString(radix: number) { + let high = readInt32(0) + let low = readInt32(4) let str = '' do { @@ -219,25 +225,21 @@ export class TraceIdentifier { /** * Format used everywhere except the trace intake */ - toDecimalString() { - return this.toString(10) + function toDecimalString() { + return toString(10) } /** * Format used by OTel headers */ - toPaddedHexadecimalString() { - const traceId = this.toString(16) + function toPaddedHexadecimalString() { + const traceId = toString(16) return Array(17 - traceId.length).join('0') + traceId } - private readInt32(offset: number) { - return ( - this.buffer[offset] * 16777216 + - (this.buffer[offset + 1] << 16) + - (this.buffer[offset + 2] << 8) + - this.buffer[offset + 3] - ) + return { + toDecimalString, + toPaddedHexadecimalString, } } /* eslint-enable no-bitwise */ From 92c7f7ebbab8f3060268d077064f26b651396c84 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 22 Jul 2024 15:52:51 +0200 Subject: [PATCH 02/12] Discard BoundedBuffer class in favour of function --- .../core/src/domain/telemetry/telemetry.ts | 6 ++-- packages/core/src/index.ts | 2 +- packages/core/src/tools/boundedBuffer.spec.ts | 8 ++--- packages/core/src/tools/boundedBuffer.ts | 32 +++++++++++++------ packages/logs/src/boot/preStartLogs.ts | 4 +-- packages/rum-core/src/boot/preStartRum.ts | 4 +-- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index 1205a18437..a98b008b4f 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -17,7 +17,7 @@ import { NonErrorPrefix } from '../error/error.types' import type { StackTrace } from '../../tools/stackTrace/computeStackTrace' import { computeStackTrace } from '../../tools/stackTrace/computeStackTrace' import { getConnectivity } from '../connectivity' -import { BoundedBuffer } from '../../tools/boundedBuffer' +import { boundedBuffer } from '../../tools/boundedBuffer' import type { TelemetryEvent } from './telemetryEvent.types' import type { RawTelemetryConfiguration, @@ -53,7 +53,7 @@ export interface Telemetry { const TELEMETRY_EXCLUDED_SITES: string[] = [INTAKE_SITE_US1_FED] // eslint-disable-next-line local-rules/disallow-side-effects -let preStartTelemetryBuffer = new BoundedBuffer() +let preStartTelemetryBuffer = boundedBuffer() let onRawTelemetryEventCollected = (event: RawTelemetryEvent) => { preStartTelemetryBuffer.add(() => onRawTelemetryEventCollected(event)) } @@ -144,7 +144,7 @@ export function drainPreStartTelemetry() { } export function resetTelemetry() { - preStartTelemetryBuffer = new BoundedBuffer() + preStartTelemetryBuffer = boundedBuffer() onRawTelemetryEventCollected = (event: RawTelemetryEvent) => { preStartTelemetryBuffer.add(() => onRawTelemetryEventCollected(event)) } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 61ede06dcf..ad54f58d2e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -106,7 +106,7 @@ export { createPageExitObservable, PageExitEvent, PageExitReason, isPageExitReas export * from './browser/addEventListener' export * from './tools/timer' export { initConsoleObservable, resetConsoleObservable, ConsoleLog } from './domain/console/consoleObservable' -export { BoundedBuffer } from './tools/boundedBuffer' +export { boundedBuffer, BoundedBuffer } from './tools/boundedBuffer' export { catchUserErrors } from './tools/catchUserErrors' export { createContextManager, ContextManager } from './domain/context/contextManager' export { storeContextManager, removeStorageListeners } from './domain/context/storeContextManager' diff --git a/packages/core/src/tools/boundedBuffer.spec.ts b/packages/core/src/tools/boundedBuffer.spec.ts index efd5046ca7..ece0cc7653 100644 --- a/packages/core/src/tools/boundedBuffer.spec.ts +++ b/packages/core/src/tools/boundedBuffer.spec.ts @@ -1,9 +1,9 @@ -import { BoundedBuffer } from './boundedBuffer' +import { boundedBuffer } from './boundedBuffer' describe('BoundedBuffer', () => { it('collect and drain the callbacks', () => { const spy = jasmine.createSpy<() => void>() - const buffered = new BoundedBuffer() + const buffered = boundedBuffer() buffered.add(spy) expect(spy.calls.count()).toBe(0) @@ -17,7 +17,7 @@ describe('BoundedBuffer', () => { it('store at most 500 callbacks', () => { const spy = jasmine.createSpy<() => void>() - const buffered = new BoundedBuffer() + const buffered = boundedBuffer() const limit = 500 for (let i = 0; i < limit + 1; i += 1) { @@ -30,7 +30,7 @@ describe('BoundedBuffer', () => { it('removes a callback', () => { const spy = jasmine.createSpy<() => void>() - const buffered = new BoundedBuffer() + const buffered = boundedBuffer() buffered.add(spy) buffered.remove(spy) diff --git a/packages/core/src/tools/boundedBuffer.ts b/packages/core/src/tools/boundedBuffer.ts index 2b1d3b7c7f..7074adb180 100644 --- a/packages/core/src/tools/boundedBuffer.ts +++ b/packages/core/src/tools/boundedBuffer.ts @@ -2,22 +2,34 @@ import { removeItem } from './utils/arrayUtils' const BUFFER_LIMIT = 500 -export class BoundedBuffer { - private buffer: Array<(arg: T) => void> = [] +export interface BoundedBuffer { + add: (callback: (arg: T) => void) => void + remove: (callback: (arg: T) => void) => void + drain: (arg: T) => void +} + +export function boundedBuffer(): BoundedBuffer { + const buffer: Array<(arg: T) => void> = [] - add(callback: (arg: T) => void) { - const length = this.buffer.push(callback) + function add(callback: (arg: T) => void) { + const length = buffer.push(callback) if (length > BUFFER_LIMIT) { - this.buffer.splice(0, 1) + buffer.splice(0, 1) } } - remove(callback: (arg: T) => void) { - removeItem(this.buffer, callback) + function remove(callback: (arg: T) => void) { + removeItem(buffer, callback) + } + + function drain(arg: T) { + buffer.forEach((callback) => callback(arg)) + buffer.length = 0 } - drain(arg: T) { - this.buffer.forEach((callback) => callback(arg)) - this.buffer.length = 0 + return { + add, + remove, + drain, } } diff --git a/packages/logs/src/boot/preStartLogs.ts b/packages/logs/src/boot/preStartLogs.ts index a9957243b5..073ad469cf 100644 --- a/packages/logs/src/boot/preStartLogs.ts +++ b/packages/logs/src/boot/preStartLogs.ts @@ -1,6 +1,6 @@ import type { TrackingConsentState } from '@datadog/browser-core' import { - BoundedBuffer, + boundedBuffer, assign, canUseEventBridge, display, @@ -24,7 +24,7 @@ export function createPreStartStrategy( trackingConsentState: TrackingConsentState, doStartLogs: (initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration) => StartLogsResult ): Strategy { - const bufferApiCalls = new BoundedBuffer() + const bufferApiCalls = boundedBuffer() let cachedInitConfiguration: LogsInitConfiguration | undefined let cachedConfiguration: LogsConfiguration | undefined const trackingConsentStateSubscription = trackingConsentState.observable.subscribe(tryStartLogs) diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts index fbf36b1f4e..a7878220f9 100644 --- a/packages/rum-core/src/boot/preStartRum.ts +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -1,5 +1,5 @@ import { - BoundedBuffer, + boundedBuffer, display, canUseEventBridge, displayAlreadyInitializedError, @@ -40,7 +40,7 @@ export function createPreStartStrategy( initialViewOptions?: ViewOptions ) => StartRumResult ): Strategy { - const bufferApiCalls = new BoundedBuffer() + const bufferApiCalls = boundedBuffer() let firstStartViewCall: | { options: ViewOptions | undefined; callback: (startRumResult: StartRumResult) => void } | undefined From 74a7d197e2a15e6f2d9174c0317cab5207d68d41 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Fri, 19 Jul 2024 16:04:29 +0200 Subject: [PATCH 03/12] Discard ProxyStats class in favour of function --- performances/src/proxy.ts | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/performances/src/proxy.ts b/performances/src/proxy.ts index 3a6aa2b8ac..d753a3cc3b 100644 --- a/performances/src/proxy.ts +++ b/performances/src/proxy.ts @@ -12,40 +12,48 @@ export interface Proxy { stats: ProxyStats } -class ProxyStats { - constructor(private statsByHost = new Map()) {} +interface ProxyStats { + addRequest: (request: IncomingMessage, size: number) => void + getStatsByHost: () => RequestStatsForHost[] + reset: () => void +} + +function proxyStatsFactory(): ProxyStats { + const statsByHost = new Map() - addRequest(request: IncomingMessage, size: number) { + function addRequest(request: IncomingMessage, size: number) { const url = new URL(request.url!, 'http://foo') const intakeUrl = new URL(url.searchParams.get('ddforward')!) - let hostStats = this.statsByHost.get(intakeUrl.hostname) + let hostStats = statsByHost.get(intakeUrl.hostname) if (!hostStats) { hostStats = { requestsSize: 0, requestsCount: 0 } - this.statsByHost.set(intakeUrl.hostname, hostStats) + statsByHost.set(intakeUrl.hostname, hostStats) } hostStats.requestsCount += 1 hostStats.requestsSize += size } - getStatsByHost(): RequestStatsForHost[] { - return Array.from(this.statsByHost, ([host, { requestsSize, requestsCount }]) => ({ + function getStatsByHost(): RequestStatsForHost[] { + return Array.from(statsByHost, ([host, { requestsSize, requestsCount }]) => ({ host, requestsSize, requestsCount, })) } - reset() { - this.statsByHost.clear() + function reset() { + statsByHost.clear() } + + return { addRequest, getStatsByHost, reset } } export function startProxy() { return new Promise((resolve, reject) => { const { key, cert, spkiFingerprint } = createSelfSignedCertificate() - const stats = new ProxyStats() + const stats = proxyStatsFactory() const server = createServer({ key, cert }) server.on('error', reject) server.on('request', (req: IncomingMessage, res: ServerResponse) => { From 6c2ed51b10c2500ae95d1f187866e8335899c758 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 22 Jul 2024 16:33:01 +0200 Subject: [PATCH 04/12] Discard Segment class in favour of function --- .../domain/segmentCollection/segment.spec.ts | 6 +- .../src/domain/segmentCollection/segment.ts | 87 ++++++++++--------- .../segmentCollection/segmentCollection.ts | 6 +- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/packages/rum/src/domain/segmentCollection/segment.spec.ts b/packages/rum/src/domain/segmentCollection/segment.spec.ts index 50394db92b..1b028e5b39 100644 --- a/packages/rum/src/domain/segmentCollection/segment.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segment.spec.ts @@ -7,8 +7,8 @@ import type { CreationReason, BrowserRecord, SegmentContext, BrowserSegment, Bro import { RecordType } from '../../types' import { getReplayStats, resetReplayStats } from '../replayStats' import { createDeflateEncoder } from '../deflate' -import type { AddRecordCallback, FlushCallback } from './segment' -import { Segment } from './segment' +import type { AddRecordCallback, FlushCallback, Segment } from './segment' +import { segmentFactory } from './segment' const CONTEXT: SegmentContext = { application: { id: 'a' }, view: { id: 'b' }, session: { id: 'c' } } const RECORD_TIMESTAMP = 10 as TimeStamp @@ -265,7 +265,7 @@ describe('Segment', () => { context?: SegmentContext creationReason?: CreationReason } = {}) { - return new Segment(encoder, context, creationReason) + return segmentFactory({ encoder, context, creationReason }) } }) diff --git a/packages/rum/src/domain/segmentCollection/segment.ts b/packages/rum/src/domain/segmentCollection/segment.ts index 97f965797b..4f8a940240 100644 --- a/packages/rum/src/domain/segmentCollection/segment.ts +++ b/packages/rum/src/domain/segmentCollection/segment.ts @@ -8,55 +8,60 @@ export type FlushReason = Exclude | 'stop' export type FlushCallback = (metadata: BrowserSegmentMetadata, encoderResult: EncoderResult) => void export type AddRecordCallback = (encodedBytesCount: number) => void -export class Segment { - private metadata: BrowserSegmentMetadata - private encodedBytesCount = 0 - - constructor( - private encoder: Encoder, - context: SegmentContext, - creationReason: CreationReason - ) { - const viewId = context.view.id - - this.metadata = assign( - { - start: Infinity, - end: -Infinity, - creation_reason: creationReason, - records_count: 0, - has_full_snapshot: false, - index_in_view: replayStats.getSegmentsCount(viewId), - source: 'browser' as const, - }, - context - ) - - replayStats.addSegment(viewId) - } +export interface Segment { + addRecord: (record: BrowserRecord, callback: AddRecordCallback) => void + flush: (callback: FlushCallback) => void +} - addRecord(record: BrowserRecord, callback: AddRecordCallback): void { - this.metadata.start = Math.min(this.metadata.start, record.timestamp) - this.metadata.end = Math.max(this.metadata.end, record.timestamp) - this.metadata.records_count += 1 - this.metadata.has_full_snapshot ||= record.type === RecordType.FullSnapshot +export function segmentFactory({ + context, + creationReason, + encoder, +}: { + context: SegmentContext + creationReason: CreationReason + encoder: Encoder +}): Segment { + let encodedBytesCount = 0 + const viewId = context.view.id + const metadata: BrowserSegmentMetadata = assign( + { + start: Infinity, + end: -Infinity, + creation_reason: creationReason, + records_count: 0, + has_full_snapshot: false, + index_in_view: replayStats.getSegmentsCount(viewId), + source: 'browser' as const, + }, + context + ) + replayStats.addSegment(viewId) - const prefix = this.encoder.isEmpty ? '{"records":[' : ',' - this.encoder.write(prefix + JSON.stringify(record), (additionalEncodedBytesCount) => { - this.encodedBytesCount += additionalEncodedBytesCount - callback(this.encodedBytesCount) + function addRecord(record: BrowserRecord, callback: AddRecordCallback): void { + metadata.start = Math.min(metadata.start, record.timestamp) + metadata.end = Math.max(metadata.end, record.timestamp) + metadata.records_count += 1 + metadata.has_full_snapshot ||= record.type === RecordType.FullSnapshot + + const prefix = encoder.isEmpty ? '{"records":[' : ',' + encoder.write(prefix + JSON.stringify(record), (additionalEncodedBytesCount) => { + encodedBytesCount += additionalEncodedBytesCount + callback(encodedBytesCount) }) } - flush(callback: FlushCallback) { - if (this.encoder.isEmpty) { + function flush(callback: FlushCallback) { + if (encoder.isEmpty) { throw new Error('Empty segment flushed') } - this.encoder.write(`],${JSON.stringify(this.metadata).slice(1)}\n`) - this.encoder.finish((encoderResult) => { - replayStats.addWroteData(this.metadata.view.id, encoderResult.rawBytesCount) - callback(this.metadata, encoderResult) + encoder.write(`],${JSON.stringify(metadata).slice(1)}\n`) + encoder.finish((encoderResult) => { + replayStats.addWroteData(metadata.view.id, encoderResult.rawBytesCount) + callback(metadata, encoderResult) }) } + + return { addRecord, flush } } diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 0d5d3dddf7..ae69ad22a4 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -4,8 +4,8 @@ import type { LifeCycle, ViewContexts, RumSessionManager, RumConfiguration } fro import { LifeCycleEventType } from '@datadog/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' import { buildReplayPayload } from './buildReplayPayload' -import type { FlushReason } from './segment' -import { Segment } from './segment' +import type { FlushReason, Segment } from './segment' +import { segmentFactory } from './segment' export const SEGMENT_DURATION_LIMIT = 30 * ONE_SECOND /** @@ -136,7 +136,7 @@ export function doStartSegmentCollection( state = { status: SegmentCollectionStatus.SegmentPending, - segment: new Segment(encoder, context, state.nextSegmentCreationReason), + segment: segmentFactory({ encoder, context, creationReason: state.nextSegmentCreationReason }), expirationTimeoutId: setTimeout(() => { flushSegment('segment_duration_limit') }, SEGMENT_DURATION_LIMIT), From 6776865dcbb9845defbc025d6442b31774b4d5b5 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 22 Jul 2024 16:37:05 +0200 Subject: [PATCH 05/12] Discard ValueHistory class in favour of function --- .../core/src/domain/session/sessionManager.ts | 6 +- packages/core/src/index.ts | 7 +- packages/core/src/tools/valueHistory.spec.ts | 4 +- packages/core/src/tools/valueHistory.ts | 71 +++++++++++-------- .../src/domain/action/trackClickActions.ts | 6 +- .../src/domain/contexts/featureFlagContext.ts | 6 +- .../src/domain/contexts/pageStateHistory.ts | 10 +-- .../src/domain/contexts/urlContexts.ts | 4 +- .../src/domain/contexts/viewContexts.ts | 4 +- 9 files changed, 69 insertions(+), 49 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 526899d937..9db77183ff 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -1,6 +1,6 @@ import { Observable } from '../../tools/observable' import type { Context } from '../../tools/serialisation/context' -import { ValueHistory } from '../../tools/valueHistory' +import { valueHistoryFactory } from '../../tools/valueHistory' import type { RelativeTime } from '../../tools/utils/timeUtils' import { relativeNow, clocksOrigin, ONE_MINUTE } from '../../tools/utils/timeUtils' import { DOM_EVENT, addEventListener, addEventListeners } from '../../browser/addEventListener' @@ -46,7 +46,9 @@ export function startSessionManager( const sessionStore = startSessionStore(configuration.sessionStoreStrategyType!, productKey, computeSessionState) stopCallbacks.push(() => sessionStore.stop()) - const sessionContextHistory = new ValueHistory>(SESSION_CONTEXT_TIMEOUT_DELAY) + const sessionContextHistory = valueHistoryFactory>({ + expireDelay: SESSION_CONTEXT_TIMEOUT_DELAY, + }) stopCallbacks.push(() => sessionContextHistory.stop()) sessionStore.renewObservable.subscribe(() => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ad54f58d2e..7a77ceb038 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -119,7 +119,12 @@ export { CustomerDataCompressionStatus, } from './domain/context/customerDataTracker' export { CustomerDataType } from './domain/context/contextConstants' -export { ValueHistory, ValueHistoryEntry, CLEAR_OLD_VALUES_INTERVAL } from './tools/valueHistory' +export { + valueHistoryFactory, + type ValueHistory, + ValueHistoryEntry, + CLEAR_OLD_VALUES_INTERVAL, +} from './tools/valueHistory' export { readBytesFromStream } from './tools/readBytesFromStream' export { STORAGE_POLL_DELAY } from './domain/session/sessionStore' export { SESSION_STORE_KEY } from './domain/session/storeStrategies/sessionStoreStrategy' diff --git a/packages/core/src/tools/valueHistory.spec.ts b/packages/core/src/tools/valueHistory.spec.ts index 9f368b7eab..c3da89845d 100644 --- a/packages/core/src/tools/valueHistory.spec.ts +++ b/packages/core/src/tools/valueHistory.spec.ts @@ -2,7 +2,7 @@ import type { Clock } from '../../test' import { mockClock } from '../../test' import type { Duration, RelativeTime } from './utils/timeUtils' import { addDuration, ONE_MINUTE } from './utils/timeUtils' -import { CLEAR_OLD_VALUES_INTERVAL, ValueHistory } from './valueHistory' +import { CLEAR_OLD_VALUES_INTERVAL, type ValueHistory, valueHistoryFactory } from './valueHistory' const EXPIRE_DELAY = 10 * ONE_MINUTE const MAX_ENTRIES = 5 @@ -13,7 +13,7 @@ describe('valueHistory', () => { beforeEach(() => { clock = mockClock() - valueHistory = new ValueHistory(EXPIRE_DELAY, MAX_ENTRIES) + valueHistory = valueHistoryFactory({ expireDelay: EXPIRE_DELAY, maxEntries: MAX_ENTRIES }) }) afterEach(() => { diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index 1f14deaab6..ca45a52ac1 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -20,39 +20,55 @@ export const CLEAR_OLD_VALUES_INTERVAL = ONE_MINUTE * Store and keep track of values spans. This whole class assumes that values are added in * chronological order (i.e. all entries have an increasing start time). */ -export class ValueHistory { - private entries: Array> = [] - private clearOldValuesInterval: TimeoutId - - constructor( - private expireDelay: number, - private maxEntries?: number - ) { - this.clearOldValuesInterval = setInterval(() => this.clearOldValues(), CLEAR_OLD_VALUES_INTERVAL) +export interface ValueHistory { + add: (value: Value, startTime: RelativeTime) => ValueHistoryEntry + find: (startTime?: RelativeTime, options?: { returnInactive: boolean }) => Value | undefined + + closeActive: (endTime: RelativeTime) => void + findAll: (startTime?: RelativeTime, duration?: Duration) => Value[] + reset: () => void + stop: () => void +} + +export function valueHistoryFactory({ + expireDelay, + maxEntries, +}: { + expireDelay: number + maxEntries?: number +}): ValueHistory { + let entries: Array> = [] + const clearOldValuesInterval: TimeoutId = setInterval(() => clearOldValues(), CLEAR_OLD_VALUES_INTERVAL) + + function clearOldValues() { + const oldTimeThreshold = relativeNow() - expireDelay + while (entries.length > 0 && entries[entries.length - 1].endTime < oldTimeThreshold) { + entries.pop() + } } /** * Add a value to the history associated with a start time. Returns a reference to this newly * added entry that can be removed or closed. */ - add(value: Value, startTime: RelativeTime): ValueHistoryEntry { + function add(value: Value, startTime: RelativeTime): ValueHistoryEntry { const entry: ValueHistoryEntry = { value, startTime, endTime: END_OF_TIMES, remove: () => { - removeItem(this.entries, entry) + removeItem(entries, entry) }, close: (endTime: RelativeTime) => { entry.endTime = endTime }, } - if (this.maxEntries && this.entries.length >= this.maxEntries) { - this.entries.pop() + if (maxEntries && entries.length >= maxEntries) { + entries.pop() } - this.entries.unshift(entry) + entries.unshift(entry) return entry } @@ -63,11 +79,11 @@ export class ValueHistory { * * If `option.returnInactive` is true, returns the value at `startTime` (active or not). */ - find( + function find( startTime: RelativeTime = END_OF_TIMES, options: { returnInactive: boolean } = { returnInactive: false } ): Value | undefined { - for (const entry of this.entries) { + for (const entry of entries) { if (entry.startTime <= startTime) { if (options.returnInactive || startTime <= entry.endTime) { return entry.value @@ -81,8 +97,8 @@ export class ValueHistory { * Helper function to close the currently active value, if any. This method assumes that entries * are not overlapping. */ - closeActive(endTime: RelativeTime) { - const latestEntry = this.entries[0] + function closeActive(endTime: RelativeTime) { + const latestEntry = entries[0] if (latestEntry && latestEntry.endTime === END_OF_TIMES) { latestEntry.close(endTime) } @@ -93,9 +109,9 @@ export class ValueHistory { * or all values that were active during `startTime` if no duration is provided, * or all currently active values if no `startTime` is provided. */ - findAll(startTime: RelativeTime = END_OF_TIMES, duration = 0 as Duration): Value[] { + function findAll(startTime: RelativeTime = END_OF_TIMES, duration = 0 as Duration): Value[] { const endTime = addDuration(startTime, duration) - return this.entries + return entries .filter((entry) => entry.startTime <= endTime && startTime <= entry.endTime) .map((entry) => entry.value) } @@ -103,21 +119,16 @@ export class ValueHistory { /** * Remove all entries from this collection. */ - reset() { - this.entries = [] + function reset() { + entries = [] } /** * Stop internal garbage collection of past entries. */ - stop() { - clearInterval(this.clearOldValuesInterval) + function stop() { + clearInterval(clearOldValuesInterval) } - private clearOldValues() { - const oldTimeThreshold = relativeNow() - this.expireDelay - while (this.entries.length > 0 && this.entries[this.entries.length - 1].endTime < oldTimeThreshold) { - this.entries.pop() - } - } + return { add, find, closeActive, findAll, reset, stop } } diff --git a/packages/rum-core/src/domain/action/trackClickActions.ts b/packages/rum-core/src/domain/action/trackClickActions.ts index 5f47ba98e0..06c5cd31b0 100644 --- a/packages/rum-core/src/domain/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/action/trackClickActions.ts @@ -1,4 +1,4 @@ -import type { Duration, ClocksState, RelativeTime, TimeStamp } from '@datadog/browser-core' +import type { Duration, ClocksState, RelativeTime, TimeStamp, ValueHistory } from '@datadog/browser-core' import { includes, timeStampNow, @@ -6,11 +6,11 @@ import { assign, getRelativeTime, ONE_MINUTE, - ValueHistory, generateUUID, clocksNow, ONE_SECOND, elapsed, + valueHistoryFactory, } from '@datadog/browser-core' import type { FrustrationType } from '../../rawRumEvent.types' import { ActionType } from '../../rawRumEvent.types' @@ -67,7 +67,7 @@ export function trackClickActions( domMutationObservable: Observable, configuration: RumConfiguration ) { - const history: ClickActionIdHistory = new ValueHistory(ACTION_CONTEXT_TIME_OUT_DELAY) + const history: ClickActionIdHistory = valueHistoryFactory({ expireDelay: ACTION_CONTEXT_TIME_OUT_DELAY }) const stopObservable = new Observable() let currentClickChain: ClickChain | undefined diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.ts index ecdd2c256d..6e48fa9dd4 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.ts @@ -1,5 +1,5 @@ import type { RelativeTime, ContextValue, Context, CustomerDataTracker } from '@datadog/browser-core' -import { SESSION_TIME_OUT_DELAY, ValueHistory } from '@datadog/browser-core' +import { SESSION_TIME_OUT_DELAY, valueHistoryFactory } from '@datadog/browser-core' import type { LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' @@ -26,7 +26,9 @@ export function startFeatureFlagContexts( lifeCycle: LifeCycle, customerDataTracker: CustomerDataTracker ): FeatureFlagContexts { - const featureFlagContexts = new ValueHistory(FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY) + const featureFlagContexts = valueHistoryFactory({ + expireDelay: FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY, + }) lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, ({ startClocks }) => { featureFlagContexts.add({}, startClocks.relative) diff --git a/packages/rum-core/src/domain/contexts/pageStateHistory.ts b/packages/rum-core/src/domain/contexts/pageStateHistory.ts index 3df0f93d76..e5fdd50a4b 100644 --- a/packages/rum-core/src/domain/contexts/pageStateHistory.ts +++ b/packages/rum-core/src/domain/contexts/pageStateHistory.ts @@ -1,7 +1,7 @@ import type { Duration, RelativeTime } from '@datadog/browser-core' import { elapsed, - ValueHistory, + valueHistoryFactory, SESSION_TIME_OUT_DELAY, toServerDuration, addEventListeners, @@ -40,10 +40,10 @@ export function startPageStateHistory( configuration: RumConfiguration, maxPageStateEntriesSelectable = MAX_PAGE_STATE_ENTRIES_SELECTABLE ): PageStateHistory { - const pageStateEntryHistory = new ValueHistory( - PAGE_STATE_CONTEXT_TIME_OUT_DELAY, - MAX_PAGE_STATE_ENTRIES - ) + const pageStateEntryHistory = valueHistoryFactory({ + expireDelay: PAGE_STATE_CONTEXT_TIME_OUT_DELAY, + maxEntries: MAX_PAGE_STATE_ENTRIES, + }) let currentPageState: PageState addPageState(getPageState(), relativeNow()) diff --git a/packages/rum-core/src/domain/contexts/urlContexts.ts b/packages/rum-core/src/domain/contexts/urlContexts.ts index 51f5ab3679..318977b02e 100644 --- a/packages/rum-core/src/domain/contexts/urlContexts.ts +++ b/packages/rum-core/src/domain/contexts/urlContexts.ts @@ -1,5 +1,5 @@ import type { RelativeTime, Observable } from '@datadog/browser-core' -import { SESSION_TIME_OUT_DELAY, relativeNow, ValueHistory } from '@datadog/browser-core' +import { SESSION_TIME_OUT_DELAY, relativeNow, valueHistoryFactory } from '@datadog/browser-core' import type { LocationChange } from '../../browser/locationChangeObservable' import type { LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' @@ -27,7 +27,7 @@ export function startUrlContexts( locationChangeObservable: Observable, location: Location ) { - const urlContextHistory = new ValueHistory(URL_CONTEXT_TIME_OUT_DELAY) + const urlContextHistory = valueHistoryFactory({ expireDelay: URL_CONTEXT_TIME_OUT_DELAY }) let previousViewUrl: string | undefined diff --git a/packages/rum-core/src/domain/contexts/viewContexts.ts b/packages/rum-core/src/domain/contexts/viewContexts.ts index d83bb945fb..cadeda4485 100644 --- a/packages/rum-core/src/domain/contexts/viewContexts.ts +++ b/packages/rum-core/src/domain/contexts/viewContexts.ts @@ -1,5 +1,5 @@ import type { RelativeTime, ClocksState } from '@datadog/browser-core' -import { SESSION_TIME_OUT_DELAY, ValueHistory } from '@datadog/browser-core' +import { SESSION_TIME_OUT_DELAY, valueHistoryFactory } from '@datadog/browser-core' import type { LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' import type { ViewCreatedEvent, ViewEvent } from '../view/trackViews' @@ -20,7 +20,7 @@ export interface ViewContexts { } export function startViewContexts(lifeCycle: LifeCycle): ViewContexts { - const viewContextHistory = new ValueHistory(VIEW_CONTEXT_TIME_OUT_DELAY) + const viewContextHistory = valueHistoryFactory({ expireDelay: VIEW_CONTEXT_TIME_OUT_DELAY }) lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, (view) => { viewContextHistory.add(buildViewContext(view), view.startClocks.relative) From 23515c8eecf5d8f285b1f255f3c5e3bf8d6fc20a Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 22 Jul 2024 16:44:08 +0200 Subject: [PATCH 06/12] Fix compatibility issue --- packages/core/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7a77ceb038..7c5deb60ea 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -121,7 +121,7 @@ export { export { CustomerDataType } from './domain/context/contextConstants' export { valueHistoryFactory, - type ValueHistory, + ValueHistory, ValueHistoryEntry, CLEAR_OLD_VALUES_INTERVAL, } from './tools/valueHistory' From 54892f9c32d5ed1023e07a82ce7f47b2c7155249 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Mon, 22 Jul 2024 16:51:39 +0200 Subject: [PATCH 07/12] Format code --- packages/core/src/index.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7c5deb60ea..41ad98abe6 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -119,12 +119,7 @@ export { CustomerDataCompressionStatus, } from './domain/context/customerDataTracker' export { CustomerDataType } from './domain/context/contextConstants' -export { - valueHistoryFactory, - ValueHistory, - ValueHistoryEntry, - CLEAR_OLD_VALUES_INTERVAL, -} from './tools/valueHistory' +export { valueHistoryFactory, ValueHistory, ValueHistoryEntry, CLEAR_OLD_VALUES_INTERVAL } from './tools/valueHistory' export { readBytesFromStream } from './tools/readBytesFromStream' export { STORAGE_POLL_DELAY } from './domain/session/sessionStore' export { SESSION_STORE_KEY } from './domain/session/storeStrategies/sessionStoreStrategy' From 21b17ad80bcd6b92ab618c9c3c134649150571e8 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Tue, 23 Jul 2024 10:07:57 +0200 Subject: [PATCH 08/12] Update comment --- packages/core/src/tools/valueHistory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index ca45a52ac1..db7f7c6ff8 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -17,7 +17,7 @@ export interface ValueHistoryEntry { export const CLEAR_OLD_VALUES_INTERVAL = ONE_MINUTE /** - * Store and keep track of values spans. This whole class assumes that values are added in + * Store and keep track of values spans. This whole cache assumes that values are added in * chronological order (i.e. all entries have an increasing start time). */ export interface ValueHistory { From be71e3b8fe26b64df41b47e3d21d83a3c4537c37 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Tue, 23 Jul 2024 14:37:13 +0200 Subject: [PATCH 09/12] Discard Batch class in favour of function --- packages/core/src/transport/batch.spec.ts | 4 +- packages/core/src/transport/batch.ts | 183 +++++++++--------- .../transport/startBatchWithReplica.spec.ts | 79 ++++---- .../src/transport/startBatchWithReplica.ts | 15 +- 4 files changed, 147 insertions(+), 134 deletions(-) diff --git a/packages/core/src/transport/batch.spec.ts b/packages/core/src/transport/batch.spec.ts index 42c52f8647..58330b09e5 100644 --- a/packages/core/src/transport/batch.spec.ts +++ b/packages/core/src/transport/batch.spec.ts @@ -3,7 +3,7 @@ import { createMockFlushController } from '../../test' import { display } from '../tools/display' import type { Encoder } from '../tools/encoder' import { createIdentityEncoder } from '../tools/encoder' -import { Batch } from './batch' +import { batchFactory, type Batch } from './batch' import type { HttpRequest } from './httpRequest' describe('batch', () => { @@ -30,7 +30,7 @@ describe('batch', () => { } satisfies HttpRequest flushController = createMockFlushController() encoder = createIdentityEncoder() - batch = new Batch(encoder, transport, flushController, MESSAGE_BYTES_LIMIT) + batch = batchFactory({ encoder, request: transport, flushController, messageBytesLimit: MESSAGE_BYTES_LIMIT }) }) it('should send a message', () => { diff --git a/packages/core/src/transport/batch.ts b/packages/core/src/transport/batch.ts index ac3611252e..91e021c9bb 100644 --- a/packages/core/src/transport/batch.ts +++ b/packages/core/src/transport/batch.ts @@ -3,43 +3,104 @@ import type { Context } from '../tools/serialisation/context' import { objectValues } from '../tools/utils/polyfills' import { isPageExitReason } from '../browser/pageExitObservable' import { jsonStringify } from '../tools/serialisation/jsonStringify' -import type { Subscription } from '../tools/observable' import type { Encoder, EncoderResult } from '../tools/encoder' import { computeBytesCount } from '../tools/utils/byteUtils' import type { HttpRequest, Payload } from './httpRequest' import type { FlushController, FlushEvent } from './flushController' -export class Batch { - private upsertBuffer: { [key: string]: string } = {} - private flushSubscription: Subscription - - constructor( - private encoder: Encoder, - private request: HttpRequest, - public flushController: FlushController, - private messageBytesLimit: number - ) { - this.flushSubscription = this.flushController.flushObservable.subscribe((event) => this.flush(event)) +export interface Batch { + flushController: FlushController + add: (message: Context) => void + upsert: (message: Context, key: string) => void + stop: () => void +} + +function formatPayloadFromEncoder(encoderResult: EncoderResult): Payload { + let data: string | Blob + if (typeof encoderResult.output === 'string') { + data = encoderResult.output + } else { + data = new Blob([encoderResult.output], { + // This will set the 'Content-Type: text/plain' header. Reasoning: + // * The intake rejects the request if there is no content type. + // * The browser will issue CORS preflight requests if we set it to 'application/json', which + // could induce higher intake load (and maybe has other impacts). + // * Also it's not quite JSON, since we are concatenating multiple JSON objects separated by + // new lines. + type: 'text/plain', + }) + } + + return { + data, + bytesCount: encoderResult.outputBytesCount, + encoding: encoderResult.encoding, } +} + +export function batchFactory({ + encoder, + request, + flushController, + messageBytesLimit, +}: { + encoder: Encoder + request: HttpRequest + flushController: FlushController + messageBytesLimit: number +}): Batch { + let upsertBuffer: { [key: string]: string } = {} + const flushSubscription = flushController.flushObservable.subscribe((event) => flush(event)) + + function push(serializedMessage: string, estimatedMessageBytesCount: number, key?: string) { + flushController.notifyBeforeAddMessage(estimatedMessageBytesCount) - add(message: Context) { - this.addOrUpdate(message) + if (key !== undefined) { + upsertBuffer[key] = serializedMessage + flushController.notifyAfterAddMessage() + } else { + encoder.write(encoder.isEmpty ? serializedMessage : `\n${serializedMessage}`, (realMessageBytesCount) => { + flushController.notifyAfterAddMessage(realMessageBytesCount - estimatedMessageBytesCount) + }) + } } - upsert(message: Context, key: string) { - this.addOrUpdate(message, key) + function hasMessageFor(key?: string): key is string { + return key !== undefined && upsertBuffer[key] !== undefined } - stop() { - this.flushSubscription.unsubscribe() + function remove(key: string) { + const removedMessage = upsertBuffer[key] + delete upsertBuffer[key] + const messageBytesCount = encoder.estimateEncodedBytesCount(removedMessage) + flushController.notifyAfterRemoveMessage(messageBytesCount) } - private flush(event: FlushEvent) { - const upsertMessages = objectValues(this.upsertBuffer).join('\n') - this.upsertBuffer = {} + function addOrUpdate(message: Context, key?: string) { + const serializedMessage = jsonStringify(message)! + + const estimatedMessageBytesCount = encoder.estimateEncodedBytesCount(serializedMessage) + + if (estimatedMessageBytesCount >= messageBytesLimit) { + display.warn( + `Discarded a message exceeding the maximum allowed size ${messageBytesLimit}KB. More details: ${DOCS_ORIGIN}/real_user_monitoring/browser/troubleshooting/#technical-limitations` + ) + return + } + + if (hasMessageFor(key)) { + remove(key) + } + + push(serializedMessage, estimatedMessageBytesCount, key) + } + + function flush(event: FlushEvent) { + const upsertMessages = objectValues(upsertBuffer).join('\n') + upsertBuffer = {} const isPageExit = isPageExitReason(event.reason) - const send = isPageExit ? this.request.sendOnExit : this.request.send + const send = isPageExit ? request.sendOnExit : request.send if ( isPageExit && @@ -47,9 +108,9 @@ export class Batch { // if the encoder is async we need to send two requests in some cases (one for encoded data // and the other for non-encoded data). But if it's not async, we don't have to worry about // it and always send a single request. - this.encoder.isAsync + encoder.isAsync ) { - const encoderResult = this.encoder.finishSync() + const encoderResult = encoder.finishSync() // Send encoded messages if (encoderResult.outputBytesCount) { @@ -66,80 +127,18 @@ export class Batch { } } else { if (upsertMessages) { - this.encoder.write(this.encoder.isEmpty ? upsertMessages : `\n${upsertMessages}`) + encoder.write(encoder.isEmpty ? upsertMessages : `\n${upsertMessages}`) } - this.encoder.finish((encoderResult) => { + encoder.finish((encoderResult) => { send(formatPayloadFromEncoder(encoderResult)) }) } } - private addOrUpdate(message: Context, key?: string) { - const serializedMessage = jsonStringify(message)! - - const estimatedMessageBytesCount = this.encoder.estimateEncodedBytesCount(serializedMessage) - - if (estimatedMessageBytesCount >= this.messageBytesLimit) { - display.warn( - `Discarded a message whose size was bigger than the maximum allowed size ${this.messageBytesLimit}KB. More details: ${DOCS_ORIGIN}/real_user_monitoring/browser/troubleshooting/#technical-limitations` - ) - return - } - - if (this.hasMessageFor(key)) { - this.remove(key) - } - - this.push(serializedMessage, estimatedMessageBytesCount, key) - } - - private push(serializedMessage: string, estimatedMessageBytesCount: number, key?: string) { - this.flushController.notifyBeforeAddMessage(estimatedMessageBytesCount) - - if (key !== undefined) { - this.upsertBuffer[key] = serializedMessage - this.flushController.notifyAfterAddMessage() - } else { - this.encoder.write( - this.encoder.isEmpty ? serializedMessage : `\n${serializedMessage}`, - (realMessageBytesCount) => { - this.flushController.notifyAfterAddMessage(realMessageBytesCount - estimatedMessageBytesCount) - } - ) - } - } - - private remove(key: string) { - const removedMessage = this.upsertBuffer[key] - delete this.upsertBuffer[key] - const messageBytesCount = this.encoder.estimateEncodedBytesCount(removedMessage) - this.flushController.notifyAfterRemoveMessage(messageBytesCount) - } - - private hasMessageFor(key?: string): key is string { - return key !== undefined && this.upsertBuffer[key] !== undefined - } -} - -function formatPayloadFromEncoder(encoderResult: EncoderResult): Payload { - let data: string | Blob - if (typeof encoderResult.output === 'string') { - data = encoderResult.output - } else { - data = new Blob([encoderResult.output], { - // This will set the 'Content-Type: text/plain' header. Reasoning: - // * The intake rejects the request if there is no content type. - // * The browser will issue CORS preflight requests if we set it to 'application/json', which - // could induce higher intake load (and maybe has other impacts). - // * Also it's not quite JSON, since we are concatenating multiple JSON objects separated by - // new lines. - type: 'text/plain', - }) - } - return { - data, - bytesCount: encoderResult.outputBytesCount, - encoding: encoderResult.encoding, + flushController, + add: addOrUpdate, + upsert: addOrUpdate, + stop: flushSubscription.unsubscribe, } } diff --git a/packages/core/src/transport/startBatchWithReplica.spec.ts b/packages/core/src/transport/startBatchWithReplica.spec.ts index f9972bf1c8..924b21e32c 100644 --- a/packages/core/src/transport/startBatchWithReplica.spec.ts +++ b/packages/core/src/transport/startBatchWithReplica.spec.ts @@ -5,7 +5,8 @@ import type { RawError } from '../domain/error/error.types' import { createIdentityEncoder } from '../tools/encoder' import { Observable } from '../tools/observable' import { noop } from '../tools/utils/functionUtils' -import { Batch } from './batch' +import type { batchFactory } from './batch' +import type { FlushController } from './flushController' import type { BatchConfiguration } from './startBatchWithReplica' import { startBatchWithReplica } from './startBatchWithReplica' @@ -15,6 +16,9 @@ describe('startBatchWithReplica', () => { let pageExitObservable: Observable let sessionExpireObservable: Observable let batchConfiguration: BatchConfiguration + let batchFactoryAddSpy: jasmine.Spy + let batchFactoryUpsertSpy: jasmine.Spy + let batchFactoryFakeImpl: typeof batchFactory beforeEach(() => { pageExitObservable = new Observable() @@ -23,77 +27,84 @@ describe('startBatchWithReplica', () => { endpoint: mockEndpointBuilder('https://example.com'), encoder: createIdentityEncoder(), } + + batchFactoryAddSpy = jasmine.createSpy() + batchFactoryUpsertSpy = jasmine.createSpy() + batchFactoryFakeImpl = () => ({ + flushController: {} as FlushController, + add: batchFactoryAddSpy, + upsert: batchFactoryUpsertSpy, + stop: noop, + }) }) it('adds a message to a batch and its replica', () => { - const batchAddSpy = spyOn(Batch.prototype, 'add') - const batch = startBatchWithReplica( DEFAULT_CONFIGURATION, batchConfiguration, batchConfiguration, reportError, pageExitObservable, - sessionExpireObservable + sessionExpireObservable, + batchFactoryFakeImpl ) + batch.add({ foo: true }) - expect(batchAddSpy.calls.thisFor(0)).not.toBe(batchAddSpy.calls.thisFor(1)) - expect(batchAddSpy).toHaveBeenCalledTimes(2) - expect(batchAddSpy.calls.argsFor(0)).toEqual([{ foo: true }]) - expect(batchAddSpy.calls.argsFor(1)).toEqual([{ foo: true }]) + expect(batchFactoryAddSpy.calls.thisFor(0)).not.toBe(batchFactoryAddSpy.calls.thisFor(1)) + expect(batchFactoryAddSpy).toHaveBeenCalledTimes(2) + expect(batchFactoryAddSpy.calls.argsFor(0)).toEqual([{ foo: true }]) + expect(batchFactoryAddSpy.calls.argsFor(1)).toEqual([{ foo: true }]) }) it('does not add a message to the replica if no replica is specified', () => { - const batchAddSpy = spyOn(Batch.prototype, 'add') - const batch = startBatchWithReplica( DEFAULT_CONFIGURATION, batchConfiguration, undefined, reportError, pageExitObservable, - sessionExpireObservable + sessionExpireObservable, + batchFactoryFakeImpl ) + batch.add({ foo: true }) - expect(batchAddSpy).toHaveBeenCalledTimes(1) + expect(batchFactoryAddSpy).toHaveBeenCalledTimes(1) }) it("does not add a message to the replica if it shouldn't be replicated", () => { - const batchAddSpy = spyOn(Batch.prototype, 'add') - const batch = startBatchWithReplica( DEFAULT_CONFIGURATION, batchConfiguration, batchConfiguration, reportError, pageExitObservable, - sessionExpireObservable + sessionExpireObservable, + batchFactoryFakeImpl ) + batch.add({ foo: true }, false) - expect(batchAddSpy).toHaveBeenCalledTimes(1) + expect(batchFactoryAddSpy).toHaveBeenCalledTimes(1) }) it('upserts a message to a batch and its replica', () => { - const batchUpsertSpy = spyOn(Batch.prototype, 'upsert') - const batch = startBatchWithReplica( DEFAULT_CONFIGURATION, batchConfiguration, batchConfiguration, reportError, pageExitObservable, - sessionExpireObservable + sessionExpireObservable, + batchFactoryFakeImpl ) + batch.upsert({ foo: true }, 'message-id') - expect(batchUpsertSpy).toHaveBeenCalledTimes(2) - expect(batchUpsertSpy.calls.thisFor(0)).not.toBe(batchUpsertSpy.calls.thisFor(1)) - expect(batchUpsertSpy.calls.argsFor(0)).toEqual([{ foo: true }, 'message-id']) - expect(batchUpsertSpy.calls.argsFor(1)).toEqual([{ foo: true }, 'message-id']) + expect(batchFactoryUpsertSpy).toHaveBeenCalledTimes(2) + expect(batchFactoryUpsertSpy.calls.thisFor(0)).not.toBe(batchFactoryUpsertSpy.calls.thisFor(1)) + expect(batchFactoryUpsertSpy.calls.argsFor(0)).toEqual([{ foo: true }, 'message-id']) + expect(batchFactoryUpsertSpy.calls.argsFor(1)).toEqual([{ foo: true }, 'message-id']) }) it('transforms a message when adding it to the replica', () => { - const batchAddSpy = spyOn(Batch.prototype, 'add') - const batch = startBatchWithReplica( DEFAULT_CONFIGURATION, batchConfiguration, @@ -103,16 +114,16 @@ describe('startBatchWithReplica', () => { }, reportError, pageExitObservable, - sessionExpireObservable + sessionExpireObservable, + batchFactoryFakeImpl ) + batch.add({ foo: true }) - expect(batchAddSpy.calls.argsFor(0)).toEqual([{ foo: true }]) - expect(batchAddSpy.calls.argsFor(1)).toEqual([{ foo: true, bar: true }]) + expect(batchFactoryAddSpy.calls.argsFor(0)).toEqual([{ foo: true }]) + expect(batchFactoryAddSpy.calls.argsFor(1)).toEqual([{ foo: true, bar: true }]) }) it('transforms a message when upserting it to the replica', () => { - const batchUpsertSpy = spyOn(Batch.prototype, 'upsert') - const batch = startBatchWithReplica<{ foo?: boolean; bar?: boolean }>( DEFAULT_CONFIGURATION, batchConfiguration, @@ -122,10 +133,12 @@ describe('startBatchWithReplica', () => { }, reportError, pageExitObservable, - sessionExpireObservable + sessionExpireObservable, + batchFactoryFakeImpl ) + batch.upsert({ foo: true }, 'message-id') - expect(batchUpsertSpy.calls.argsFor(0)).toEqual([{ foo: true }, 'message-id']) - expect(batchUpsertSpy.calls.argsFor(1)).toEqual([{ foo: true, bar: true }, 'message-id']) + expect(batchFactoryUpsertSpy.calls.argsFor(0)).toEqual([{ foo: true }, 'message-id']) + expect(batchFactoryUpsertSpy.calls.argsFor(1)).toEqual([{ foo: true, bar: true }, 'message-id']) }) }) diff --git a/packages/core/src/transport/startBatchWithReplica.ts b/packages/core/src/transport/startBatchWithReplica.ts index 83b1e1284c..154cce415d 100644 --- a/packages/core/src/transport/startBatchWithReplica.ts +++ b/packages/core/src/transport/startBatchWithReplica.ts @@ -4,7 +4,7 @@ import type { Observable } from '../tools/observable' import type { PageExitEvent } from '../browser/pageExitObservable' import type { RawError } from '../domain/error/error.types' import type { Encoder } from '../tools/encoder' -import { Batch } from './batch' +import { batchFactory } from './batch' import { createHttpRequest } from './httpRequest' import { createFlushController } from './flushController' @@ -23,24 +23,25 @@ export function startBatchWithReplica( replica: ReplicaBatchConfiguration | undefined, reportError: (error: RawError) => void, pageExitObservable: Observable, - sessionExpireObservable: Observable + sessionExpireObservable: Observable, + batchFactoryImp = batchFactory ) { const primaryBatch = createBatch(configuration, primary) const replicaBatch = replica && createBatch(configuration, replica) function createBatch(configuration: Configuration, { endpoint, encoder }: BatchConfiguration) { - return new Batch( + return batchFactoryImp({ encoder, - createHttpRequest(configuration, endpoint, configuration.batchBytesLimit, reportError), - createFlushController({ + request: createHttpRequest(configuration, endpoint, configuration.batchBytesLimit, reportError), + flushController: createFlushController({ messagesLimit: configuration.batchMessagesLimit, bytesLimit: configuration.batchBytesLimit, durationLimit: configuration.flushTimeout, pageExitObservable, sessionExpireObservable, }), - configuration.messageBytesLimit - ) + messageBytesLimit: configuration.messageBytesLimit, + }) } return { From 0e6765ec9d6f67595d8d9b52c7eb5e968b2dc469 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Thu, 25 Jul 2024 10:37:48 +0200 Subject: [PATCH 10/12] Rename functions to follow pattern --- .../core/src/domain/session/sessionManager.ts | 4 +- .../core/src/domain/telemetry/telemetry.ts | 6 +-- packages/core/src/index.ts | 4 +- packages/core/src/tools/boundedBuffer.spec.ts | 8 ++-- packages/core/src/tools/boundedBuffer.ts | 8 ++-- packages/core/src/tools/valueHistory.spec.ts | 4 +- packages/core/src/tools/valueHistory.ts | 2 +- packages/core/src/transport/batch.spec.ts | 4 +- packages/core/src/transport/batch.ts | 48 +++++++++---------- .../transport/startBatchWithReplica.spec.ts | 4 +- .../src/transport/startBatchWithReplica.ts | 10 ++-- packages/logs/src/boot/preStartLogs.ts | 4 +- packages/rum-core/src/boot/preStartRum.ts | 4 +- .../src/domain/action/trackClickActions.ts | 4 +- .../src/domain/contexts/featureFlagContext.ts | 4 +- .../src/domain/contexts/pageStateHistory.ts | 4 +- .../src/domain/contexts/urlContexts.ts | 4 +- .../src/domain/contexts/viewContexts.ts | 4 +- .../src/domain/requestCollection.spec.ts | 10 ++-- .../resource/resourceCollection.spec.ts | 26 +++++----- .../src/domain/resource/resourceCollection.ts | 4 +- .../src/domain/tracing/tracer.spec.ts | 14 +++--- .../rum-core/src/domain/tracing/tracer.ts | 6 +-- .../domain/segmentCollection/segment.spec.ts | 40 ++++++++-------- .../src/domain/segmentCollection/segment.ts | 2 +- .../segmentCollection/segmentCollection.ts | 4 +- performances/src/proxy.ts | 4 +- 27 files changed, 120 insertions(+), 120 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 9db77183ff..0fea27298c 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -1,6 +1,6 @@ import { Observable } from '../../tools/observable' import type { Context } from '../../tools/serialisation/context' -import { valueHistoryFactory } from '../../tools/valueHistory' +import { createValueHistory } from '../../tools/valueHistory' import type { RelativeTime } from '../../tools/utils/timeUtils' import { relativeNow, clocksOrigin, ONE_MINUTE } from '../../tools/utils/timeUtils' import { DOM_EVENT, addEventListener, addEventListeners } from '../../browser/addEventListener' @@ -46,7 +46,7 @@ export function startSessionManager( const sessionStore = startSessionStore(configuration.sessionStoreStrategyType!, productKey, computeSessionState) stopCallbacks.push(() => sessionStore.stop()) - const sessionContextHistory = valueHistoryFactory>({ + const sessionContextHistory = createValueHistory>({ expireDelay: SESSION_CONTEXT_TIMEOUT_DELAY, }) stopCallbacks.push(() => sessionContextHistory.stop()) diff --git a/packages/core/src/domain/telemetry/telemetry.ts b/packages/core/src/domain/telemetry/telemetry.ts index a98b008b4f..e748abe3bf 100644 --- a/packages/core/src/domain/telemetry/telemetry.ts +++ b/packages/core/src/domain/telemetry/telemetry.ts @@ -17,7 +17,7 @@ import { NonErrorPrefix } from '../error/error.types' import type { StackTrace } from '../../tools/stackTrace/computeStackTrace' import { computeStackTrace } from '../../tools/stackTrace/computeStackTrace' import { getConnectivity } from '../connectivity' -import { boundedBuffer } from '../../tools/boundedBuffer' +import { createBoundedBuffer } from '../../tools/boundedBuffer' import type { TelemetryEvent } from './telemetryEvent.types' import type { RawTelemetryConfiguration, @@ -53,7 +53,7 @@ export interface Telemetry { const TELEMETRY_EXCLUDED_SITES: string[] = [INTAKE_SITE_US1_FED] // eslint-disable-next-line local-rules/disallow-side-effects -let preStartTelemetryBuffer = boundedBuffer() +let preStartTelemetryBuffer = createBoundedBuffer() let onRawTelemetryEventCollected = (event: RawTelemetryEvent) => { preStartTelemetryBuffer.add(() => onRawTelemetryEventCollected(event)) } @@ -144,7 +144,7 @@ export function drainPreStartTelemetry() { } export function resetTelemetry() { - preStartTelemetryBuffer = boundedBuffer() + preStartTelemetryBuffer = createBoundedBuffer() onRawTelemetryEventCollected = (event: RawTelemetryEvent) => { preStartTelemetryBuffer.add(() => onRawTelemetryEventCollected(event)) } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ec3dc1cd6d..c842be9115 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -107,7 +107,7 @@ export { createPageExitObservable, PageExitEvent, PageExitReason, isPageExitReas export * from './browser/addEventListener' export * from './tools/timer' export { initConsoleObservable, resetConsoleObservable, ConsoleLog } from './domain/console/consoleObservable' -export { boundedBuffer, BoundedBuffer } from './tools/boundedBuffer' +export { createBoundedBuffer, BoundedBuffer } from './tools/boundedBuffer' export { catchUserErrors } from './tools/catchUserErrors' export { createContextManager, ContextManager } from './domain/context/contextManager' export { storeContextManager, removeStorageListeners } from './domain/context/storeContextManager' @@ -120,7 +120,7 @@ export { CustomerDataCompressionStatus, } from './domain/context/customerDataTracker' export { CustomerDataType } from './domain/context/contextConstants' -export { valueHistoryFactory, ValueHistory, ValueHistoryEntry, CLEAR_OLD_VALUES_INTERVAL } from './tools/valueHistory' +export { createValueHistory, ValueHistory, ValueHistoryEntry, CLEAR_OLD_VALUES_INTERVAL } from './tools/valueHistory' export { readBytesFromStream } from './tools/readBytesFromStream' export { STORAGE_POLL_DELAY } from './domain/session/sessionStore' export { SESSION_STORE_KEY } from './domain/session/storeStrategies/sessionStoreStrategy' diff --git a/packages/core/src/tools/boundedBuffer.spec.ts b/packages/core/src/tools/boundedBuffer.spec.ts index ece0cc7653..207816b67f 100644 --- a/packages/core/src/tools/boundedBuffer.spec.ts +++ b/packages/core/src/tools/boundedBuffer.spec.ts @@ -1,9 +1,9 @@ -import { boundedBuffer } from './boundedBuffer' +import { createBoundedBuffer } from './boundedBuffer' describe('BoundedBuffer', () => { it('collect and drain the callbacks', () => { const spy = jasmine.createSpy<() => void>() - const buffered = boundedBuffer() + const buffered = createBoundedBuffer() buffered.add(spy) expect(spy.calls.count()).toBe(0) @@ -17,7 +17,7 @@ describe('BoundedBuffer', () => { it('store at most 500 callbacks', () => { const spy = jasmine.createSpy<() => void>() - const buffered = boundedBuffer() + const buffered = createBoundedBuffer() const limit = 500 for (let i = 0; i < limit + 1; i += 1) { @@ -30,7 +30,7 @@ describe('BoundedBuffer', () => { it('removes a callback', () => { const spy = jasmine.createSpy<() => void>() - const buffered = boundedBuffer() + const buffered = createBoundedBuffer() buffered.add(spy) buffered.remove(spy) diff --git a/packages/core/src/tools/boundedBuffer.ts b/packages/core/src/tools/boundedBuffer.ts index 7074adb180..3b48796966 100644 --- a/packages/core/src/tools/boundedBuffer.ts +++ b/packages/core/src/tools/boundedBuffer.ts @@ -8,21 +8,21 @@ export interface BoundedBuffer { drain: (arg: T) => void } -export function boundedBuffer(): BoundedBuffer { +export function createBoundedBuffer(): BoundedBuffer { const buffer: Array<(arg: T) => void> = [] - function add(callback: (arg: T) => void) { + const add: BoundedBuffer['add'] = (callback: (arg: T) => void) => { const length = buffer.push(callback) if (length > BUFFER_LIMIT) { buffer.splice(0, 1) } } - function remove(callback: (arg: T) => void) { + const remove: BoundedBuffer['remove'] = (callback: (arg: T) => void) => { removeItem(buffer, callback) } - function drain(arg: T) { + const drain = (arg: T) => { buffer.forEach((callback) => callback(arg)) buffer.length = 0 } diff --git a/packages/core/src/tools/valueHistory.spec.ts b/packages/core/src/tools/valueHistory.spec.ts index c3da89845d..f5923a0aa8 100644 --- a/packages/core/src/tools/valueHistory.spec.ts +++ b/packages/core/src/tools/valueHistory.spec.ts @@ -2,7 +2,7 @@ import type { Clock } from '../../test' import { mockClock } from '../../test' import type { Duration, RelativeTime } from './utils/timeUtils' import { addDuration, ONE_MINUTE } from './utils/timeUtils' -import { CLEAR_OLD_VALUES_INTERVAL, type ValueHistory, valueHistoryFactory } from './valueHistory' +import { CLEAR_OLD_VALUES_INTERVAL, type ValueHistory, createValueHistory } from './valueHistory' const EXPIRE_DELAY = 10 * ONE_MINUTE const MAX_ENTRIES = 5 @@ -13,7 +13,7 @@ describe('valueHistory', () => { beforeEach(() => { clock = mockClock() - valueHistory = valueHistoryFactory({ expireDelay: EXPIRE_DELAY, maxEntries: MAX_ENTRIES }) + valueHistory = createValueHistory({ expireDelay: EXPIRE_DELAY, maxEntries: MAX_ENTRIES }) }) afterEach(() => { diff --git a/packages/core/src/tools/valueHistory.ts b/packages/core/src/tools/valueHistory.ts index db7f7c6ff8..6115257166 100644 --- a/packages/core/src/tools/valueHistory.ts +++ b/packages/core/src/tools/valueHistory.ts @@ -30,7 +30,7 @@ export interface ValueHistory { stop: () => void } -export function valueHistoryFactory({ +export function createValueHistory({ expireDelay, maxEntries, }: { diff --git a/packages/core/src/transport/batch.spec.ts b/packages/core/src/transport/batch.spec.ts index 58330b09e5..4a54fbf6d9 100644 --- a/packages/core/src/transport/batch.spec.ts +++ b/packages/core/src/transport/batch.spec.ts @@ -3,7 +3,7 @@ import { createMockFlushController } from '../../test' import { display } from '../tools/display' import type { Encoder } from '../tools/encoder' import { createIdentityEncoder } from '../tools/encoder' -import { batchFactory, type Batch } from './batch' +import { createBatch, type Batch } from './batch' import type { HttpRequest } from './httpRequest' describe('batch', () => { @@ -30,7 +30,7 @@ describe('batch', () => { } satisfies HttpRequest flushController = createMockFlushController() encoder = createIdentityEncoder() - batch = batchFactory({ encoder, request: transport, flushController, messageBytesLimit: MESSAGE_BYTES_LIMIT }) + batch = createBatch({ encoder, request: transport, flushController, messageBytesLimit: MESSAGE_BYTES_LIMIT }) }) it('should send a message', () => { diff --git a/packages/core/src/transport/batch.ts b/packages/core/src/transport/batch.ts index 97a4178500..0e234b5972 100644 --- a/packages/core/src/transport/batch.ts +++ b/packages/core/src/transport/batch.ts @@ -15,30 +15,7 @@ export interface Batch { stop: () => void } -function formatPayloadFromEncoder(encoderResult: EncoderResult): Payload { - let data: string | Blob - if (typeof encoderResult.output === 'string') { - data = encoderResult.output - } else { - data = new Blob([encoderResult.output], { - // This will set the 'Content-Type: text/plain' header. Reasoning: - // * The intake rejects the request if there is no content type. - // * The browser will issue CORS preflight requests if we set it to 'application/json', which - // could induce higher intake load (and maybe has other impacts). - // * Also it's not quite JSON, since we are concatenating multiple JSON objects separated by - // new lines. - type: 'text/plain', - }) - } - - return { - data, - bytesCount: encoderResult.outputBytesCount, - encoding: encoderResult.encoding, - } -} - -export function batchFactory({ +export function createBatch({ encoder, request, flushController, @@ -142,3 +119,26 @@ export function batchFactory({ stop: flushSubscription.unsubscribe, } } + +function formatPayloadFromEncoder(encoderResult: EncoderResult): Payload { + let data: string | Blob + if (typeof encoderResult.output === 'string') { + data = encoderResult.output + } else { + data = new Blob([encoderResult.output], { + // This will set the 'Content-Type: text/plain' header. Reasoning: + // * The intake rejects the request if there is no content type. + // * The browser will issue CORS preflight requests if we set it to 'application/json', which + // could induce higher intake load (and maybe has other impacts). + // * Also it's not quite JSON, since we are concatenating multiple JSON objects separated by + // new lines. + type: 'text/plain', + }) + } + + return { + data, + bytesCount: encoderResult.outputBytesCount, + encoding: encoderResult.encoding, + } +} diff --git a/packages/core/src/transport/startBatchWithReplica.spec.ts b/packages/core/src/transport/startBatchWithReplica.spec.ts index 924b21e32c..a3025f0250 100644 --- a/packages/core/src/transport/startBatchWithReplica.spec.ts +++ b/packages/core/src/transport/startBatchWithReplica.spec.ts @@ -5,7 +5,7 @@ import type { RawError } from '../domain/error/error.types' import { createIdentityEncoder } from '../tools/encoder' import { Observable } from '../tools/observable' import { noop } from '../tools/utils/functionUtils' -import type { batchFactory } from './batch' +import type { createBatch } from './batch' import type { FlushController } from './flushController' import type { BatchConfiguration } from './startBatchWithReplica' import { startBatchWithReplica } from './startBatchWithReplica' @@ -18,7 +18,7 @@ describe('startBatchWithReplica', () => { let batchConfiguration: BatchConfiguration let batchFactoryAddSpy: jasmine.Spy let batchFactoryUpsertSpy: jasmine.Spy - let batchFactoryFakeImpl: typeof batchFactory + let batchFactoryFakeImpl: typeof createBatch beforeEach(() => { pageExitObservable = new Observable() diff --git a/packages/core/src/transport/startBatchWithReplica.ts b/packages/core/src/transport/startBatchWithReplica.ts index 154cce415d..66bf8ca281 100644 --- a/packages/core/src/transport/startBatchWithReplica.ts +++ b/packages/core/src/transport/startBatchWithReplica.ts @@ -4,7 +4,7 @@ import type { Observable } from '../tools/observable' import type { PageExitEvent } from '../browser/pageExitObservable' import type { RawError } from '../domain/error/error.types' import type { Encoder } from '../tools/encoder' -import { batchFactory } from './batch' +import { createBatch } from './batch' import { createHttpRequest } from './httpRequest' import { createFlushController } from './flushController' @@ -24,12 +24,12 @@ export function startBatchWithReplica( reportError: (error: RawError) => void, pageExitObservable: Observable, sessionExpireObservable: Observable, - batchFactoryImp = batchFactory + batchFactoryImp = createBatch ) { - const primaryBatch = createBatch(configuration, primary) - const replicaBatch = replica && createBatch(configuration, replica) + const primaryBatch = createBatchFromConfig(configuration, primary) + const replicaBatch = replica && createBatchFromConfig(configuration, replica) - function createBatch(configuration: Configuration, { endpoint, encoder }: BatchConfiguration) { + function createBatchFromConfig(configuration: Configuration, { endpoint, encoder }: BatchConfiguration) { return batchFactoryImp({ encoder, request: createHttpRequest(configuration, endpoint, configuration.batchBytesLimit, reportError), diff --git a/packages/logs/src/boot/preStartLogs.ts b/packages/logs/src/boot/preStartLogs.ts index 073ad469cf..dbac286fa1 100644 --- a/packages/logs/src/boot/preStartLogs.ts +++ b/packages/logs/src/boot/preStartLogs.ts @@ -1,6 +1,6 @@ import type { TrackingConsentState } from '@datadog/browser-core' import { - boundedBuffer, + createBoundedBuffer, assign, canUseEventBridge, display, @@ -24,7 +24,7 @@ export function createPreStartStrategy( trackingConsentState: TrackingConsentState, doStartLogs: (initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration) => StartLogsResult ): Strategy { - const bufferApiCalls = boundedBuffer() + const bufferApiCalls = createBoundedBuffer() let cachedInitConfiguration: LogsInitConfiguration | undefined let cachedConfiguration: LogsConfiguration | undefined const trackingConsentStateSubscription = trackingConsentState.observable.subscribe(tryStartLogs) diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts index a7878220f9..7d15dbb053 100644 --- a/packages/rum-core/src/boot/preStartRum.ts +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -1,5 +1,5 @@ import { - boundedBuffer, + createBoundedBuffer, display, canUseEventBridge, displayAlreadyInitializedError, @@ -40,7 +40,7 @@ export function createPreStartStrategy( initialViewOptions?: ViewOptions ) => StartRumResult ): Strategy { - const bufferApiCalls = boundedBuffer() + const bufferApiCalls = createBoundedBuffer() let firstStartViewCall: | { options: ViewOptions | undefined; callback: (startRumResult: StartRumResult) => void } | undefined diff --git a/packages/rum-core/src/domain/action/trackClickActions.ts b/packages/rum-core/src/domain/action/trackClickActions.ts index 06c5cd31b0..e6d8553c70 100644 --- a/packages/rum-core/src/domain/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/action/trackClickActions.ts @@ -10,7 +10,7 @@ import { clocksNow, ONE_SECOND, elapsed, - valueHistoryFactory, + createValueHistory, } from '@datadog/browser-core' import type { FrustrationType } from '../../rawRumEvent.types' import { ActionType } from '../../rawRumEvent.types' @@ -67,7 +67,7 @@ export function trackClickActions( domMutationObservable: Observable, configuration: RumConfiguration ) { - const history: ClickActionIdHistory = valueHistoryFactory({ expireDelay: ACTION_CONTEXT_TIME_OUT_DELAY }) + const history: ClickActionIdHistory = createValueHistory({ expireDelay: ACTION_CONTEXT_TIME_OUT_DELAY }) const stopObservable = new Observable() let currentClickChain: ClickChain | undefined diff --git a/packages/rum-core/src/domain/contexts/featureFlagContext.ts b/packages/rum-core/src/domain/contexts/featureFlagContext.ts index 6e48fa9dd4..bd815715dc 100644 --- a/packages/rum-core/src/domain/contexts/featureFlagContext.ts +++ b/packages/rum-core/src/domain/contexts/featureFlagContext.ts @@ -1,5 +1,5 @@ import type { RelativeTime, ContextValue, Context, CustomerDataTracker } from '@datadog/browser-core' -import { SESSION_TIME_OUT_DELAY, valueHistoryFactory } from '@datadog/browser-core' +import { SESSION_TIME_OUT_DELAY, createValueHistory } from '@datadog/browser-core' import type { LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' @@ -26,7 +26,7 @@ export function startFeatureFlagContexts( lifeCycle: LifeCycle, customerDataTracker: CustomerDataTracker ): FeatureFlagContexts { - const featureFlagContexts = valueHistoryFactory({ + const featureFlagContexts = createValueHistory({ expireDelay: FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY, }) diff --git a/packages/rum-core/src/domain/contexts/pageStateHistory.ts b/packages/rum-core/src/domain/contexts/pageStateHistory.ts index e5fdd50a4b..e3eaaa5be3 100644 --- a/packages/rum-core/src/domain/contexts/pageStateHistory.ts +++ b/packages/rum-core/src/domain/contexts/pageStateHistory.ts @@ -1,7 +1,7 @@ import type { Duration, RelativeTime } from '@datadog/browser-core' import { elapsed, - valueHistoryFactory, + createValueHistory, SESSION_TIME_OUT_DELAY, toServerDuration, addEventListeners, @@ -40,7 +40,7 @@ export function startPageStateHistory( configuration: RumConfiguration, maxPageStateEntriesSelectable = MAX_PAGE_STATE_ENTRIES_SELECTABLE ): PageStateHistory { - const pageStateEntryHistory = valueHistoryFactory({ + const pageStateEntryHistory = createValueHistory({ expireDelay: PAGE_STATE_CONTEXT_TIME_OUT_DELAY, maxEntries: MAX_PAGE_STATE_ENTRIES, }) diff --git a/packages/rum-core/src/domain/contexts/urlContexts.ts b/packages/rum-core/src/domain/contexts/urlContexts.ts index 318977b02e..aec6ce3c97 100644 --- a/packages/rum-core/src/domain/contexts/urlContexts.ts +++ b/packages/rum-core/src/domain/contexts/urlContexts.ts @@ -1,5 +1,5 @@ import type { RelativeTime, Observable } from '@datadog/browser-core' -import { SESSION_TIME_OUT_DELAY, relativeNow, valueHistoryFactory } from '@datadog/browser-core' +import { SESSION_TIME_OUT_DELAY, relativeNow, createValueHistory } from '@datadog/browser-core' import type { LocationChange } from '../../browser/locationChangeObservable' import type { LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' @@ -27,7 +27,7 @@ export function startUrlContexts( locationChangeObservable: Observable, location: Location ) { - const urlContextHistory = valueHistoryFactory({ expireDelay: URL_CONTEXT_TIME_OUT_DELAY }) + const urlContextHistory = createValueHistory({ expireDelay: URL_CONTEXT_TIME_OUT_DELAY }) let previousViewUrl: string | undefined diff --git a/packages/rum-core/src/domain/contexts/viewContexts.ts b/packages/rum-core/src/domain/contexts/viewContexts.ts index cadeda4485..43500117d4 100644 --- a/packages/rum-core/src/domain/contexts/viewContexts.ts +++ b/packages/rum-core/src/domain/contexts/viewContexts.ts @@ -1,5 +1,5 @@ import type { RelativeTime, ClocksState } from '@datadog/browser-core' -import { SESSION_TIME_OUT_DELAY, valueHistoryFactory } from '@datadog/browser-core' +import { SESSION_TIME_OUT_DELAY, createValueHistory } from '@datadog/browser-core' import type { LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' import type { ViewCreatedEvent, ViewEvent } from '../view/trackViews' @@ -20,7 +20,7 @@ export interface ViewContexts { } export function startViewContexts(lifeCycle: LifeCycle): ViewContexts { - const viewContextHistory = valueHistoryFactory({ expireDelay: VIEW_CONTEXT_TIME_OUT_DELAY }) + const viewContextHistory = createValueHistory({ expireDelay: VIEW_CONTEXT_TIME_OUT_DELAY }) lifeCycle.subscribe(LifeCycleEventType.BEFORE_VIEW_CREATED, (view) => { viewContextHistory.add(buildViewContext(view), view.startClocks.relative) diff --git a/packages/rum-core/src/domain/requestCollection.spec.ts b/packages/rum-core/src/domain/requestCollection.spec.ts index 578cb27d06..27a8da52fe 100644 --- a/packages/rum-core/src/domain/requestCollection.spec.ts +++ b/packages/rum-core/src/domain/requestCollection.spec.ts @@ -8,7 +8,7 @@ import { LifeCycle, LifeCycleEventType } from './lifeCycle' import type { RequestCompleteEvent, RequestStartEvent } from './requestCollection' import { trackFetch, trackXhr } from './requestCollection' import type { Tracer } from './tracing/tracer' -import { clearTracingIfNeeded, traceIdentifier } from './tracing/tracer' +import { clearTracingIfNeeded, createTraceIdentifier } from './tracing/tracer' const DEFAULT_PAYLOAD = {} as Payload @@ -40,8 +40,8 @@ describe('collect fetch', () => { const tracerStub: Partial = { clearTracingIfNeeded, traceFetch: (context) => { - context.traceId = traceIdentifier() - context.spanId = traceIdentifier() + context.traceId = createTraceIdentifier() + context.spanId = createTraceIdentifier() }, } ;({ stop: stopFetchTracking } = trackFetch(lifeCycle, configuration, tracerStub as Tracer)) @@ -214,8 +214,8 @@ describe('collect xhr', () => { const tracerStub: Partial = { clearTracingIfNeeded, traceXhr: (context) => { - context.traceId = traceIdentifier() - context.spanId = traceIdentifier() + context.traceId = createTraceIdentifier() + context.spanId = createTraceIdentifier() }, } ;({ stop: stopXhrTracking } = trackXhr(lifeCycle, configuration, tracerStub as Tracer)) diff --git a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts index 5a90c2555a..2d658c0763 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts @@ -7,7 +7,7 @@ import type { RawRumResourceEvent } from '../../rawRumEvent.types' import { RumEventType } from '../../rawRumEvent.types' import { LifeCycleEventType } from '../lifeCycle' import type { RequestCompleteEvent } from '../requestCollection' -import { traceIdentifier } from '../tracing/tracer' +import { createTraceIdentifier } from '../tracing/tracer' import { validateAndBuildRumConfiguration } from '../configuration' import type { RumPerformanceEntry } from '../../browser/performanceObservable' import { RumPerformanceEntryType } from '../../browser/performanceObservable' @@ -163,8 +163,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ type: RequestType.XHR, - traceId: traceIdentifier(), - spanId: traceIdentifier(), + traceId: createTraceIdentifier(), + spanId: createTraceIdentifier(), traceSampled: true, }) ) @@ -292,8 +292,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -307,8 +307,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: false, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -334,8 +334,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -359,8 +359,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd @@ -385,8 +385,8 @@ describe('resourceCollection', () => { LifeCycleEventType.REQUEST_COMPLETED, createCompletedRequest({ traceSampled: true, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), }) ) const privateFields = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd diff --git a/packages/rum-core/src/domain/resource/resourceCollection.ts b/packages/rum-core/src/domain/resource/resourceCollection.ts index 19d38e150c..7243de6d3f 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.ts @@ -23,7 +23,7 @@ import type { RawRumEventCollectedData, LifeCycle } from '../lifeCycle' import type { RequestCompleteEvent } from '../requestCollection' import type { PageStateHistory } from '../contexts/pageStateHistory' import { PageState } from '../contexts/pageStateHistory' -import { traceIdentifier } from '../tracing/tracer' +import { createTraceIdentifier } from '../tracing/tracer' import { matchRequestTiming } from './matchRequestTiming' import { computePerformanceResourceDetails, @@ -205,7 +205,7 @@ function computeEntryTracingInfo(entry: RumPerformanceResourceTiming, configurat return { _dd: { trace_id: entry.traceId, - span_id: traceIdentifier().toDecimalString(), + span_id: createTraceIdentifier().toDecimalString(), rule_psr: getRulePsr(configuration), }, } diff --git a/packages/rum-core/src/domain/tracing/tracer.spec.ts b/packages/rum-core/src/domain/tracing/tracer.spec.ts index c8a7036a80..ea4dc1b7e4 100644 --- a/packages/rum-core/src/domain/tracing/tracer.spec.ts +++ b/packages/rum-core/src/domain/tracing/tracer.spec.ts @@ -4,7 +4,7 @@ import { createRumSessionManagerMock } from '../../../test' import type { RumFetchResolveContext, RumFetchStartContext, RumXhrStartContext } from '../requestCollection' import type { RumConfiguration, RumInitConfiguration } from '../configuration' import { validateAndBuildRumConfiguration } from '../configuration' -import { startTracer, traceIdentifier, type TraceIdentifier } from './tracer' +import { startTracer, createTraceIdentifier, type TraceIdentifier } from './tracer' describe('tracer', () => { let configuration: RumConfiguration @@ -622,8 +622,8 @@ describe('tracer', () => { const context: RumFetchResolveContext = { status: 0, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), } as any tracer.clearTracingIfNeeded(context) @@ -636,8 +636,8 @@ describe('tracer', () => { const context: RumFetchResolveContext = { status: 200, - spanId: traceIdentifier(), - traceId: traceIdentifier(), + spanId: createTraceIdentifier(), + traceId: createTraceIdentifier(), } as any tracer.clearTracingIfNeeded(context) @@ -649,13 +649,13 @@ describe('tracer', () => { describe('TraceIdentifier', () => { it('should generate id', () => { - const identifier = traceIdentifier() + const identifier = createTraceIdentifier() expect(identifier.toDecimalString()).toMatch(/^\d+$/) }) // it('should pad the string to 16 characters', () => { - // const identifier = traceIdentifier() + // const identifier = createTraceIdentifier() // // Forcing as any to access private member: buffer // ;(identifier as any).buffer = new Uint8Array([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]) diff --git a/packages/rum-core/src/domain/tracing/tracer.ts b/packages/rum-core/src/domain/tracing/tracer.ts index 47cd730f35..df2d4b84d6 100644 --- a/packages/rum-core/src/domain/tracing/tracer.ts +++ b/packages/rum-core/src/domain/tracing/tracer.ts @@ -124,8 +124,8 @@ function injectHeadersIfTracingAllowed( return } - context.traceId = traceIdentifier() - context.spanId = traceIdentifier() + context.traceId = createTraceIdentifier() + context.spanId = createTraceIdentifier() inject(makeTracingHeaders(context.traceId, context.spanId, context.traceSampled, tracingOption.propagatorTypes)) } @@ -198,7 +198,7 @@ export interface TraceIdentifier { toPaddedHexadecimalString: () => string } -export function traceIdentifier(): TraceIdentifier { +export function createTraceIdentifier(): TraceIdentifier { const buffer: Uint8Array = new Uint8Array(8) getCrypto().getRandomValues(buffer) buffer[0] = buffer[0] & 0x7f // force 63-bit diff --git a/packages/rum/src/domain/segmentCollection/segment.spec.ts b/packages/rum/src/domain/segmentCollection/segment.spec.ts index 1b028e5b39..067a1c2002 100644 --- a/packages/rum/src/domain/segmentCollection/segment.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segment.spec.ts @@ -8,7 +8,7 @@ import { RecordType } from '../../types' import { getReplayStats, resetReplayStats } from '../replayStats' import { createDeflateEncoder } from '../deflate' import type { AddRecordCallback, FlushCallback, Segment } from './segment' -import { segmentFactory } from './segment' +import { createSegment } from './segment' const CONTEXT: SegmentContext = { application: { id: 'a' }, view: { id: 'b' }, session: { id: 'c' } } const RECORD_TIMESTAMP = 10 as TimeStamp @@ -46,7 +46,7 @@ describe('Segment', () => { it('writes a segment', () => { const addRecordCallbackSpy = jasmine.createSpy() const flushCallbackSpy = jasmine.createSpy() - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(RECORD, addRecordCallbackSpy) worker.processAllMessages() @@ -78,7 +78,7 @@ describe('Segment', () => { it('compressed bytes count is updated when a record is added', () => { const addRecordCallbackSpy = jasmine.createSpy() - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(RECORD, addRecordCallbackSpy) worker.processAllMessages() expect(addRecordCallbackSpy).toHaveBeenCalledOnceWith( @@ -88,7 +88,7 @@ describe('Segment', () => { it('calls the flush callback with metadata and encoder output as argument', () => { const flushCallbackSpy = jasmine.createSpy() - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(RECORD, noop) segment.flush(flushCallbackSpy) worker.processAllMessages() @@ -119,11 +119,11 @@ describe('Segment', () => { it('resets the encoder when a segment is flushed', () => { const flushCallbackSpy = jasmine.createSpy() - const segment1 = createSegment({ creationReason: 'init' }) + const segment1 = createTestSegment({ creationReason: 'init' }) segment1.addRecord(RECORD, noop) segment1.flush(flushCallbackSpy) - const segment2 = createSegment({ creationReason: 'segment_duration_limit' }) + const segment2 = createTestSegment({ creationReason: 'segment_duration_limit' }) segment2.addRecord(FULL_SNAPSHOT_RECORD, noop) segment2.flush(flushCallbackSpy) @@ -133,7 +133,7 @@ describe('Segment', () => { }) it('throws when trying to flush an empty segment', () => { - const segment = createSegment() + const segment = createTestSegment() expect(() => segment.flush(noop)).toThrowError('Empty segment flushed') }) @@ -141,7 +141,7 @@ describe('Segment', () => { describe('when adding a record', () => { let segment: Segment beforeEach(() => { - segment = createSegment() + segment = createTestSegment() segment.addRecord({ type: RecordType.ViewEnd, timestamp: 10 as TimeStamp }, noop) segment.addRecord({ type: RecordType.ViewEnd, timestamp: 15 as TimeStamp }, noop) }) @@ -171,19 +171,19 @@ describe('Segment', () => { describe('has_full_snapshot', () => { it('sets has_full_snapshot to false if a segment has a no FullSnapshot', () => { - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment).has_full_snapshot).toEqual(false) }) it('sets has_full_snapshot to true if a segment has a FullSnapshot', () => { - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(FULL_SNAPSHOT_RECORD, noop) expect(flushAndGetMetadata(segment).has_full_snapshot).toEqual(true) }) it("doesn't overrides has_full_snapshot to false once it has been set to true", () => { - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(FULL_SNAPSHOT_RECORD, noop) segment.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment).has_full_snapshot).toEqual(true) @@ -192,25 +192,25 @@ describe('Segment', () => { describe('index_in_view', () => { it('increments index_in_view every time a segment is created for the same view', () => { - const segment1 = createSegment() + const segment1 = createTestSegment() segment1.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment1).index_in_view).toBe(0) - const segment2 = createSegment() + const segment2 = createTestSegment() segment2.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment2).index_in_view).toBe(1) - const segment3 = createSegment() + const segment3 = createTestSegment() segment3.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment3).index_in_view).toBe(2) }) it('resets segments_count when creating a segment for a new view', () => { - const segment1 = createSegment() + const segment1 = createTestSegment() segment1.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment1).index_in_view).toBe(0) - const segment2 = createSegment({ context: { ...CONTEXT, view: { id: 'view-2' } } }) + const segment2 = createTestSegment({ context: { ...CONTEXT, view: { id: 'view-2' } } }) segment2.addRecord(RECORD, noop) expect(flushAndGetMetadata(segment2).index_in_view).toBe(0) }) @@ -232,7 +232,7 @@ describe('Segment', () => { }) it('when creating a segment', () => { - createSegment() + createTestSegment() worker.processAllMessages() expect(getReplayStats('b')).toEqual( jasmine.objectContaining({ @@ -244,7 +244,7 @@ describe('Segment', () => { }) it('when flushing a segment', () => { - const segment = createSegment() + const segment = createTestSegment() segment.addRecord(RECORD, noop) segment.flush(noop) worker.processAllMessages() @@ -258,14 +258,14 @@ describe('Segment', () => { }) }) - function createSegment({ + function createTestSegment({ context = CONTEXT, creationReason = 'init', }: { context?: SegmentContext creationReason?: CreationReason } = {}) { - return segmentFactory({ encoder, context, creationReason }) + return createSegment({ encoder, context, creationReason }) } }) diff --git a/packages/rum/src/domain/segmentCollection/segment.ts b/packages/rum/src/domain/segmentCollection/segment.ts index 4f8a940240..016ab17ea4 100644 --- a/packages/rum/src/domain/segmentCollection/segment.ts +++ b/packages/rum/src/domain/segmentCollection/segment.ts @@ -13,7 +13,7 @@ export interface Segment { flush: (callback: FlushCallback) => void } -export function segmentFactory({ +export function createSegment({ context, creationReason, encoder, diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index ae69ad22a4..2098d8866c 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -5,7 +5,7 @@ import { LifeCycleEventType } from '@datadog/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' import { buildReplayPayload } from './buildReplayPayload' import type { FlushReason, Segment } from './segment' -import { segmentFactory } from './segment' +import { createSegment } from './segment' export const SEGMENT_DURATION_LIMIT = 30 * ONE_SECOND /** @@ -136,7 +136,7 @@ export function doStartSegmentCollection( state = { status: SegmentCollectionStatus.SegmentPending, - segment: segmentFactory({ encoder, context, creationReason: state.nextSegmentCreationReason }), + segment: createSegment({ encoder, context, creationReason: state.nextSegmentCreationReason }), expirationTimeoutId: setTimeout(() => { flushSegment('segment_duration_limit') }, SEGMENT_DURATION_LIMIT), diff --git a/performances/src/proxy.ts b/performances/src/proxy.ts index d753a3cc3b..52f63ef50c 100644 --- a/performances/src/proxy.ts +++ b/performances/src/proxy.ts @@ -18,7 +18,7 @@ interface ProxyStats { reset: () => void } -function proxyStatsFactory(): ProxyStats { +function createProxyStats(): ProxyStats { const statsByHost = new Map() function addRequest(request: IncomingMessage, size: number) { @@ -53,7 +53,7 @@ function proxyStatsFactory(): ProxyStats { export function startProxy() { return new Promise((resolve, reject) => { const { key, cert, spkiFingerprint } = createSelfSignedCertificate() - const stats = proxyStatsFactory() + const stats = createProxyStats() const server = createServer({ key, cert }) server.on('error', reject) server.on('request', (req: IncomingMessage, res: ServerResponse) => { From 57109e1b1b10b41ae833cee6c3fee2ef04f96d8a Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Thu, 25 Jul 2024 10:51:41 +0200 Subject: [PATCH 11/12] Add eslint rule to forbid class usage --- .eslintrc.js | 4 ++++ packages/core/src/tools/abstractLifeCycle.ts | 1 + packages/core/src/tools/observable.ts | 1 + packages/logs/src/domain/logger.ts | 2 +- packages/rum-core/src/browser/polyfills.ts | 1 + packages/rum-react/src/domain/error/errorBoundary.ts | 1 + packages/worker/src/domain/deflate.d.ts | 1 + 7 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 186a1bb223..bf6d274c79 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -230,6 +230,10 @@ module.exports = { 'local-rules/disallow-url-constructor-patched-values': 'error', 'no-restricted-syntax': [ 'error', + { + selector: 'ClassDeclaration', + message: 'Classes are not allowed. Use functions instead.', + }, { selector: 'ObjectExpression > SpreadElement', message: 'Object spread is not authorized. Please use "assign" from the core package utils instead.', diff --git a/packages/core/src/tools/abstractLifeCycle.ts b/packages/core/src/tools/abstractLifeCycle.ts index 3e9048d38c..96e645b2c2 100644 --- a/packages/core/src/tools/abstractLifeCycle.ts +++ b/packages/core/src/tools/abstractLifeCycle.ts @@ -16,6 +16,7 @@ type EventTypesWithoutData = { [K in keyof EventMap]: EventMap[K] extends void ? K : never }[keyof EventMap] +// eslint-disable-next-line no-restricted-syntax export class AbstractLifeCycle { private callbacks: { [key in keyof EventMap]?: Array<(data: any) => void> } = {} diff --git a/packages/core/src/tools/observable.ts b/packages/core/src/tools/observable.ts index 202f6295c9..60b672126b 100644 --- a/packages/core/src/tools/observable.ts +++ b/packages/core/src/tools/observable.ts @@ -2,6 +2,7 @@ export interface Subscription { unsubscribe: () => void } +// eslint-disable-next-line no-restricted-syntax export class Observable { private observers: Array<(data: T) => void> = [] private onLastUnsubscribe?: () => void diff --git a/packages/logs/src/domain/logger.ts b/packages/logs/src/domain/logger.ts index bbe9de17b3..f48256a744 100644 --- a/packages/logs/src/domain/logger.ts +++ b/packages/logs/src/domain/logger.ts @@ -32,7 +32,7 @@ export type HandlerType = (typeof HandlerType)[keyof typeof HandlerType] export const STATUSES = Object.keys(StatusType) as StatusType[] // note: it is safe to merge declarations as long as the methods are actually defined on the prototype -// eslint-disable-next-line @typescript-eslint/no-unsafe-declaration-merging +// eslint-disable-next-line @typescript-eslint/no-unsafe-declaration-merging, no-restricted-syntax export class Logger { private contextManager: ContextManager diff --git a/packages/rum-core/src/browser/polyfills.ts b/packages/rum-core/src/browser/polyfills.ts index 31f32ee803..44bfaea124 100644 --- a/packages/rum-core/src/browser/polyfills.ts +++ b/packages/rum-core/src/browser/polyfills.ts @@ -68,6 +68,7 @@ export function getClassList(element: Element): DOMTokenList | string[] { // ie11 supports WeakMap but not WeakSet const PLACEHOLDER = 1 +// eslint-disable-next-line no-restricted-syntax export class WeakSet { private map = new WeakMap() diff --git a/packages/rum-react/src/domain/error/errorBoundary.ts b/packages/rum-react/src/domain/error/errorBoundary.ts index 682e1ecb03..920a565fbc 100644 --- a/packages/rum-react/src/domain/error/errorBoundary.ts +++ b/packages/rum-react/src/domain/error/errorBoundary.ts @@ -21,6 +21,7 @@ type State = const INITIAL_STATE: State = { didCatch: false, error: null } +// eslint-disable-next-line no-restricted-syntax export class ErrorBoundary extends React.Component { constructor(props: Props) { super(props) diff --git a/packages/worker/src/domain/deflate.d.ts b/packages/worker/src/domain/deflate.d.ts index 1cd22de574..6652168d79 100644 --- a/packages/worker/src/domain/deflate.d.ts +++ b/packages/worker/src/domain/deflate.d.ts @@ -1,3 +1,4 @@ +// eslint-disable-next-line no-restricted-syntax export class Deflate { chunks: Uint8Array[] result: Uint8Array From d7d48c72decacbb908ac48832df2a45794f63d46 Mon Sep 17 00:00:00 2001 From: Najib Boutaib Date: Thu, 25 Jul 2024 11:11:03 +0200 Subject: [PATCH 12/12] Fix unit test --- .../rum-core/src/domain/tracing/tracer.spec.ts | 16 ++++++++-------- packages/rum-core/src/domain/tracing/tracer.ts | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/rum-core/src/domain/tracing/tracer.spec.ts b/packages/rum-core/src/domain/tracing/tracer.spec.ts index ea4dc1b7e4..eff44929d8 100644 --- a/packages/rum-core/src/domain/tracing/tracer.spec.ts +++ b/packages/rum-core/src/domain/tracing/tracer.spec.ts @@ -4,7 +4,7 @@ import { createRumSessionManagerMock } from '../../../test' import type { RumFetchResolveContext, RumFetchStartContext, RumXhrStartContext } from '../requestCollection' import type { RumConfiguration, RumInitConfiguration } from '../configuration' import { validateAndBuildRumConfiguration } from '../configuration' -import { startTracer, createTraceIdentifier, type TraceIdentifier } from './tracer' +import { startTracer, createTraceIdentifier, type TraceIdentifier, getCrypto } from './tracer' describe('tracer', () => { let configuration: RumConfiguration @@ -654,13 +654,13 @@ describe('TraceIdentifier', () => { expect(identifier.toDecimalString()).toMatch(/^\d+$/) }) - // it('should pad the string to 16 characters', () => { - // const identifier = createTraceIdentifier() - // // Forcing as any to access private member: buffer - // ;(identifier as any).buffer = new Uint8Array([0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]) - - // expect(identifier.toPaddedHexadecimalString()).toEqual('0001020304050607') - // }) + it('should pad the string to 16 characters', () => { + spyOn(getCrypto() as any, 'getRandomValues').and.callFake((buffer: Uint8Array) => { + buffer[buffer.length - 1] = 0x01 + }) + const identifier = createTraceIdentifier() + expect(identifier.toPaddedHexadecimalString()).toEqual('0000000000000001') + }) }) function toPlainObject(headers: Headers) { diff --git a/packages/rum-core/src/domain/tracing/tracer.ts b/packages/rum-core/src/domain/tracing/tracer.ts index df2d4b84d6..6d377ea019 100644 --- a/packages/rum-core/src/domain/tracing/tracer.ts +++ b/packages/rum-core/src/domain/tracing/tracer.ts @@ -134,7 +134,7 @@ export function isTracingSupported() { return getCrypto() !== undefined } -function getCrypto() { +export function getCrypto() { return window.crypto || (window as any).msCrypto }