Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ [RUMF-1084] ignore init if a RUM instance is or will be injected by synthetics #1170

Merged
merged 9 commits into from
Nov 22, 2021
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export * from './tools/browserDetection'
export { instrumentMethod, instrumentMethodAndCallOriginal } from './tools/instrumentMethod'
export { ErrorSource, ErrorHandling, formatUnknownError, createHandlingStack, RawError } from './tools/error'
export { Context, ContextArray, ContextValue } from './tools/context'
export { areCookiesAuthorized, getCookie, setCookie, COOKIE_ACCESS_DELAY } from './browser/cookie'
export { areCookiesAuthorized, getCookie, setCookie, deleteCookie, COOKIE_ACCESS_DELAY } from './browser/cookie'
export { initXhrObservable, XhrCompleteContext, XhrStartContext } from './browser/xhrObservable'
export { initFetchObservable, FetchCompleteContext, FetchStartContext, FetchContext } from './browser/fetchObservable'
export { EndpointBuilder } from './domain/configuration/endpointBuilder'
Expand Down
28 changes: 27 additions & 1 deletion packages/rum-core/src/boot/rumPublicApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ import {
resetExperimentalFeatures,
} from '@datadog/browser-core'
import { initEventBridgeStub, deleteEventBridgeStub } from '../../../core/test/specHelper'
import { noopRecorderApi, setup, TestSetupBuilder } from '../../test/specHelper'
import {
cleanupSyntheticsWorkerValues,
mockSyntheticsWorkerValues,
noopRecorderApi,
setup,
TestSetupBuilder,
} from '../../test/specHelper'
import { ActionType } from '../rawRumEvent.types'
import {
makeRumPublicApi,
Expand Down Expand Up @@ -143,6 +149,26 @@ describe('rum public api', () => {
})
})

describe('init', () => {
let rumPublicApi: RumPublicApi
let startRumSpy: jasmine.Spy<StartRum>

beforeEach(() => {
startRumSpy = jasmine.createSpy()
rumPublicApi = makeRumPublicApi(startRumSpy, noopRecorderApi)
})

afterEach(() => {
cleanupSyntheticsWorkerValues()
})

it('does not call startRum if Synthetics will inject its own instance of RUM', () => {
mockSyntheticsWorkerValues({ injectsRum: true })
rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
expect(startRumSpy).not.toHaveBeenCalled()
})
})

