Skip to content

Commit

Permalink
♻️ Simplify RUM assembly (#1588)
Browse files Browse the repository at this point in the history
  • Loading branch information
amortemousque authored Jun 15, 2022
1 parent 1ea6cc8 commit dc6cf2a
Show file tree
Hide file tree
Showing 14 changed files with 73 additions and 93 deletions.
2 changes: 1 addition & 1 deletion packages/rum-core/src/boot/startRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function startRum(
id: session.findTrackedSession()?.id,
},
view: {
id: viewContexts.findView()?.view.id,
id: viewContexts.findView()?.id,
},
action: {
id: actionContexts.findActionId(),
Expand Down
10 changes: 4 additions & 6 deletions packages/rum-core/src/domain/assembly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ describe('rum assembly', () => {
let findView: () => ViewContext
beforeEach(() => {
findView = () => ({
view: {
id: '7890',
name: 'view name',
},
id: '7890',
name: 'view name',
})
commonContext = {
context: {},
Expand Down Expand Up @@ -490,7 +488,7 @@ describe('rum assembly', () => {

it('should be overridden by the view context', () => {
const { lifeCycle } = setupBuilder.build()
findView = () => ({ service: 'new service', version: 'new version', view: { id: '1234' } })
findView = () => ({ service: 'new service', version: 'new version', id: '1234' })
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.ACTION),
})
Expand All @@ -502,7 +500,7 @@ describe('rum assembly', () => {
describe('when sub-apps ff disabled', () => {
it('should not be overridden by the view context', () => {
const { lifeCycle } = setupBuilder.build()
findView = () => ({ service: 'new service', version: 'new version', view: { id: '1234' } })
findView = () => ({ service: 'new service', version: 'new version', id: '1234' })
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.ACTION),
})
Expand Down
29 changes: 14 additions & 15 deletions packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ export function startRumAssembly(
const session = sessionManager.findTrackedSession(rawRumEvent.type !== RumEventType.VIEW ? startTime : undefined)
if (session && viewContext && urlContext) {
const commonContext = savedCommonContext || getCommonContext()
const actionId = actionContexts.findActionId(startTime)

const rumContext: RumContext = {
_dd: {
format_version: 2,
Expand All @@ -113,30 +115,27 @@ export function startRumAssembly(
id: configuration.applicationId,
},
date: timeStampNow(),
service: configuration.service,
service: isExperimentalFeatureEnabled('sub-apps')
? viewContext.service || configuration.service
: configuration.service,
version: isExperimentalFeatureEnabled('sub-apps') ? viewContext.version || configuration.version : undefined,
source: 'browser',
session: {
id: session.id,
type: syntheticsContext ? SessionType.SYNTHETICS : ciTestContext ? SessionType.CI_TEST : SessionType.USER,
},
view: {
id: viewContext.id,
name: viewContext.name,
url: urlContext.url,
referrer: urlContext.referrer,
},
action: needToAssembleWithAction(rawRumEvent) && actionId ? { id: actionId } : undefined,
synthetics: syntheticsContext,
ci_test: ciTestContext,
}
const actionId = actionContexts.findActionId(startTime)

if (!isExperimentalFeatureEnabled('sub-apps')) {
delete viewContext.service
delete viewContext.version
} else {
rumContext.version = configuration.version
}

const serverRumEvent = (
needToAssembleWithAction(rawRumEvent) && actionId
? combine(rumContext, urlContext, viewContext, { action: { id: actionId } }, rawRumEvent)
: combine(rumContext, urlContext, viewContext, rawRumEvent)
) as RumEvent & Context

const serverRumEvent = combine(rumContext as RumContext & Context, rawRumEvent) as RumEvent & Context
serverRumEvent.context = combine(commonContext.context, customerContext)

if (!('has_replay' in serverRumEvent.session)) {
Expand Down
6 changes: 3 additions & 3 deletions packages/rum-core/src/domain/internalContext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ describe('internal context', () => {
beforeEach(() => {
viewContextsStub = {
findView: jasmine.createSpy('findView').and.returnValue({
view: {
id: 'abcde',
},
id: 'abcde',
name: 'foo',
}),
}
actionContextsStub = {
Expand Down Expand Up @@ -53,6 +52,7 @@ describe('internal context', () => {
},
view: {
id: 'abcde',
name: 'foo',
referrer: document.referrer,
url: fakeLocation.href!,
},
Expand Down
3 changes: 1 addition & 2 deletions packages/rum-core/src/domain/internalContext.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { RelativeTime } from '@datadog/browser-core'
import { assign } from '@datadog/browser-core'
import type { InternalContext } from '../rawRumEvent.types'
import type { ViewContexts } from './viewContexts'
import type { ActionContexts } from './rumEventsCollection/action/actionCollection'
Expand Down Expand Up @@ -28,7 +27,7 @@ export function startInternalContext(
application_id: applicationId,
session_id: session.id,
user_action: actionId ? { id: actionId } : undefined,
view: assign({}, viewContext.view, urlContext.view),
view: { id: viewContext.id, name: viewContext.name, referrer: urlContext.referrer, url: urlContext.url },
}
}
},
Expand Down
36 changes: 14 additions & 22 deletions packages/rum-core/src/domain/urlContexts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ describe('urlContexts', () => {
} as ViewCreatedEvent)

const urlContext = urlContexts.findUrl()!
expect(urlContext.view.url).toBe('http://fake-url.com/')
expect(urlContext.view.referrer).toBe(document.referrer)
expect(urlContext.url).toBe('http://fake-url.com/')
expect(urlContext.referrer).toBe(document.referrer)
})

it('should update url context on location change', () => {
Expand All @@ -60,8 +60,8 @@ describe('urlContexts', () => {
changeLocation('/foo')

const urlContext = urlContexts.findUrl()!
expect(urlContext.view.url).toContain('http://fake-url.com/foo')
expect(urlContext.view.referrer).toBe(document.referrer)
expect(urlContext.url).toContain('http://fake-url.com/foo')
expect(urlContext.referrer).toBe(document.referrer)
})

it('should update url context on new view', () => {
Expand All @@ -79,8 +79,8 @@ describe('urlContexts', () => {
} as ViewCreatedEvent)

const urlContext = urlContexts.findUrl()!
expect(urlContext.view.url).toBe('http://fake-url.com/foo')
expect(urlContext.view.referrer).toBe('http://fake-url.com/')
expect(urlContext.url).toBe('http://fake-url.com/foo')
expect(urlContext.referrer).toBe('http://fake-url.com/')
})

it('should return the url context corresponding to the start time', () => {
Expand Down Expand Up @@ -112,28 +112,20 @@ describe('urlContexts', () => {
} as ViewCreatedEvent)

expect(urlContexts.findUrl(5 as RelativeTime)).toEqual({
view: {
url: 'http://fake-url.com/',
referrer: document.referrer,
},
url: 'http://fake-url.com/',
referrer: document.referrer,
})
expect(urlContexts.findUrl(15 as RelativeTime)).toEqual({
view: {
url: 'http://fake-url.com/foo',
referrer: 'http://fake-url.com/',
},
url: 'http://fake-url.com/foo',
referrer: 'http://fake-url.com/',
})
expect(urlContexts.findUrl(25 as RelativeTime)).toEqual({
view: {
url: 'http://fake-url.com/foo#bar',
referrer: 'http://fake-url.com/',
},
url: 'http://fake-url.com/foo#bar',
referrer: 'http://fake-url.com/',
})
expect(urlContexts.findUrl(35 as RelativeTime)).toEqual({
view: {
url: 'http://fake-url.com/qux',
referrer: 'http://fake-url.com/foo',
},
url: 'http://fake-url.com/qux',
referrer: 'http://fake-url.com/foo',
})
})

Expand Down
8 changes: 3 additions & 5 deletions packages/rum-core/src/domain/urlContexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function startUrlContexts(
urlContextHistory.add(
buildUrlContext({
url: newLocation.href,
referrer: current.view.referrer,
referrer: current.referrer,
}),
changeTime
)
Expand All @@ -60,10 +60,8 @@ export function startUrlContexts(

function buildUrlContext({ url, referrer }: { url: string; referrer: string }) {
return {
view: {
url,
referrer,
},
url,
referrer,
}
}

Expand Down
12 changes: 6 additions & 6 deletions packages/rum-core/src/domain/viewContexts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('viewContexts', () => {
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, buildViewCreatedEvent())

expect(viewContexts.findView()).toBeDefined()
expect(viewContexts.findView()!.view.id).toEqual(FAKE_ID)
expect(viewContexts.findView()!.id).toEqual(FAKE_ID)
})

it('should return the view context corresponding to startTime', () => {
Expand All @@ -71,9 +71,9 @@ describe('viewContexts', () => {
buildViewCreatedEvent({ startClocks: relativeToClocks(30 as RelativeTime), id: 'view 3' })
)

expect(viewContexts.findView(15 as RelativeTime)!.view.id).toEqual('view 1')
expect(viewContexts.findView(20 as RelativeTime)!.view.id).toEqual('view 2')
expect(viewContexts.findView(40 as RelativeTime)!.view.id).toEqual('view 3')
expect(viewContexts.findView(15 as RelativeTime)!.id).toEqual('view 1')
expect(viewContexts.findView(20 as RelativeTime)!.id).toEqual('view 2')
expect(viewContexts.findView(40 as RelativeTime)!.id).toEqual('view 3')
})

it('should return undefined when no view context corresponding to startTime', () => {
Expand All @@ -100,14 +100,14 @@ describe('viewContexts', () => {
const newViewId = 'fake 2'
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, buildViewCreatedEvent({ id: newViewId }))

expect(viewContexts.findView()!.view.id).toEqual(newViewId)
expect(viewContexts.findView()!.id).toEqual(newViewId)
})

it('should return the view name with the view', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, buildViewCreatedEvent({ name: 'Fake name' }))
expect(viewContexts.findView()!.view.name).toBe('Fake name')
expect(viewContexts.findView()!.name).toBe('Fake name')
})
})

Expand Down
6 changes: 2 additions & 4 deletions packages/rum-core/src/domain/viewContexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ export function startViewContexts(lifeCycle: LifeCycle): ViewContexts {
return {
service: view.service,
version: view.version,
view: {
id: view.id,
name: view.name,
},
id: view.id,
name: view.name,
}
}

Expand Down
27 changes: 16 additions & 11 deletions packages/rum-core/src/rawRumEvent.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ export interface RumContext {
type: string
has_replay?: boolean
}
view: {
id: string
referrer?: string
url: string
name?: string
}
action?: {
id: string | string[]
}
synthetics?: {
test_id: string
result_id: string
Expand All @@ -211,26 +220,22 @@ export interface RumContext {
}
}

export interface ViewContext extends Context {
export interface ViewContext {
service?: string
version?: string
view: {
id: string
name?: string
}
id: string
name?: string
}

export interface ActionContext extends Context {
export interface ActionContext {
action: {
id: string | string[]
}
}

export interface UrlContext extends Context {
view: {
url: string
referrer: string
}
export interface UrlContext {
url: string
referrer: string
}

export interface InternalContext {
Expand Down
14 changes: 6 additions & 8 deletions packages/rum-core/test/specHelper.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { TimeStamp } from '@datadog/browser-core'
import type { Context, TimeStamp } from '@datadog/browser-core'
import { assign, combine, Observable, noop, setCookie, deleteCookie, ONE_MINUTE } from '@datadog/browser-core'
import type { Clock } from '../../core/test/specHelper'
import { SPEC_ENDPOINTS, mockClock, buildLocation } from '../../core/test/specHelper'
Expand All @@ -11,7 +11,7 @@ import type { ViewEvent, ViewOptions } from '../src/domain/rumEventsCollection/v
import { trackViews } from '../src/domain/rumEventsCollection/view/trackViews'
import type { RumSessionManager } from '../src/domain/rumSessionManager'
import { RumSessionPlan } from '../src/domain/rumSessionManager'
import type { RawRumEvent, RumContext, ViewContext, UrlContext } from '../src/rawRumEvent.types'
import type { RawRumEvent, RumContext } from '../src/rawRumEvent.types'
import type { LocationChange } from '../src/browser/locationChangeObservable'
import type { UrlContexts } from '../src/domain/urlContexts'
import type { BrowserWindow } from '../src/domain/syntheticsContext'
Expand Down Expand Up @@ -82,10 +82,8 @@ export function setup(): TestSetupBuilder {
let viewContexts: ViewContexts
const urlContexts: UrlContexts = {
findUrl: () => ({
view: {
url: fakeLocation.href!,
referrer: document.referrer,
},
url: fakeLocation.href!,
referrer: document.referrer,
}),
stop: noop,
}
Expand Down Expand Up @@ -193,7 +191,7 @@ export function setup(): TestSetupBuilder {

function validateRumEventFormat(rawRumEvent: RawRumEvent) {
const fakeId = '00000000-aaaa-0000-aaaa-000000000000'
const fakeContext: RumContext & ViewContext & UrlContext = {
const fakeContext: RumContext = {
_dd: {
format_version: 2,
drift: 0,
Expand All @@ -216,7 +214,7 @@ function validateRumEventFormat(rawRumEvent: RawRumEvent) {
url: 'fake url',
},
}
validateRumFormat(combine(fakeContext, rawRumEvent))
validateRumFormat(combine(fakeContext as RumContext & Context, rawRumEvent))
}

export type ViewTest = ReturnType<typeof setupViewTest>
Expand Down
6 changes: 1 addition & 5 deletions packages/rum/src/boot/startRecording.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ describe('startRecording', () => {
setupBuilder = setup()
.withViewContexts({
findView() {
return {
view: {
id: viewId,
},
}
return { id: viewId }
},
})
.withSessionManager(sessionManager)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,7 @@ describe('startSegmentCollection', () => {
})

describe('computeSegmentContext', () => {
const DEFAULT_VIEW_CONTEXT: ViewContext = {
view: { id: '123' },
}

const DEFAULT_VIEW_CONTEXT: ViewContext = { id: '123' }
const DEFAULT_SESSION = createRumSessionManagerMock().setId('456')

it('returns a segment context', () => {
Expand Down
Loading

0 comments on commit dc6cf2a

Please sign in to comment.