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-1440] improve feature flag collection implementation #1839

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/rum-core/src/boot/startRum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ function startRumStub(
locationChangeObservable: Observable<LocationChange>,
reportError: (error: RawError) => void
) {
const { stop: rumEventCollectionStop, foregroundContexts } = startRumEventCollection(
const {
stop: rumEventCollectionStop,
foregroundContexts,
featureFlagContexts,
} = startRumEventCollection(
lifeCycle,
configuration,
location,
Expand All @@ -63,6 +67,7 @@ function startRumStub(
domMutationObservable,
locationChangeObservable,
foregroundContexts,
featureFlagContexts,
noopRecorderApi
)

Expand Down
32 changes: 19 additions & 13 deletions packages/rum-core/src/boot/startRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { createLocationChangeObservable } from '../browser/locationChangeObserva
import type { RumConfiguration, RumInitConfiguration } from '../domain/configuration'
import { serializeRumConfiguration } from '../domain/configuration'
import type { ViewOptions } from '../domain/rumEventsCollection/view/trackViews'
import { startFeatureFlagContexts } from '../domain/contexts/featureFlagContext'
import type { RecorderApi } from './rumPublicApi'

export function startRum(
Expand Down Expand Up @@ -75,31 +76,33 @@ export function startRum(
const domMutationObservable = createDOMMutationObservable()
const locationChangeObservable = createLocationChangeObservable(location)

const { viewContexts, foregroundContexts, urlContexts, actionContexts, addAction } = startRumEventCollection(
lifeCycle,
configuration,
location,
session,
locationChangeObservable,
domMutationObservable,
getCommonContext,
reportError
)
const { viewContexts, foregroundContexts, featureFlagContexts, urlContexts, actionContexts, addAction } =
startRumEventCollection(
lifeCycle,
configuration,
location,
session,
locationChangeObservable,
domMutationObservable,
getCommonContext,
reportError
)
addTelemetryConfiguration(serializeRumConfiguration(initConfiguration))

startLongTaskCollection(lifeCycle, session)
startResourceCollection(lifeCycle, configuration, session)
const { addTiming, addFeatureFlagEvaluation, startView } = startViewCollection(
const { addTiming, startView } = startViewCollection(
lifeCycle,
configuration,
location,
domMutationObservable,
locationChangeObservable,
foregroundContexts,
featureFlagContexts,
recorderApi,
initialViewOptions
)
const { addError } = startErrorCollection(lifeCycle, foregroundContexts, viewContexts)
const { addError } = startErrorCollection(lifeCycle, foregroundContexts, featureFlagContexts)

startRequestCollection(lifeCycle, configuration, session)
startPerformanceCollection(lifeCycle, configuration)
Expand All @@ -116,7 +119,7 @@ export function startRum(
addAction,
addError,
addTiming,
addFeatureFlagEvaluation,
addFeatureFlagEvaluation: featureFlagContexts.addFeatureFlagEvaluation,
startView,
lifeCycle,
viewContexts,
Expand Down Expand Up @@ -146,6 +149,8 @@ export function startRumEventCollection(
) {
const viewContexts = startViewContexts(lifeCycle)
const urlContexts = startUrlContexts(lifeCycle, locationChangeObservable, location)
const featureFlagContexts = startFeatureFlagContexts(lifeCycle)

const foregroundContexts = startForegroundContexts()
const { addAction, actionContexts } = startActionCollection(
lifeCycle,
Expand All @@ -169,6 +174,7 @@ export function startRumEventCollection(
viewContexts,
foregroundContexts,
urlContexts,
featureFlagContexts,
addAction,
actionContexts,
stop: () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/rum-core/src/domain/assembly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe('rum assembly', () => {
findView = () => ({
id: '7890',
name: 'view name',
featureFlagEvaluations: {},
})
reportErrorSpy = jasmine.createSpy('reportError')
commonContext = {
Expand Down Expand Up @@ -473,7 +472,7 @@ describe('rum assembly', () => {

it('should be overridden by the view context', () => {
const { lifeCycle } = setupBuilder.build()
findView = () => ({ service: 'new service', version: 'new version', id: '1234', featureFlagEvaluations: {} })
findView = () => ({ service: 'new service', version: 'new version', id: '1234' })
notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.ACTION),
})
Expand Down
147 changes: 147 additions & 0 deletions packages/rum-core/src/domain/contexts/featureFlagContext.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import type { RelativeTime } from '@datadog/browser-core'
import { resetExperimentalFeatures, updateExperimentalFeatures, relativeToClocks } from '@datadog/browser-core'
import type { TestSetupBuilder } from '../../../test/specHelper'
import { setup } from '../../../test/specHelper'
import { LifeCycleEventType } from '../lifeCycle'
import type { ViewCreatedEvent, ViewEndedEvent } from '../rumEventsCollection/view/trackViews'
import type { FeatureFlagContexts } from './featureFlagContext'
import { startFeatureFlagContexts } from './featureFlagContext'

describe('featureFlagContexts', () => {
let setupBuilder: TestSetupBuilder
let featureFlagContexts: FeatureFlagContexts

beforeEach(() => {
setupBuilder = setup().beforeBuild(({ lifeCycle }) => {
featureFlagContexts = startFeatureFlagContexts(lifeCycle)
})
})

afterEach(() => {
resetExperimentalFeatures()
setupBuilder.cleanup()
})

it('should return undefined before the initial view', () => {
setupBuilder.build()

expect(featureFlagContexts.findFeatureFlagEvaluations()).toBeUndefined()
})

describe('addFeatureFlagEvaluation', () => {
it('should add feature flag evaluations of any type when the ff feature_flags is enabled', () => {
updateExperimentalFeatures(['feature_flags'])

const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo')
featureFlagContexts.addFeatureFlagEvaluation('feature2', 2)
featureFlagContexts.addFeatureFlagEvaluation('feature3', true)
featureFlagContexts.addFeatureFlagEvaluation('feature4', { foo: 'bar' })

const featureFlagContext = featureFlagContexts.findFeatureFlagEvaluations()!

expect(featureFlagContext).toEqual({
feature: 'foo',
feature2: 2,
feature3: true,
feature4: { foo: 'bar' },
})
})

it('should replace existing feature flag evaluation to the current context when the ff feature_flags is enabled', () => {
updateExperimentalFeatures(['feature_flags'])

const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo')
featureFlagContexts.addFeatureFlagEvaluation('feature2', 'baz')
featureFlagContexts.addFeatureFlagEvaluation('feature', 'bar')

const featureFlagContext = featureFlagContexts.findFeatureFlagEvaluations()!

expect(featureFlagContext).toEqual({ feature: 'bar', feature2: 'baz' })
})

it('should not add feature flag evaluation when the ff feature_flags is disabled', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo')

const featureFlagContext = featureFlagContexts.findFeatureFlagEvaluations()!

expect(featureFlagContext).toBeUndefined()
})
})

describe('findFeatureFlagEvaluations', () => {
/**
* It could happen if there is an event happening just between view end and view creation
* (which seems unlikely) and this event would anyway be rejected by lack of view id
*/
it('should return undefined when no current view', () => {
updateExperimentalFeatures(['feature_flags'])

setupBuilder.build()

expect(featureFlagContexts.findFeatureFlagEvaluations()).toBeUndefined()
})

it('should clear feature flag context on new view', () => {
updateExperimentalFeatures(['feature_flags'])

const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)
featureFlagContexts.addFeatureFlagEvaluation('feature', 'foo')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(10 as RelativeTime),
} as ViewCreatedEvent)

const featureFlagContext = featureFlagContexts.findFeatureFlagEvaluations()!
expect(featureFlagContext).toEqual({})
})

it('should return the feature flag context corresponding to the start time', () => {
updateExperimentalFeatures(['feature_flags'])
liywjl marked this conversation as resolved.
Show resolved Hide resolved

const { lifeCycle, clock } = setupBuilder.withFakeClock().build()

lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(0 as RelativeTime),
} as ViewCreatedEvent)

clock.tick(10)
featureFlagContexts.addFeatureFlagEvaluation('feature', 'one')
lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, {
endClocks: relativeToClocks(10 as RelativeTime),
} as ViewEndedEvent)
lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, {
startClocks: relativeToClocks(10 as RelativeTime),
} as ViewCreatedEvent)

clock.tick(10)
featureFlagContexts.addFeatureFlagEvaluation('feature', 'two')

expect(featureFlagContexts.findFeatureFlagEvaluations(5 as RelativeTime)).toEqual({ feature: 'one' })
expect(featureFlagContexts.findFeatureFlagEvaluations(15 as RelativeTime)).toEqual({ feature: 'two' })
})
})
})
49 changes: 49 additions & 0 deletions packages/rum-core/src/domain/contexts/featureFlagContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import type { RelativeTime, ContextValue, Context } from '@datadog/browser-core'
import {
deepClone,
noop,
isExperimentalFeatureEnabled,
SESSION_TIME_OUT_DELAY,
ContextHistory,
} from '@datadog/browser-core'
import type { LifeCycle } from '../lifeCycle'
import { LifeCycleEventType } from '../lifeCycle'

export const FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY = SESSION_TIME_OUT_DELAY

export type FeatureFlagContext = Context

export interface FeatureFlagContexts {
findFeatureFlagEvaluations: (startTime?: RelativeTime) => FeatureFlagContext | undefined
addFeatureFlagEvaluation: (key: string, value: ContextValue) => void
}

export function startFeatureFlagContexts(lifeCycle: LifeCycle): FeatureFlagContexts {
if (!isExperimentalFeatureEnabled('feature_flags')) {
return {
findFeatureFlagEvaluations: () => undefined,
addFeatureFlagEvaluation: noop,
}
}

const featureFlagContexts = new ContextHistory<FeatureFlagContext>(FEATURE_FLAG_CONTEXT_TIME_OUT_DELAY)

lifeCycle.subscribe(LifeCycleEventType.VIEW_ENDED, ({ endClocks }) => {
featureFlagContexts.closeActive(endClocks.relative)
})

lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, ({ startClocks }) => {
featureFlagContexts.add({}, startClocks.relative)
})

return {
findFeatureFlagEvaluations: (startTime?: RelativeTime) => featureFlagContexts.find(startTime),
addFeatureFlagEvaluation: (key: string, value: ContextValue) => {
const currentContext = featureFlagContexts.find()
if (currentContext) {
// mutate the current context to avoid creating a new context history entry to save memory
amortemousque marked this conversation as resolved.
Show resolved Hide resolved
currentContext[key] = deepClone(value)
}
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ describe('viewContexts', () => {
return {
startClocks,
id: FAKE_ID,
featureFlagEvaluations: {},
...partialViewCreatedEvent,
}
}
Expand Down
4 changes: 1 addition & 3 deletions packages/rum-core/src/domain/contexts/viewContexts.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Context, RelativeTime } from '@datadog/browser-core'
import type { RelativeTime } from '@datadog/browser-core'
import { SESSION_TIME_OUT_DELAY, ContextHistory } from '@datadog/browser-core'
import type { LifeCycle } from '../lifeCycle'
import { LifeCycleEventType } from '../lifeCycle'
Expand All @@ -11,7 +11,6 @@ export interface ViewContext {
version?: string
id: string
name?: string
featureFlagEvaluations: Context
}

export interface ViewContexts {
Expand Down Expand Up @@ -40,7 +39,6 @@ export function startViewContexts(lifeCycle: LifeCycle): ViewContexts {
version: view.version,
id: view.id,
name: view.name,
featureFlagEvaluations: view.featureFlagEvaluations,
}
}

Expand Down
Loading