describe('getInternalContext', () => {
let getInternalContextSpy: jasmine.Spy<ReturnType<StartRum>['getInternalContext']>
let rumPublicApi: RumPublicApi
Expand Down
3 changes: 2 additions & 1 deletion packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { RumSession } from '../domain/rumSession'
import { RumEventDomainContext } from '../domainContext.types'
import { CommonContext, User, ActionType, ReplayStats } from '../rawRumEvent.types'
import { RumEvent } from '../rumEvent.types'
import { willSyntheticsInjectRum } from '../tools/syntheticsContext'
import { buildEnv } from './buildEnv'
import { startRum } from './startRum'

Expand Down Expand Up @@ -103,7 +104,7 @@ export function makeRumPublicApi<C extends RumInitConfiguration>(startRumImpl: S
return
}

if (!isValidInitConfiguration(initConfiguration)) {
if (willSyntheticsInjectRum() || !isValidInitConfiguration(initConfiguration)) {
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
return
}

Expand Down
109 changes: 15 additions & 94 deletions packages/rum-core/src/domain/assembly.spec.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
import { ErrorSource, ONE_MINUTE, RawError, RelativeTime, display, setCookie } from '@datadog/browser-core'
import { deleteCookie } from 'packages/core/src/browser/cookie'
import { createRumSessionMock } from 'packages/rum-core/test/mockRumSession'
import { ErrorSource, ONE_MINUTE, RawError, RelativeTime, display } from '@datadog/browser-core'
import { createRumSessionMock } from '../../test/mockRumSession'
import { createRawRumEvent } from '../../test/fixtures'
import { setup, TestSetupBuilder } from '../../test/specHelper'
import {
cleanupSyntheticsWorkerValues,
mockSyntheticsWorkerValues,
setup,
TestSetupBuilder,
} from '../../test/specHelper'
import { RumEventDomainContext } from '../domainContext.types'
import { CommonContext, RawRumActionEvent, RawRumErrorEvent, RawRumEvent, RumEventType } from '../rawRumEvent.types'
import { RumActionEvent, RumErrorEvent, RumEvent } from '../rumEvent.types'
import {
BrowserWindow,
startRumAssembly,
SYNTHETICS_RESULT_ID_COOKIE_NAME,
SYNTHETICS_TEST_ID_COOKIE_NAME,
} from './assembly'
import { startRumAssembly } from './assembly'
import { LifeCycle, LifeCycleEventType, RawRumEventCollectedData } from './lifeCycle'
import { RumSessionPlan } from './rumSession'

// Duration to create a cookie lasting at least until the end of the test
const COOKIE_DURATION = 1000

describe('rum assembly', () => {
let setupBuilder: TestSetupBuilder
let commonContext: CommonContext
Expand Down Expand Up @@ -65,9 +61,7 @@ describe('rum assembly', () => {

afterEach(() => {
setupBuilder.cleanup()
cleanupSyntheticsGlobals()
deleteCookie(SYNTHETICS_TEST_ID_COOKIE_NAME)
deleteCookie(SYNTHETICS_RESULT_ID_COOKIE_NAME)
cleanupSyntheticsWorkerValues()
})

describe('beforeSend', () => {
Expand Down Expand Up @@ -527,20 +521,8 @@ describe('rum assembly', () => {
})
})

it('should detect synthetics sessions from global', () => {
setSyntheticsGlobals('foo', 'bar')

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].session.type).toEqual('synthetics')
})

it('should detect synthetics sessions from cookies', () => {
setCookie(SYNTHETICS_TEST_ID_COOKIE_NAME, 'foo', COOKIE_DURATION)
setCookie(SYNTHETICS_RESULT_ID_COOKIE_NAME, 'bar', COOKIE_DURATION)
it('should detect synthetics sessions based on synthetics worker values', () => {
mockSyntheticsWorkerValues()

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
Expand Down Expand Up @@ -572,66 +554,15 @@ describe('rum assembly', () => {
})

describe('synthetics context', () => {
it('sets the synthetics context defined by global variables', () => {
setSyntheticsGlobals('foo', 'bar')

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].synthetics).toEqual({
test_id: 'foo',
result_id: 'bar',
})
})

it('sets the synthetics context defined by global cookie', () => {
setCookie(SYNTHETICS_TEST_ID_COOKIE_NAME, 'foo', COOKIE_DURATION)
setCookie(SYNTHETICS_RESULT_ID_COOKIE_NAME, 'bar', COOKIE_DURATION)

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].synthetics).toEqual({
test_id: 'foo',
result_id: 'bar',
})
})

it('does not set synthetics context if one global variable is undefined', () => {
setSyntheticsGlobals('foo')

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].synthetics).toBeUndefined()
})

it('does not set synthetics context if global variables are not strings', () => {
setSyntheticsGlobals(1, 2)
it('includes the synthetics context', () => {
mockSyntheticsWorkerValues()

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].synthetics).toBeUndefined()
})

it('does not set synthetics context if one cookie is undefined', () => {
setCookie(SYNTHETICS_TEST_ID_COOKIE_NAME, 'foo', COOKIE_DURATION)

const { lifeCycle } = setupBuilder.build()
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].synthetics).toBeUndefined()
expect(serverRumEvents[0].synthetics).toBeTruthy()
})
})

Expand Down Expand Up @@ -783,13 +714,3 @@ function notifyRawRumEvent<E extends RawRumEvent>(
}
lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, fullData)
}

function setSyntheticsGlobals(publicId: any, resultId?: any) {
;(window as BrowserWindow)._DATADOG_SYNTHETICS_PUBLIC_ID = publicId
;(window as BrowserWindow)._DATADOG_SYNTHETICS_RESULT_ID = resultId
}

