Skip to content

Commit

Permalink
👌 simplify encoding creation condition
Browse files Browse the repository at this point in the history
  • Loading branch information
BenoitZugmeyer committed Oct 30, 2023
1 parent 95c68f8 commit 8bbf305
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 48 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export {
FlushReason,
} from './transport'
export * from './tools/display'
export { Encoder, EncoderResult } from './tools/encoder'
export { Encoder, EncoderResult, createIdentityEncoder } from './tools/encoder'
export * from './tools/utils/urlPolyfill'
export * from './tools/utils/timeUtils'
export * from './tools/utils/arrayUtils'
Expand Down
35 changes: 20 additions & 15 deletions packages/core/src/transport/startBatchWithReplica.spec.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,37 @@
import { stubEndpointBuilder } from '../../test'
import type { PageExitEvent } from '../browser/pageExitObservable'
import type { Configuration, EndpointBuilder } from '../domain/configuration'
import type { Configuration } from '../domain/configuration'
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 { BatchConfiguration } from './startBatchWithReplica'
import { startBatchWithReplica } from './startBatchWithReplica'

describe('startBatchWithReplica', () => {
const DEFAULT_CONFIGURATION: Configuration = {} as Configuration
const reportError: (error: RawError) => void = noop
let pageExitObservable: Observable<PageExitEvent>
let sessionExpireObservable: Observable<void>
let endpoint: EndpointBuilder
let batchConfiguration: BatchConfiguration

beforeEach(() => {
pageExitObservable = new Observable()
sessionExpireObservable = new Observable()
endpoint = stubEndpointBuilder('https://example.com')
batchConfiguration = {
endpoint: stubEndpointBuilder('https://example.com'),
encoder: createIdentityEncoder(),
}
})

it('adds a message to a batch and its replica', () => {
const batchAddSpy = spyOn(Batch.prototype, 'add')

const batch = startBatchWithReplica(
DEFAULT_CONFIGURATION,
{ endpoint },
{ endpoint },
batchConfiguration,
batchConfiguration,
reportError,
pageExitObservable,
sessionExpireObservable
Expand All @@ -43,7 +48,7 @@ describe('startBatchWithReplica', () => {

const batch = startBatchWithReplica(
DEFAULT_CONFIGURATION,
{ endpoint },
batchConfiguration,
undefined,
reportError,
pageExitObservable,
Expand All @@ -58,8 +63,8 @@ describe('startBatchWithReplica', () => {

const batch = startBatchWithReplica(
DEFAULT_CONFIGURATION,
{ endpoint },
{ endpoint },
batchConfiguration,
batchConfiguration,
reportError,
pageExitObservable,
sessionExpireObservable
Expand All @@ -73,8 +78,8 @@ describe('startBatchWithReplica', () => {

const batch = startBatchWithReplica(
DEFAULT_CONFIGURATION,
{ endpoint },
{ endpoint },
batchConfiguration,
batchConfiguration,
reportError,
pageExitObservable,
sessionExpireObservable
Expand All @@ -91,9 +96,9 @@ describe('startBatchWithReplica', () => {

const batch = startBatchWithReplica(
DEFAULT_CONFIGURATION,
{ endpoint },
batchConfiguration,
{
endpoint,
...batchConfiguration,
transformMessage: (message) => ({ ...message, bar: true }),
},
reportError,
Expand All @@ -108,11 +113,11 @@ describe('startBatchWithReplica', () => {
it('transforms a message when upserting it to the replica', () => {
const batchUpsertSpy = spyOn(Batch.prototype, 'upsert')

const batch = startBatchWithReplica(
const batch = startBatchWithReplica<{ foo?: boolean; bar?: boolean }>(
DEFAULT_CONFIGURATION,
{ endpoint },
batchConfiguration,
{
endpoint,
...batchConfiguration,
transformMessage: (message) => ({ ...message, bar: true }),
},
reportError,
Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/transport/startBatchWithReplica.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ 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 { createIdentityEncoder } from '../tools/encoder'
import { Batch } from './batch'
import { createHttpRequest } from './httpRequest'
import { createFlushController } from './flushController'

interface BatchConfiguration {
export interface BatchConfiguration {
endpoint: EndpointBuilder
encoder?: Encoder
encoder: Encoder
}

interface ReplicaBatchConfiguration<T> extends BatchConfiguration {
Expand All @@ -31,7 +30,7 @@ export function startBatchWithReplica<T extends Context>(

function createBatch(configuration: Configuration, { endpoint, encoder }: BatchConfiguration) {
return new Batch(
encoder || createIdentityEncoder(),
encoder,
createHttpRequest(configuration, endpoint, configuration.batchBytesLimit, reportError),
createFlushController({
messagesLimit: configuration.batchMessagesLimit,
Expand Down
3 changes: 3 additions & 0 deletions packages/logs/src/boot/startLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ErrorSource,
addTelemetryConfiguration,
addTelemetryDebug,
createIdentityEncoder,
} from '@datadog/browser-core'
import { startLogsSessionManager, startLogsSessionManagerStub } from '../domain/logsSessionManager'
import type { LogsConfiguration, LogsInitConfiguration } from '../domain/configuration'
Expand Down Expand Up @@ -129,9 +130,11 @@ function startLogsTelemetry(
configuration,
{
endpoint: configuration.rumEndpointBuilder,
encoder: createIdentityEncoder(),
},
configuration.replica && {
endpoint: configuration.replica.rumEndpointBuilder,
encoder: createIdentityEncoder(),
},
reportError,
pageExitObservable,
Expand Down
4 changes: 3 additions & 1 deletion packages/logs/src/transport/startLogsBatch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Context, Observable, PageExitEvent, RawError } from '@datadog/browser-core'
import { startBatchWithReplica } from '@datadog/browser-core'
import { createIdentityEncoder, startBatchWithReplica } from '@datadog/browser-core'
import type { LogsConfiguration } from '../domain/configuration'
import type { LifeCycle } from '../domain/lifeCycle'
import { LifeCycleEventType } from '../domain/lifeCycle'
Expand All @@ -16,9 +16,11 @@ export function startLogsBatch(
configuration,
{
endpoint: configuration.logsEndpointBuilder,
encoder: createIdentityEncoder(),
},
configuration.replica && {
endpoint: configuration.replica.logsEndpointBuilder,
encoder: createIdentityEncoder(),
},
reportError,
pageExitObservable,
Expand Down
5 changes: 3 additions & 2 deletions packages/rum-core/src/boot/rumPublicApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
noop,
resetExperimentalFeatures,
ExperimentalFeature,
createIdentityEncoder,
} from '@datadog/browser-core'
import {
initEventBridgeStub,
Expand Down Expand Up @@ -199,7 +200,7 @@ describe('rum public api', () => {
rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)

expect(startDeflateWorkerSpy).not.toHaveBeenCalled()
expect(startRumSpy.calls.mostRecent().args[6]).toBeUndefined()
expect(startRumSpy.calls.mostRecent().args[6]).toBe(createIdentityEncoder)
})
})

Expand All @@ -211,7 +212,7 @@ describe('rum public api', () => {
})

expect(startDeflateWorkerSpy).toHaveBeenCalledTimes(1)
expect(startRumSpy.calls.mostRecent().args[6]).toEqual(jasmine.any(Function))
expect(startRumSpy.calls.mostRecent().args[6]).not.toBe(createIdentityEncoder)
})

it('aborts the initialization if it fails to create a deflate worker', () => {
Expand Down
22 changes: 17 additions & 5 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
combine,
isExperimentalFeatureEnabled,
ExperimentalFeature,
createIdentityEncoder,
} from '@datadog/browser-core'
import type { LifeCycle } from '../domain/lifeCycle'
import type { ViewContexts } from '../domain/contexts/viewContexts'
Expand Down Expand Up @@ -69,7 +70,11 @@ export interface RecorderApi {
}
interface RumPublicApiOptions {
ignoreInitIfSyntheticsWillInjectRum?: boolean
startDeflateWorker?: (configuration: RumConfiguration, source: string) => DeflateWorker | undefined
startDeflateWorker?: (
configuration: RumConfiguration,
source: string,
onInitializationFailure: () => void
) => DeflateWorker | undefined
createDeflateEncoder?: (
configuration: RumConfiguration,
worker: DeflateWorker,
Expand Down Expand Up @@ -168,7 +173,14 @@ export function makeRumPublicApi(
!eventBridgeAvailable &&
startDeflateWorker
) {
deflateWorker = startDeflateWorker(configuration, 'Datadog RUM')
deflateWorker = startDeflateWorker(
configuration,
'Datadog RUM',
// Worker initialization can fail asynchronously, especially in Firefox where even CSP
// issues are reported asynchronously. For now, the SDK will continue its execution even if
// data won't be sent to Datadog. We could improve this behavior in the future.
noop
)
if (!deflateWorker) {
return
}
Expand Down Expand Up @@ -214,9 +226,9 @@ export function makeRumPublicApi(
globalContextManager,
userContextManager,
initialViewOptions,
deflateWorker &&
createDeflateEncoder &&
((streamId) => createDeflateEncoder(configuration, deflateWorker!, streamId))
deflateWorker && createDeflateEncoder
? (streamId) => createDeflateEncoder(configuration, deflateWorker!, streamId)
: createIdentityEncoder
)
getSessionReplayLinkStrategy = () =>
recorderApi.getSessionReplayLink(configuration, startRumResults.session, startRumResults.viewContexts)
Expand Down
5 changes: 4 additions & 1 deletion packages/rum-core/src/boot/startRum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
noop,
isIE,
relativeNow,
createIdentityEncoder,
} from '@datadog/browser-core'
import {
createNewEvent,
Expand Down Expand Up @@ -332,7 +333,9 @@ describe('view events', () => {
configuration,
noopRecorderApi,
createContextManager(CustomerDataType.GlobalContext),
createContextManager(CustomerDataType.User)
createContextManager(CustomerDataType.User),
undefined,
createIdentityEncoder
)
)
interceptor = interceptRequests()
Expand Down
8 changes: 4 additions & 4 deletions packages/rum-core/src/boot/startRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {
RawError,
ContextManager,
DeflateEncoderStreamId,
DeflateEncoder,
Encoder,
} from '@datadog/browser-core'
import {
sendToExtension,
Expand Down Expand Up @@ -52,8 +52,8 @@ export function startRum(
recorderApi: RecorderApi,
globalContextManager: ContextManager,
userContextManager: ContextManager,
initialViewOptions?: ViewOptions,
createDeflateEncoder?: (streamId: DeflateEncoderStreamId) => DeflateEncoder
initialViewOptions: ViewOptions | undefined,
createEncoder: (streamId: DeflateEncoderStreamId) => Encoder
) {
const cleanupTasks: Array<() => void> = []
const lifeCycle = new LifeCycle()
Expand Down Expand Up @@ -97,7 +97,7 @@ export function startRum(
reportError,
pageExitObservable,
session.expireObservable,
createDeflateEncoder
createEncoder
)
cleanupTasks.push(() => batch.stop())
startCustomerDataTelemetry(
Expand Down
15 changes: 4 additions & 11 deletions packages/rum-core/src/transport/startRumBatch.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import type {
Context,
TelemetryEvent,
Observable,
RawError,
PageExitEvent,
DeflateEncoder,
} from '@datadog/browser-core'
import type { Context, TelemetryEvent, Observable, RawError, PageExitEvent, Encoder } from '@datadog/browser-core'
import {
DeflateEncoderStreamId,
combine,
Expand All @@ -25,20 +18,20 @@ export function startRumBatch(
reportError: (error: RawError) => void,
pageExitObservable: Observable<PageExitEvent>,
sessionExpireObservable: Observable<void>,
createDeflateEncoder?: (streamId: DeflateEncoderStreamId) => DeflateEncoder
createEncoder: (streamId: DeflateEncoderStreamId) => Encoder
) {
const replica = configuration.replica

const batch = startBatchWithReplica(
configuration,
{
endpoint: configuration.rumEndpointBuilder,
encoder: createDeflateEncoder && createDeflateEncoder(DeflateEncoderStreamId.RUM),
encoder: createEncoder(DeflateEncoderStreamId.RUM),
},
replica && {
endpoint: replica.rumEndpointBuilder,
transformMessage: (message) => combine(message, { application: { id: replica.applicationId } }),
encoder: createDeflateEncoder && createDeflateEncoder(DeflateEncoderStreamId.RUM_REPLICA),
encoder: createEncoder(DeflateEncoderStreamId.RUM_REPLICA),
},
reportError,
pageExitObservable,
Expand Down
6 changes: 2 additions & 4 deletions packages/rum/src/domain/deflate/deflateWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ let state: DeflateWorkerState = { status: DeflateWorkerStatus.Nil }
export function startDeflateWorker(
configuration: RumConfiguration,
source: string,
onInitializationFailure?: () => void,
onInitializationFailure: () => void,
createDeflateWorkerImpl = createDeflateWorker
) {
if (state.status === DeflateWorkerStatus.Nil) {
Expand All @@ -59,9 +59,7 @@ export function startDeflateWorker(

switch (state.status) {
case DeflateWorkerStatus.Loading:
if (onInitializationFailure) {
state.initializationFailureCallbacks.push(onInitializationFailure)
}
state.initializationFailureCallbacks.push(onInitializationFailure)
return state.worker
case DeflateWorkerStatus.Initialized:
return state.worker
Expand Down

0 comments on commit 8bbf305

Please sign in to comment.