Skip to content

Commit

Permalink
👌 re-use TrackingConsentState instance
Browse files Browse the repository at this point in the history
  • Loading branch information
BenoitZugmeyer committed Feb 7, 2024
1 parent 3e1b702 commit 2bfb326
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 70 deletions.
9 changes: 5 additions & 4 deletions packages/logs/src/boot/logsPublicApi.ts
Expand Up @@ -12,6 +12,7 @@ import {
storeContextManager,
displayAlreadyInitializedError,
deepClone,
createTrackingConsentState,
} from '@datadog/browser-core'
import type { LogsInitConfiguration } from '../domain/configuration'
import type { HandlerType, StatusType } from '../domain/logger'
Expand All @@ -32,7 +33,6 @@ const LOGS_STORAGE_KEY = 'logs'

export interface Strategy {
init: (initConfiguration: LogsInitConfiguration) => void
setTrackingConsent: StartLogsResult['setTrackingConsent']
initConfiguration: LogsInitConfiguration | undefined
getInternalContext: StartLogsResult['getInternalContext']
handleLog: StartLogsResult['handleLog']
Expand All @@ -44,18 +44,19 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext)
)
const userContextManager = createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User))
const trackingConsentState = createTrackingConsentState()

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

let strategy = createPreStartStrategy(getCommonContext, (initConfiguration, configuration) => {
let strategy = createPreStartStrategy(getCommonContext, trackingConsentState, (initConfiguration, configuration) => {
if (initConfiguration.storeContextsAcrossPages) {
storeContextManager(configuration, globalContextManager, LOGS_STORAGE_KEY, CustomerDataType.GlobalContext)
storeContextManager(configuration, userContextManager, LOGS_STORAGE_KEY, CustomerDataType.User)
}

const startLogsResult = startLogsImpl(initConfiguration, configuration, getCommonContext)
const startLogsResult = startLogsImpl(initConfiguration, configuration, getCommonContext, trackingConsentState)

strategy = createPostStartStrategy(initConfiguration, startLogsResult)
return startLogsResult
Expand Down Expand Up @@ -85,7 +86,7 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
* If this method is called before the init() method, the provided value will take precedence
* over the one provided as initialization parameter.
*/
setTrackingConsent: monitor((trackingConsent: TrackingConsent) => strategy.setTrackingConsent(trackingConsent)),
setTrackingConsent: monitor((trackingConsent: TrackingConsent) => trackingConsentState.update(trackingConsent)),

getGlobalContext: monitor(() => globalContextManager.getContext()),

Expand Down
33 changes: 21 additions & 12 deletions packages/logs/src/boot/preStartLogs.spec.ts
Expand Up @@ -5,8 +5,14 @@ import {
initEventBridgeStub,
mockExperimentalFeatures,
} from '@datadog/browser-core/test'
import type { TimeStamp } from '@datadog/browser-core'
import { ExperimentalFeature, ONE_SECOND, TrackingConsent, display } from '@datadog/browser-core'
import type { TimeStamp, TrackingConsentState } from '@datadog/browser-core'
import {
ExperimentalFeature,
ONE_SECOND,
TrackingConsent,
createTrackingConsentState,
display,
} from '@datadog/browser-core'
import type { CommonContext } from '../rawLogsEvent.types'
import type { HybridInitConfiguration, LogsConfiguration, LogsInitConfiguration } from '../domain/configuration'
import { StatusType, type Logger } from '../domain/logger'
Expand Down Expand Up @@ -37,7 +43,7 @@ describe('preStartLogs', () => {
handleLog: handleLogSpy,
} as unknown as StartLogsResult)
getCommonContextSpy = jasmine.createSpy()
strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
strategy = createPreStartStrategy(getCommonContextSpy, createTrackingConsentState(), doStartLogsSpy)
clock = mockClock()
})

Expand Down Expand Up @@ -203,19 +209,26 @@ describe('preStartLogs', () => {

describe('internal context', () => {
it('should return undefined if not initialized', () => {
const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
const strategy = createPreStartStrategy(getCommonContextSpy, createTrackingConsentState(), doStartLogsSpy)
expect(strategy.getInternalContext()).toBeUndefined()
})
})

describe('tracking consent', () => {
let strategy: Strategy
let trackingConsentState: TrackingConsentState

beforeEach(() => {
trackingConsentState = createTrackingConsentState()
strategy = createPreStartStrategy(getCommonContextSpy, trackingConsentState, doStartLogsSpy)
})

describe('with tracking_consent enabled', () => {
beforeEach(() => {
mockExperimentalFeatures([ExperimentalFeature.TRACKING_CONSENT])
})

it('does not start logs if tracking consent is not granted at init', () => {
const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
strategy.init({
...DEFAULT_INIT_CONFIGURATION,
trackingConsent: TrackingConsent.NOT_GRANTED,
Expand All @@ -224,8 +237,7 @@ describe('preStartLogs', () => {
})

it('starts logs if tracking consent is granted before init', () => {
const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
strategy.setTrackingConsent(TrackingConsent.GRANTED)
trackingConsentState.update(TrackingConsent.GRANTED)
strategy.init({
...DEFAULT_INIT_CONFIGURATION,
trackingConsent: TrackingConsent.NOT_GRANTED,
Expand All @@ -234,8 +246,7 @@ describe('preStartLogs', () => {
})

it('does not start logs if tracking consent is not withdrawn before init', () => {
const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
strategy.setTrackingConsent(TrackingConsent.NOT_GRANTED)
trackingConsentState.update(TrackingConsent.NOT_GRANTED)
strategy.init({
...DEFAULT_INIT_CONFIGURATION,
trackingConsent: TrackingConsent.GRANTED,
Expand All @@ -246,7 +257,6 @@ describe('preStartLogs', () => {

describe('with tracking_consent disabled', () => {
it('ignores the trackingConsent init param', () => {
const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
strategy.init({
...DEFAULT_INIT_CONFIGURATION,
trackingConsent: TrackingConsent.NOT_GRANTED,
Expand All @@ -255,8 +265,7 @@ describe('preStartLogs', () => {
})

it('ignores setTrackingConsent', () => {
const strategy = createPreStartStrategy(getCommonContextSpy, doStartLogsSpy)
strategy.setTrackingConsent(TrackingConsent.NOT_GRANTED)
trackingConsentState.update(TrackingConsent.NOT_GRANTED)
strategy.init(DEFAULT_INIT_CONFIGURATION)
expect(doStartLogsSpy).toHaveBeenCalledTimes(1)
})
Expand Down
9 changes: 4 additions & 5 deletions packages/logs/src/boot/preStartLogs.ts
@@ -1,8 +1,8 @@
import type { TrackingConsentState } from '@datadog/browser-core'
import {
BoundedBuffer,
assign,
canUseEventBridge,
createTrackingConsentState,
display,
displayAlreadyInitializedError,
noop,
Expand All @@ -19,19 +19,20 @@ import type { StartLogsResult } from './startLogs'

export function createPreStartStrategy(
getCommonContext: () => CommonContext,
trackingConsentState: TrackingConsentState,
doStartLogs: (initConfiguration: LogsInitConfiguration, configuration: LogsConfiguration) => StartLogsResult
): Strategy {
const bufferApiCalls = new BoundedBuffer<StartLogsResult>()
let cachedInitConfiguration: LogsInitConfiguration | undefined
let cachedConfiguration: LogsConfiguration | undefined
const trackingConsentState = createTrackingConsentState()
trackingConsentState.observable.subscribe(tryStartLogs)
const trackingConsentStateSubscription = trackingConsentState.observable.subscribe(tryStartLogs)

function tryStartLogs() {
if (!cachedConfiguration || !cachedInitConfiguration || !trackingConsentState.isGranted()) {
return
}

trackingConsentStateSubscription.unsubscribe()
const startLogsResult = doStartLogs(cachedInitConfiguration, cachedConfiguration)

bufferApiCalls.drain(startLogsResult)
Expand Down Expand Up @@ -66,8 +67,6 @@ export function createPreStartStrategy(
tryStartLogs()
},

setTrackingConsent: trackingConsentState.update,

get initConfiguration() {
return cachedInitConfiguration
},
Expand Down
57 changes: 48 additions & 9 deletions packages/logs/src/boot/startLogs.spec.ts
Expand Up @@ -7,6 +7,8 @@ import {
SESSION_STORE_KEY,
createCustomerDataTracker,
noop,
createTrackingConsentState,
TrackingConsent,
} from '@datadog/browser-core'
import type { Request } from '@datadog/browser-core/test'
import {
Expand Down Expand Up @@ -81,7 +83,12 @@ describe('logs', () => {

describe('request', () => {
it('should send the needed data', () => {
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
baseConfiguration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

handleLog({ message: 'message', status: StatusType.warn, context: { foo: 'bar' } }, logger, COMMON_CONTEXT)
Expand All @@ -107,7 +114,8 @@ describe('logs', () => {
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
{ ...baseConfiguration, batchMessagesLimit: 3 },
() => COMMON_CONTEXT
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

Expand All @@ -120,7 +128,12 @@ describe('logs', () => {

it('should send bridge event when bridge is present', () => {
const sendSpy = spyOn(initEventBridgeStub(), 'send')
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
baseConfiguration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

handleLog(DEFAULT_MESSAGE, logger)
Expand All @@ -140,14 +153,24 @@ describe('logs', () => {
const sendSpy = spyOn(initEventBridgeStub(), 'send')

let configuration = { ...baseConfiguration, sessionSampleRate: 0 }
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, configuration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
configuration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)
handleLog(DEFAULT_MESSAGE, logger)

expect(sendSpy).not.toHaveBeenCalled()

configuration = { ...baseConfiguration, sessionSampleRate: 100 }
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, configuration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
configuration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)
handleLog(DEFAULT_MESSAGE, logger)

Expand All @@ -160,7 +183,8 @@ describe('logs', () => {
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
{ ...baseConfiguration, forwardConsoleLogs: ['log'] },
() => COMMON_CONTEXT
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

Expand All @@ -177,23 +201,38 @@ describe('logs', () => {
})

it('creates a session on normal conditions', () => {
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
baseConfiguration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

expect(getCookie(SESSION_STORE_KEY)).not.toBeUndefined()
})

it('does not create a session if event bridge is present', () => {
initEventBridgeStub()
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
baseConfiguration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

expect(getCookie(SESSION_STORE_KEY)).toBeUndefined()
})

it('does not create a session if synthetics worker will inject RUM', () => {
mockSyntheticsWorkerValues({ injectsRum: true })
;({ handleLog, stop: stopLogs } = startLogs(initConfiguration, baseConfiguration, () => COMMON_CONTEXT))
;({ handleLog, stop: stopLogs } = startLogs(
initConfiguration,
baseConfiguration,
() => COMMON_CONTEXT,
createTrackingConsentState(TrackingConsent.GRANTED)
))
registerCleanupTask(stopLogs)

expect(getCookie(SESSION_STORE_KEY)).toBeUndefined()
Expand Down
10 changes: 5 additions & 5 deletions packages/logs/src/boot/startLogs.ts
@@ -1,10 +1,9 @@
import type { TrackingConsentState } from '@datadog/browser-core'
import {
TrackingConsent,
sendToExtension,
createPageExitObservable,
willSyntheticsInjectRum,
canUseEventBridge,
createTrackingConsentState,
} from '@datadog/browser-core'
import { startLogsSessionManager, startLogsSessionManagerStub } from '../domain/logsSessionManager'
import type { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration'
Expand All @@ -28,7 +27,10 @@ export type StartLogsResult = ReturnType<StartLogs>
export function startLogs(
initConfiguration: LogsInitConfiguration,
configuration: LogsConfiguration,
getCommonContext: () => CommonContext
getCommonContext: () => CommonContext,

// Tracking consent should always be granted when the startLogs is called.
trackingConsentState: TrackingConsentState
) {
const lifeCycle = new LifeCycle()
const cleanupTasks: Array<() => void> = []
Expand All @@ -37,7 +39,6 @@ export function startLogs(

const reportError = startReportError(lifeCycle)
const pageExitObservable = createPageExitObservable(configuration)
const trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED)

const session =
configuration.sessionStoreStrategyType && !canUseEventBridge() && !willSyntheticsInjectRum()
Expand Down Expand Up @@ -73,7 +74,6 @@ export function startLogs(
return {
handleLog,
getInternalContext: internalContext.get,
setTrackingConsent: trackingConsentState.update,
stop: () => {
cleanupTasks.forEach((task) => task())
},
Expand Down

0 comments on commit 2bfb326

Please sign in to comment.