function cleanupSyntheticsGlobals() {
delete (window as BrowserWindow)._DATADOG_SYNTHETICS_PUBLIC_ID
delete (window as BrowserWindow)._DATADOG_SYNTHETICS_RESULT_ID
}
23 changes: 1 addition & 22 deletions packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
RawError,
createEventRateLimiter,
EventRateLimiter,
getCookie,
} from '@datadog/browser-core'
import { RumEventDomainContext } from '../domainContext.types'
import {
Expand All @@ -25,16 +24,12 @@ import {
User,
} from '../rawRumEvent.types'
import { RumEvent } from '../rumEvent.types'
import { getSyntheticsContext } from '../tools/syntheticsContext'
import { LifeCycle, LifeCycleEventType } from './lifeCycle'
import { ParentContexts } from './parentContexts'
import { RumSession, RumSessionPlan } from './rumSession'
import { UrlContexts } from './urlContexts'

export interface BrowserWindow extends Window {
_DATADOG_SYNTHETICS_PUBLIC_ID?: string
_DATADOG_SYNTHETICS_RESULT_ID?: string
}

enum SessionType {
SYNTHETICS = 'synthetics',
USER = 'user',
Expand Down Expand Up @@ -158,19 +153,3 @@ function needToAssembleWithAction(
): event is RawRumErrorEvent | RawRumResourceEvent | RawRumLongTaskEvent {
return [RumEventType.ERROR, RumEventType.RESOURCE, RumEventType.LONG_TASK].indexOf(event.type) !== -1
}

export const SYNTHETICS_TEST_ID_COOKIE_NAME = 'datadog-synthetics-public-id'
export const SYNTHETICS_RESULT_ID_COOKIE_NAME = 'datadog-synthetics-result-id'

function getSyntheticsContext() {
const testId = (window as BrowserWindow)._DATADOG_SYNTHETICS_PUBLIC_ID || getCookie(SYNTHETICS_TEST_ID_COOKIE_NAME)
const resultId =
(window as BrowserWindow)._DATADOG_SYNTHETICS_RESULT_ID || getCookie(SYNTHETICS_RESULT_ID_COOKIE_NAME)

if (typeof testId === 'string' && typeof resultId === 'string') {
return {
test_id: testId,
result_id: resultId,
}
}
}
74 changes: 74 additions & 0 deletions packages/rum-core/src/tools/syntheticsContext.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { mockSyntheticsWorkerValues, cleanupSyntheticsWorkerValues } from '../../test/specHelper'
import { getSyntheticsContext, willSyntheticsInjectRum } from './syntheticsContext'

describe('getSyntheticsContext', () => {
afterEach(() => {
cleanupSyntheticsWorkerValues()
})

it('sets the synthetics context defined by global variables', () => {
mockSyntheticsWorkerValues({ publicId: 'foo', resultId: 'bar' }, 'globals')

expect(getSyntheticsContext()).toEqual({
test_id: 'foo',
result_id: 'bar',
})
})

it('sets the synthetics context defined by global cookie', () => {
mockSyntheticsWorkerValues({ publicId: 'foo', resultId: 'bar' }, 'cookies')

expect(getSyntheticsContext()).toEqual({
test_id: 'foo',
result_id: 'bar',
})
})

it('does not set synthetics context if one global variable is undefined', () => {
mockSyntheticsWorkerValues({ publicId: 'foo' }, 'globals')

expect(getSyntheticsContext()).toBeUndefined()
})

it('does not set synthetics context if global variables are not strings', () => {
mockSyntheticsWorkerValues({ publicId: 1, resultId: 2 }, 'globals')

expect(getSyntheticsContext()).toBeUndefined()
})

it('does not set synthetics context if one cookie is undefined', () => {
mockSyntheticsWorkerValues({ publicId: 'foo' }, 'cookies')

expect(getSyntheticsContext()).toBeUndefined()
})
})

describe('willSyntheticsInjectRum', () => {
afterEach(() => {
cleanupSyntheticsWorkerValues()
})

it('returns false if nothing is defined"', () => {
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
mockSyntheticsWorkerValues({}, 'globals')

expect(willSyntheticsInjectRum()).toBeFalse()
})

it('returns false if the INJECTS_RUM global variable is false"', () => {
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
mockSyntheticsWorkerValues({ injectsRum: false }, 'globals')

expect(willSyntheticsInjectRum()).toBeFalse()
})

it('returns true if the INJECTS_RUM global variable is truthy"', () => {
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
mockSyntheticsWorkerValues({ injectsRum: true }, 'globals')

expect(willSyntheticsInjectRum()).toBeTrue()
})

it('returns true if the INJECTS_RUM cookie is truthy"', () => {
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
mockSyntheticsWorkerValues({ injectsRum: true }, 'cookies')

expect(willSyntheticsInjectRum()).toBeTrue()
})
})
30 changes: 30 additions & 0 deletions packages/rum-core/src/tools/syntheticsContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { getCookie } from '@datadog/browser-core'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tools/syntheticsContext.ts, should it be consider as a tool?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you rather put it in domain? I thought it wasn't directly related to how RUM works, but happy to move it elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not to have tools, but I would also think of moving other components like ContextHistory which also looks like tools.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO

  • syntheticsContext -> domain
  • contextHistory -> tools


export const SYNTHETICS_TEST_ID_COOKIE_NAME = 'datadog-synthetics-public-id'
export const SYNTHETICS_RESULT_ID_COOKIE_NAME = 'datadog-synthetics-result-id'
export const SYNTHETICS_INJECTS_RUM_COOKIE_NAME = 'datadog-synthetics-injects-rum'

export interface BrowserWindow extends Window {
_DATADOG_SYNTHETICS_PUBLIC_ID?: string
_DATADOG_SYNTHETICS_RESULT_ID?: string
_DATADOG_SYNTHETICS_INJECTS_RUM?: boolean
}

export function getSyntheticsContext() {
const testId = (window as BrowserWindow)._DATADOG_SYNTHETICS_PUBLIC_ID || getCookie(SYNTHETICS_TEST_ID_COOKIE_NAME)
const resultId =
(window as BrowserWindow)._DATADOG_SYNTHETICS_RESULT_ID || getCookie(SYNTHETICS_RESULT_ID_COOKIE_NAME)

if (typeof testId === 'string' && typeof resultId === 'string') {
return {
test_id: testId,
result_id: resultId,
}
}
}

export function willSyntheticsInjectRum() {
return Boolean(
(window as BrowserWindow)._DATADOG_SYNTHETICS_INJECTS_RUM || getCookie(SYNTHETICS_INJECTS_RUM_COOKIE_NAME)
)
}
Loading