Skip to content

Commit

Permalink
♻️ [RUM-2445] align rum and logs common context implementation
Browse files Browse the repository at this point in the history
Align both implementations by:

* move the logs `buildCommonContext` in its own module (similar to RUM)
* craft a `getCommonContext` based on `buildCommonContext` in both
  rumPublicApi and logsPublicApi, and pass it to `startRum` and
  `startLogs`.

This slightly refactors how common contexts are computed.
  • Loading branch information
BenoitZugmeyer committed Jan 11, 2024
1 parent 235a46f commit 37fd6d9
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 53 deletions.
16 changes: 8 additions & 8 deletions packages/logs/src/boot/logsPublicApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ describe('logs entry', () => {
it('should have the current date, view and global context', () => {
LOGS.setGlobalContextProperty('foo', 'bar')

const buildCommonContext = startLogs.calls.mostRecent().args[2]
expect(buildCommonContext()).toEqual({
const getCommonContext = startLogs.calls.mostRecent().args[2]
expect(getCommonContext()).toEqual({
view: {
referrer: document.referrer,
url: window.location.href,
Expand Down Expand Up @@ -352,8 +352,8 @@ describe('logs entry', () => {
const user = { id: 'foo', name: 'bar', email: 'qux', foo: { bar: 'qux' } }
logsPublicApi.setUser(user)

const buildCommonContext = startLogs.calls.mostRecent().args[2]
expect(buildCommonContext().user).toEqual({
const getCommonContext = startLogs.calls.mostRecent().args[2]
expect(getCommonContext().user).toEqual({
email: 'qux',
foo: { bar: 'qux' },
id: 'foo',
Expand All @@ -364,8 +364,8 @@ describe('logs entry', () => {
it('should sanitize predefined properties', () => {
const user = { id: null, name: 2, email: { bar: 'qux' } }
logsPublicApi.setUser(user as any)
const buildCommonContext = startLogs.calls.mostRecent().args[2]
expect(buildCommonContext().user).toEqual({
const getCommonContext = startLogs.calls.mostRecent().args[2]
expect(getCommonContext().user).toEqual({
email: '[object Object]',
id: 'null',
name: '2',
Expand All @@ -377,8 +377,8 @@ describe('logs entry', () => {
logsPublicApi.setUser(user)
logsPublicApi.clearUser()

const buildCommonContext = startLogs.calls.mostRecent().args[2]
expect(buildCommonContext().user).toEqual({})
const getCommonContext = startLogs.calls.mostRecent().args[2]
expect(getCommonContext().user).toEqual({})
})

it('should reject non object input', () => {
Expand Down
17 changes: 5 additions & 12 deletions packages/logs/src/boot/logsPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import type { LogsInitConfiguration } from '../domain/configuration'
import { validateAndBuildLogsConfiguration } from '../domain/configuration'
import type { HandlerType, StatusType, LogsMessage } from '../domain/logger'
import { Logger } from '../domain/logger'
import type { CommonContext } from '../rawLogsEvent.types'
import { buildCommonContext } from '../domain/contexts/commonContext'
import type { startLogs } from './startLogs'

export interface LoggerConfiguration {
Expand Down Expand Up @@ -55,7 +55,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
let handleLogStrategy: StartLogsResult['handleLog'] = (
logsMessage: LogsMessage,
logger: Logger,
savedCommonContext = deepClone(buildCommonContext()),
savedCommonContext = getCommonContext(),
date = timeStampNow()
) => {
beforeInitLoggerLog.add(() => handleLogStrategy(logsMessage, logger, savedCommonContext, date))
Expand All @@ -67,15 +67,8 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
customerDataTrackerManager.createDetachedTracker()
)

function buildCommonContext(): CommonContext {
return {
view: {
referrer: document.referrer,
url: window.location.href,
},
context: globalContextManager.getContext(),
user: userContextManager.getContext(),
}
function getCommonContext() {
return buildCommonContext(globalContextManager, userContextManager)
}

return makePublicApi({
Expand Down Expand Up @@ -125,7 +118,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
;({ handleLog: handleLogStrategy, getInternalContext: getInternalContextStrategy } = startLogsImpl(
initConfiguration,
configuration,
buildCommonContext
getCommonContext
))

beforeInitLoggerLog.drain()
Expand Down
6 changes: 3 additions & 3 deletions packages/logs/src/boot/startLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ import { startNetworkErrorCollection } from '../domain/networkError/networkError
import { startRuntimeErrorCollection } from '../domain/runtimeError/runtimeErrorCollection'
import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle'
import { startLoggerCollection } from '../domain/logger/loggerCollection'
import type { CommonContext } from '../rawLogsEvent.types'
import { startLogsBatch } from '../transport/startLogsBatch'
import { startLogsBridge } from '../transport/startLogsBridge'
import { startInternalContext } from '../domain/contexts/internalContext'
import { startReportError } from '../domain/reportError'
import { startLogsTelemetry } from '../domain/logsTelemetry'
import type { CommonContext } from '../rawLogsEvent.types'

export function startLogs(
initConfiguration: LogsInitConfiguration,
configuration: LogsConfiguration,
buildCommonContext: () => CommonContext
getCommonContext: () => CommonContext
) {
const lifeCycle = new LifeCycle()
const cleanupTasks: Array<() => void> = []
Expand Down Expand Up @@ -53,7 +53,7 @@ export function startLogs(
startReportCollection(configuration, lifeCycle)
const { handleLog } = startLoggerCollection(lifeCycle)

startLogsAssembly(session, configuration, lifeCycle, buildCommonContext, reportError)
startLogsAssembly(session, configuration, lifeCycle, getCommonContext, reportError)

if (!canUseEventBridge()) {
const { stop: stopLogsBatch } = startLogsBatch(configuration, lifeCycle, reportError, pageExitObservable, session)
Expand Down
4 changes: 2 additions & 2 deletions packages/logs/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function startLogsAssembly(
sessionManager: LogsSessionManager,
configuration: LogsConfiguration,
lifeCycle: LifeCycle,
buildCommonContext: () => CommonContext,
getCommonContext: () => CommonContext,
reportError: (error: RawError) => void
) {
const statusWithCustom = (STATUSES as string[]).concat(['custom'])
Expand All @@ -31,7 +31,7 @@ export function startLogsAssembly(
return
}

const commonContext = savedCommonContext || buildCommonContext()
const commonContext = savedCommonContext || getCommonContext()
const log = combine(
{
service: configuration.service,
Expand Down
16 changes: 16 additions & 0 deletions packages/logs/src/domain/contexts/commonContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type { ContextManager } from '@datadog/browser-core'
import type { CommonContext } from '../../rawLogsEvent.types'

export function buildCommonContext(
globalContextManager: ContextManager,
userContextManager: ContextManager
): CommonContext {
return {
view: {
referrer: document.referrer,
url: window.location.href,
},
context: globalContextManager.getContext(),
user: userContextManager.getContext(),
}
}
10 changes: 5 additions & 5 deletions packages/rum-core/src/boot/rumPublicApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ describe('rum public api', () => {
rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)

expect(startDeflateWorkerSpy).not.toHaveBeenCalled()
const createEncoder: (streamId: DeflateEncoderStreamId) => Encoder = startRumSpy.calls.mostRecent().args[7]
const createEncoder: (streamId: DeflateEncoderStreamId) => Encoder = startRumSpy.calls.mostRecent().args[6]
expect(createEncoder).toBe(createIdentityEncoder)
})
})
Expand All @@ -222,7 +222,7 @@ describe('rum public api', () => {
})

expect(startDeflateWorkerSpy).toHaveBeenCalledTimes(1)
const createEncoder: (streamId: DeflateEncoderStreamId) => Encoder = startRumSpy.calls.mostRecent().args[7]
const createEncoder: (streamId: DeflateEncoderStreamId) => Encoder = startRumSpy.calls.mostRecent().args[6]
expect(createEncoder).not.toBe(createIdentityEncoder)
})

Expand Down Expand Up @@ -896,7 +896,7 @@ describe('rum public api', () => {

rumPublicApi.init(MANUAL_CONFIGURATION)
expect(startRumSpy).toHaveBeenCalled()
const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[6]
const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[5]
expect(initialViewOptions).toEqual({ name: 'foo' })
expect(recorderApiOnRumStartSpy).toHaveBeenCalled()
expect(startViewSpy).not.toHaveBeenCalled()
Expand All @@ -909,7 +909,7 @@ describe('rum public api', () => {

rumPublicApi.startView('foo')
expect(startRumSpy).toHaveBeenCalled()
const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[6]
const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[5]
expect(initialViewOptions).toEqual({ name: 'foo' })
expect(recorderApiOnRumStartSpy).toHaveBeenCalled()
expect(startViewSpy).not.toHaveBeenCalled()
Expand All @@ -921,7 +921,7 @@ describe('rum public api', () => {
rumPublicApi.startView('bar')

expect(startRumSpy).toHaveBeenCalled()
const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[6]
const initialViewOptions: ViewOptions | undefined = startRumSpy.calls.argsFor(0)[5]
expect(initialViewOptions).toEqual({ name: 'foo' })
expect(recorderApiOnRumStartSpy).toHaveBeenCalled()
expect(startViewSpy).toHaveBeenCalled()
Expand Down
17 changes: 7 additions & 10 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ export function makeRumPublicApi(
)
let userContextManager = createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User))

function getCommonContext() {
return buildCommonContext(globalContextManager, userContextManager, recorderApi)
}

let getInternalContextStrategy: StartRumResult['getInternalContext'] = () => undefined
let getInitConfigurationStrategy = (): InitConfiguration | undefined => undefined
let stopSessionStrategy: () => void = noop
Expand All @@ -109,16 +113,10 @@ export function makeRumPublicApi(
let startViewStrategy: StartRumResult['startView'] = (options, startClocks = clocksNow()) => {
bufferApiCalls.add(() => startViewStrategy(options, startClocks))
}
let addActionStrategy: StartRumResult['addAction'] = (
action,
commonContext = buildCommonContext(globalContextManager, userContextManager, recorderApi)
) => {
let addActionStrategy: StartRumResult['addAction'] = (action, commonContext = getCommonContext()) => {
bufferApiCalls.add(() => addActionStrategy(action, commonContext))
}
let addErrorStrategy: StartRumResult['addError'] = (
providedError,
commonContext = buildCommonContext(globalContextManager, userContextManager, recorderApi)
) => {
let addErrorStrategy: StartRumResult['addError'] = (providedError, commonContext = getCommonContext()) => {
bufferApiCalls.add(() => addErrorStrategy(providedError, commonContext))
}

Expand Down Expand Up @@ -238,8 +236,7 @@ export function makeRumPublicApi(
configuration,
recorderApi,
customerDataTrackerManager,
globalContextManager,
userContextManager,
getCommonContext,
initialViewOptions,
deflateWorker && createDeflateEncoder
? (streamId) => createDeflateEncoder(configuration, deflateWorker!, streamId)
Expand Down
5 changes: 1 addition & 4 deletions packages/rum-core/src/boot/startRum.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Observable, RawError, Duration, RelativeTime } from '@datadog/browser-core'
import {
createContextManager,
stopSessionManager,
toServerDuration,
ONE_SECOND,
Expand All @@ -10,7 +9,6 @@ import {
relativeNow,
createIdentityEncoder,
createCustomerDataTracker,
CustomerDataType,
} from '@datadog/browser-core'
import {
createNewEvent,
Expand Down Expand Up @@ -334,8 +332,7 @@ describe('view events', () => {
configuration,
noopRecorderApi,
customerDataTrackerManager,
createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext)),
createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User)),
() => ({ user: {}, context: {}, hasReplay: undefined }),
undefined,
createIdentityEncoder
)
Expand Down
11 changes: 4 additions & 7 deletions packages/rum-core/src/boot/startRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type {
Observable,
TelemetryEvent,
RawError,
ContextManager,
DeflateEncoderStreamId,
Encoder,
CustomerDataTrackerManager,
Expand Down Expand Up @@ -44,7 +43,6 @@ import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext'
import { startCustomerDataTelemetry } from '../domain/startCustomerDataTelemetry'
import { startPageStateHistory } from '../domain/contexts/pageStateHistory'
import type { CommonContext } from '../domain/contexts/commonContext'
import { buildCommonContext } from '../domain/contexts/commonContext'
import { startDisplayContext } from '../domain/contexts/displayContext'
import type { RecorderApi } from './rumPublicApi'

Expand All @@ -53,8 +51,7 @@ export function startRum(
configuration: RumConfiguration,
recorderApi: RecorderApi,
customerDataTrackerManager: CustomerDataTrackerManager,
globalContextManager: ContextManager,
userContextManager: ContextManager,
getCommonContext: () => CommonContext,
initialViewOptions: ViewOptions | undefined,
createEncoder: (streamId: DeflateEncoderStreamId) => Encoder
) {
Expand Down Expand Up @@ -128,7 +125,7 @@ export function startRum(
session,
locationChangeObservable,
domMutationObservable,
() => buildCommonContext(globalContextManager, userContextManager, recorderApi),
getCommonContext,
reportError
)
cleanupTasks.push(stopRumEventCollection)
Expand Down Expand Up @@ -201,7 +198,7 @@ export function startRumEventCollection(
sessionManager: RumSessionManager,
locationChangeObservable: Observable<LocationChange>,
domMutationObservable: Observable<void>,
buildCommonContext: () => CommonContext,
getCommonContext: () => CommonContext,
reportError: (error: RawError) => void
) {
const viewContexts = startViewContexts(lifeCycle)
Expand All @@ -226,7 +223,7 @@ export function startRumEventCollection(
urlContexts,
actionContexts,
displayContext,
buildCommonContext,
getCommonContext,
reportError
)

Expand Down
4 changes: 2 additions & 2 deletions packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function startRumAssembly(
urlContexts: UrlContexts,
actionContexts: ActionContexts,
displayContext: DisplayContext,
buildCommonContext: () => CommonContext,
getCommonContext: () => CommonContext,
reportError: (error: RawError) => void
) {
modifiableFieldPathsByEvent = {
Expand Down Expand Up @@ -124,7 +124,7 @@ export function startRumAssembly(
const urlContext = urlContexts.findUrl(startTime)
const session = sessionManager.findTrackedSession(startTime)
if (session && viewContext && urlContext) {
const commonContext = savedCommonContext || buildCommonContext()
const commonContext = savedCommonContext || getCommonContext()
const actionId = actionContexts.findActionId(startTime)

const rumContext: RumContext = {
Expand Down

0 comments on commit 37fd6d9

Please sign in to comment.