Skip to content

Commit

Permalink
✨ [RUMF-1070] forward logs event to bridge (#1155)
Browse files Browse the repository at this point in the history
* Event bridge logs configuration

* ✨ Send logs to the bridge

* 🎨 Rename batch per startRumBatch for consistency
  • Loading branch information
amortemousque authored Dec 6, 2021
1 parent edad30f commit ca94e24
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 70 deletions.
50 changes: 36 additions & 14 deletions packages/logs/src/boot/logs.entry.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { Context, monitor, ONE_SECOND, display } from '@datadog/browser-core'
import { Clock, mockClock } from '../../../core/test/specHelper'
import {
Context,
monitor,
ONE_SECOND,
display,
updateExperimentalFeatures,
resetExperimentalFeatures,
} from '@datadog/browser-core'
import { LogsInitConfiguration } from '..'
import { Clock, deleteEventBridgeStub, initEventBridgeStub, mockClock } from '../../../core/test/specHelper'

import { HandlerType, LogsMessage, StatusType } from '../domain/logger'
import { LogsPublicApi, makeLogsPublicApi, StartLogs } from './logs.entry'
import { HybridInitConfiguration, LogsPublicApi, makeLogsPublicApi, StartLogs } from './logs.entry'

const DEFAULT_INIT_CONFIGURATION = { clientToken: 'xxx' }

Expand Down Expand Up @@ -32,19 +40,19 @@ describe('logs entry', () => {

describe('configuration validation', () => {
let LOGS: LogsPublicApi
let displaySpy: jasmine.Spy

beforeEach(() => {
displaySpy = spyOn(display, 'error')
LOGS = makeLogsPublicApi(startLogs)
})

it('init should log an error with no public api key', () => {
const displaySpy = spyOn(display, 'error')

LOGS.init(undefined as any)
expect(display.error).toHaveBeenCalledTimes(1)
expect(displaySpy).toHaveBeenCalledTimes(1)

LOGS.init({ stillNoApiKey: true } as any)
expect(display.error).toHaveBeenCalledTimes(2)
expect(displaySpy).toHaveBeenCalledTimes(2)

LOGS.init({ clientToken: 'yeah' })
expect(displaySpy).toHaveBeenCalledTimes(2)
Expand All @@ -54,23 +62,21 @@ describe('logs entry', () => {
const setDebug: (debug: boolean) => void = (LOGS as any)._setDebug
expect(!!setDebug).toEqual(true)

spyOn(display, 'error')
monitor(() => {
throw new Error()
})()
expect(display.error).toHaveBeenCalledTimes(0)
expect(displaySpy).toHaveBeenCalledTimes(0)

setDebug(true)
monitor(() => {
throw new Error()
})()
expect(display.error).toHaveBeenCalledTimes(1)
expect(displaySpy).toHaveBeenCalledTimes(1)

setDebug(false)
})

it('init should log an error if sampleRate is invalid', () => {
const displaySpy = spyOn(display, 'error')
LOGS.init({ clientToken: 'yes', sampleRate: 'foo' as any })
expect(displaySpy).toHaveBeenCalledTimes(1)

Expand All @@ -79,7 +85,6 @@ describe('logs entry', () => {
})

it('should log an error if init is called several times', () => {
const displaySpy = spyOn(display, 'error')
LOGS.init({ clientToken: 'yes', sampleRate: 1 })
expect(displaySpy).toHaveBeenCalledTimes(0)

Expand All @@ -88,7 +93,6 @@ describe('logs entry', () => {
})

it('should not log an error if init is called several times and silentMultipleInit is true', () => {
const displaySpy = spyOn(display, 'error')
LOGS.init({
clientToken: 'yes',
sampleRate: 1,
Expand All @@ -105,10 +109,28 @@ describe('logs entry', () => {
})

it("shouldn't trigger any console.error if the configuration is correct", () => {
const displaySpy = spyOn(display, 'error')
LOGS.init({ clientToken: 'yes', sampleRate: 1 })
expect(displaySpy).toHaveBeenCalledTimes(0)
})

describe('if event bridge present', () => {
beforeEach(() => {
updateExperimentalFeatures(['event-bridge'])
initEventBridgeStub()
})

afterEach(() => {
resetExperimentalFeatures()
deleteEventBridgeStub()
})

it('init should accept empty client token', () => {
const hybridInitConfiguration: HybridInitConfiguration = {}
LOGS.init(hybridInitConfiguration as LogsInitConfiguration)

expect(displaySpy).not.toHaveBeenCalled()
})
})
})

describe('pre-init API usages', () => {
Expand Down
11 changes: 11 additions & 0 deletions packages/logs/src/boot/logs.entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
display,
deepClone,
InitConfiguration,
canUseEventBridge,
} from '@datadog/browser-core'
import { HandlerType, Logger, LogsMessage, StatusType } from '../domain/logger'
import { startLogs, LogsInitConfiguration } from './startLogs'
Expand All @@ -21,6 +22,8 @@ export interface LoggerConfiguration {
context?: object
}

export type HybridInitConfiguration = Omit<LogsInitConfiguration, 'clientToken'>

export type LogsPublicApi = ReturnType<typeof makeLogsPublicApi>

export const datadogLogs = makeLogsPublicApi(startLogs)
Expand Down Expand Up @@ -49,6 +52,10 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
logger,

init: monitor((initConfiguration: LogsInitConfiguration) => {
if (canUseEventBridge()) {
initConfiguration = overrideInitConfigurationForBridge(initConfiguration)
}

if (!canInitLogs(initConfiguration)) {
return
}
Expand Down Expand Up @@ -80,6 +87,10 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs) {
getInitConfiguration: monitor(() => getInitConfigurationStrategy()),
})

function overrideInitConfigurationForBridge<C extends InitConfiguration>(initConfiguration: C): C {
return { ...initConfiguration, clientToken: 'empty' }
}

function canInitLogs(initConfiguration: LogsInitConfiguration) {
if (isAlreadyInitialized) {
if (!initConfiguration.silentMultipleInit) {
Expand Down
44 changes: 42 additions & 2 deletions packages/logs/src/boot/startLogs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,22 @@ import {
ONE_MINUTE,
RawError,
RelativeTime,
resetExperimentalFeatures,
TimeStamp,
updateExperimentalFeatures,
} from '@datadog/browser-core'
import sinon from 'sinon'
import { Clock, mockClock, stubEndpointBuilder } from '../../../core/test/specHelper'
import {
Clock,
deleteEventBridgeStub,
initEventBridgeStub,
mockClock,
stubEndpointBuilder,
} from '../../../core/test/specHelper'

import { Logger, LogsMessage, StatusType } from '../domain/logger'
import { LogsEvent } from '../logsEvent.types'
import { buildAssemble, doStartLogs } from './startLogs'
import { buildAssemble, doStartLogs, LogsInitConfiguration, startLogs as originalStartLogs } from './startLogs'

interface SentMessage extends LogsMessage {
logger?: { name: string }
Expand Down Expand Up @@ -75,6 +83,8 @@ describe('logs', () => {
afterEach(() => {
server.restore()
delete window.DD_RUM
resetExperimentalFeatures()
deleteEventBridgeStub()
})

describe('request', () => {
Expand Down Expand Up @@ -127,6 +137,36 @@ describe('logs', () => {

expect(server.requests.length).toEqual(1)
})

it('should send bridge event when bridge is present', () => {
updateExperimentalFeatures(['event-bridge'])
const sendSpy = spyOn(initEventBridgeStub(), 'send')

const sendLog = startLogs()
sendLog(DEFAULT_MESSAGE, {})

expect(server.requests.length).toEqual(0)
expect(sendSpy).toHaveBeenCalled()
})
})

describe('sampling', () => {
it('should be applied when event bridge is present', () => {
updateExperimentalFeatures(['event-bridge'])
const sendSpy = spyOn(initEventBridgeStub(), 'send')

let configuration = { ...baseConfiguration, ...{ sampleRate: 0 } } as LogsInitConfiguration
let sendLog = originalStartLogs(configuration, new Logger(noop))
sendLog(DEFAULT_MESSAGE, {})

expect(sendSpy).not.toHaveBeenCalled()

configuration = { ...baseConfiguration, ...{ sampleRate: 100 } } as LogsInitConfiguration
sendLog = originalStartLogs(configuration, new Logger(noop))
sendLog(DEFAULT_MESSAGE, {})

expect(sendSpy).toHaveBeenCalled()
})
})

describe('assemble', () => {
Expand Down
54 changes: 19 additions & 35 deletions packages/logs/src/boot/startLogs.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
import {
areCookiesAuthorized,
Batch,
combine,
commonInit,
Configuration,
Context,
createEventRateLimiter,
HttpRequest,
InternalMonitoring,
Observable,
RawError,
RelativeTime,
InitConfiguration,
trackRuntimeError,
trackConsoleError,
EndpointBuilder,
canUseEventBridge,
getEventBridge,
} from '@datadog/browser-core'
import { trackNetworkError } from '../domain/trackNetworkError'
import { Logger, LogsMessage, StatusType } from '../domain/logger'
import { LoggerSession, startLoggerSession } from '../domain/loggerSession'
import { LoggerSession, startLoggerSession, startLoggerSessionStub } from '../domain/loggerSession'
import { LogsEvent } from '../logsEvent.types'
import { startLoggerBatch } from '../transport/startLoggerBatch'
import { buildEnv } from './buildEnv'

export interface LogsInitConfiguration extends InitConfiguration {
Expand All @@ -37,7 +37,11 @@ export function startLogs(initConfiguration: LogsInitConfiguration, errorLogger:
trackNetworkError(configuration, errorObservable)
}

const session = startLoggerSession(configuration, areCookiesAuthorized(configuration.cookieOptions))
const session =
areCookiesAuthorized(configuration.cookieOptions) && !canUseEventBridge()
? startLoggerSession(configuration)
: startLoggerSessionStub(configuration)

return doStartLogs(configuration, errorObservable, internalMonitoring, session, errorLogger)
}

Expand All @@ -55,7 +59,15 @@ export function doStartLogs(
)

const assemble = buildAssemble(session, configuration, reportError)
const batch = startLoggerBatch(configuration)

let onLogEventCollected: (message: Context) => void
if (canUseEventBridge()) {
const bridge = getEventBridge()!
onLogEventCollected = (message) => bridge.send('log', message)
} else {
const batch = startLoggerBatch(configuration)
onLogEventCollected = (message) => batch.add(message)
}

function reportError(error: RawError) {
errorLogger.error(
Expand Down Expand Up @@ -87,39 +99,11 @@ export function doStartLogs(
return (message: LogsMessage, currentContext: Context) => {
const contextualizedMessage = assemble(message, currentContext)
if (contextualizedMessage) {
batch.add(contextualizedMessage)
onLogEventCollected(contextualizedMessage)
}
}
}

function startLoggerBatch(configuration: Configuration) {
const primaryBatch = createLoggerBatch(configuration.logsEndpointBuilder)

let replicaBatch: Batch | undefined
if (configuration.replica !== undefined) {
replicaBatch = createLoggerBatch(configuration.replica.logsEndpointBuilder)
}

function createLoggerBatch(endpointBuilder: EndpointBuilder) {
return new Batch(
new HttpRequest(endpointBuilder, configuration.batchBytesLimit),
configuration.maxBatchSize,
configuration.batchBytesLimit,
configuration.maxMessageSize,
configuration.flushTimeout
)
}

return {
add(message: Context) {
primaryBatch.add(message)
if (replicaBatch) {
replicaBatch.add(message)
}
},
}
}

export function buildAssemble(
session: LoggerSession,
configuration: Configuration,
Expand Down
19 changes: 11 additions & 8 deletions packages/logs/src/domain/loggerSession.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from '@datadog/browser-core'
import { Clock, mockClock } from '../../../core/test/specHelper'

import { LOGGER_SESSION_KEY, LoggerTrackingType, startLoggerSession } from './loggerSession'
import { LOGGER_SESSION_KEY, LoggerTrackingType, startLoggerSession, startLoggerSessionStub } from './loggerSession'

describe('logger session', () => {
const DURATION = 123456
Expand All @@ -32,7 +32,7 @@ describe('logger session', () => {
it('when tracked should store tracking type and session id', () => {
tracked = true

startLoggerSession(configuration as Configuration, true)
startLoggerSession(configuration as Configuration)

expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.TRACKED}`)
expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]+/)
Expand All @@ -41,7 +41,7 @@ describe('logger session', () => {
it('when not tracked should store tracking type', () => {
tracked = false

startLoggerSession(configuration as Configuration, true)
startLoggerSession(configuration as Configuration)

expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`)
expect(getCookie(SESSION_COOKIE_NAME)).not.toContain('id=')
Expand All @@ -50,7 +50,7 @@ describe('logger session', () => {
it('when tracked should keep existing tracking type and session id', () => {
setCookie(SESSION_COOKIE_NAME, 'id=abcdef&logs=1', DURATION)

startLoggerSession(configuration as Configuration, true)
startLoggerSession(configuration as Configuration)

expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.TRACKED}`)
expect(getCookie(SESSION_COOKIE_NAME)).toContain('id=abcdef')
Expand All @@ -59,13 +59,13 @@ describe('logger session', () => {
it('when not tracked should keep existing tracking type', () => {
setCookie(SESSION_COOKIE_NAME, 'logs=0', DURATION)

startLoggerSession(configuration as Configuration, true)
startLoggerSession(configuration as Configuration)

expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`)
})

it('should renew on activity after expiration', () => {
startLoggerSession(configuration as Configuration, true)
startLoggerSession(configuration as Configuration)

setCookie(SESSION_COOKIE_NAME, '', DURATION)
expect(getCookie(SESSION_COOKIE_NAME)).toBeUndefined()
Expand All @@ -77,9 +77,12 @@ describe('logger session', () => {
expect(getCookie(SESSION_COOKIE_NAME)).toMatch(/id=[a-f0-9-]+/)
expect(getCookie(SESSION_COOKIE_NAME)).toContain(`${LOGGER_SESSION_KEY}=${LoggerTrackingType.TRACKED}`)
})
})

it('when no cookies available, isTracked is computed at each call and getId is undefined', () => {
const session = startLoggerSession(configuration as Configuration, false)
describe('logger session stub', () => {
it('isTracked is computed at each call and getId is undefined', () => {
const configuration: Partial<Configuration> = { sampleRate: 0.5 }
const session = startLoggerSessionStub(configuration as Configuration)

expect(session.getId()).toBeUndefined()
expect(session.isTracked()).toMatch(/true|false/)
Expand Down
Loading

0 comments on commit ca94e24

Please sign in to comment.