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.

Sign up for GitHub

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

♻️ Base foreground computation on page lifecycle states #2253

Merged
merged 6 commits into from
Jun 28, 2023
Merged

♻️ Base foreground computation on page lifecycle states #2253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8a1e483
Add pageStateHistory.isActiveAt
amortemousque May 17, 2023
ed65c7c
Base foreground computation on pageStateHistory
amortemousque May 17, 2023
1030091
Use the new apis to populate foreground periods
amortemousque May 17, 2023
4a54901
Rename isActiveAt to isInActivePageStateAt
amortemousque May 17, 2023
973cdee
in_foreground can be undefined
amortemousque May 17, 2023
91ccc08
Merge branch 'main' into aymeric/foreground-based-on-page-lifecycle
amortemousque Jun 28, 2023
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
3 changes: 1 addition & 2 deletions packages/rum-core/src/boot/startRum.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function startRumStub(
pageStateHistory: PageStateHistory,
reportError: (error: RawError) => void
) {
const { stop: rumEventCollectionStop, foregroundContexts } = startRumEventCollection(
const { stop: rumEventCollectionStop } = startRumEventCollection(
lifeCycle,
configuration,
location,
Expand All @@ -71,7 +71,6 @@ function startRumStub(
location,
domMutationObservable,
locationChangeObservable,
foregroundContexts,
startFeatureFlagContexts(lifeCycle),
pageStateHistory,
noopRecorderApi
Expand Down
31 changes: 13 additions & 18 deletions packages/rum-core/src/boot/startRum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
import { createDOMMutationObservable } from '../browser/domMutationObservable'
import { startPerformanceCollection } from '../browser/performanceCollection'
import { startRumAssembly } from '../domain/assembly'
import { startForegroundContexts } from '../domain/contexts/foregroundContexts'
import { startInternalContext } from '../domain/contexts/internalContext'
import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle'
import { startViewContexts } from '../domain/contexts/viewContexts'
Expand Down Expand Up @@ -102,17 +101,16 @@ export function startRum(
const domMutationObservable = createDOMMutationObservable()
const locationChangeObservable = createLocationChangeObservable(location)

const { viewContexts, foregroundContexts, pageStateHistory, urlContexts, actionContexts, addAction } =
startRumEventCollection(
lifeCycle,
configuration,
location,
session,
locationChangeObservable,
domMutationObservable,
() => buildCommonContext(globalContextManager, userContextManager, recorderApi),
reportError
)
const { viewContexts, pageStateHistory, urlContexts, actionContexts, addAction } = startRumEventCollection(
lifeCycle,
configuration,
location,
session,
locationChangeObservable,
domMutationObservable,
() => buildCommonContext(globalContextManager, userContextManager, recorderApi),
reportError
)

addTelemetryConfiguration(serializeRumConfiguration(initConfiguration))

Expand All @@ -124,13 +122,12 @@ export function startRum(
location,
domMutationObservable,
locationChangeObservable,
foregroundContexts,
featureFlagContexts,
pageStateHistory,
recorderApi,
initialViewOptions
)
const { addError } = startErrorCollection(lifeCycle, foregroundContexts, featureFlagContexts)
const { addError } = startErrorCollection(lifeCycle, pageStateHistory, featureFlagContexts)

startRequestCollection(lifeCycle, configuration, session)
startPerformanceCollection(lifeCycle, configuration)
Expand Down Expand Up @@ -179,14 +176,13 @@ export function startRumEventCollection(
const viewContexts = startViewContexts(lifeCycle)
const urlContexts = startUrlContexts(lifeCycle, locationChangeObservable, location)

const foregroundContexts = startForegroundContexts()
const pageStateHistory = startPageStateHistory()

const { addAction, actionContexts } = startActionCollection(
lifeCycle,
domMutationObservable,
configuration,
foregroundContexts
pageStateHistory
)

startRumAssembly(
Expand All @@ -202,14 +198,13 @@ export function startRumEventCollection(

return {
viewContexts,
foregroundContexts,
pageStateHistory,
urlContexts,
addAction,
actionContexts,
stop: () => {
viewContexts.stop()
foregroundContexts.stop()
pageStateHistory.stop()
},
}
}
120 changes: 55 additions & 65 deletions packages/rum-core/src/domain/contexts/foregroundContexts.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,40 @@ import type { RelativeTime, Duration, ServerDuration } from '@datadog/browser-co
import { relativeNow } from '@datadog/browser-core'
import type { TestSetupBuilder } from '../../../test'
import { setup } from '../../../test'
import type { ForegroundContexts } from './foregroundContexts'
import {
startForegroundContexts,
MAX_NUMBER_OF_SELECTABLE_FOREGROUND_PERIODS,
MAX_NUMBER_OF_STORED_FOREGROUND_PERIODS,
closeForegroundPeriod,
addNewForegroundPeriod,
} from './foregroundContexts'
import { mapToForegroundPeriods } from './foregroundContexts'
import type { PageStateHistory } from './pageStateHistory'
import { PageState, startPageStateHistory } from './pageStateHistory'

const FOCUS_PERIOD_LENGTH = 10 as Duration
const BLUR_PERIOD_LENGTH = 5 as Duration

describe('foreground context', () => {
let foregroundContext: ForegroundContexts
let setupBuilder: TestSetupBuilder
let pageStateHistory: PageStateHistory

function addNewForegroundPeriod() {
pageStateHistory.addPageState(PageState.ACTIVE)
}

function closeForegroundPeriod() {
pageStateHistory.addPageState(PageState.PASSIVE)
}

function selectInForegroundPeriodsFor(startTime: RelativeTime, duration: Duration) {
const pageStates = pageStateHistory.findAll(startTime, duration)
return mapToForegroundPeriods(pageStates || [], duration)
}

function isInForegroundAt(startTime: RelativeTime) {
return pageStateHistory.isInActivePageStateAt(startTime)
}

beforeEach(() => {
setupBuilder = setup()
.withFakeClock()
.beforeBuild(() => {
foregroundContext = startForegroundContexts()
return foregroundContext
pageStateHistory = startPageStateHistory()
return pageStateHistory
})
})

Expand All @@ -34,6 +46,7 @@ describe('foreground context', () => {
describe('when the page do not have the focus when starting', () => {
beforeEach(() => {
spyOn(Document.prototype, 'hasFocus').and.callFake(() => false)
pageStateHistory = startPageStateHistory()
})
describe('without any focus nor blur event', () => {
describe('isInForegroundAt', () => {
Expand All @@ -42,7 +55,7 @@ describe('foreground context', () => {

clock.tick(1_000)

expect(foregroundContext.isInForegroundAt(relativeNow())).toEqual(false)
expect(isInForegroundAt(relativeNow())).toEqual(false)
})
})

Expand All @@ -52,7 +65,7 @@ describe('foreground context', () => {

clock.tick(1_000)

expect(foregroundContext.selectInForegroundPeriodsFor(relativeNow(), 0 as Duration)).toEqual([])
expect(selectInForegroundPeriodsFor(relativeNow(), 0 as Duration)).toEqual([])
})
})
})
Expand Down Expand Up @@ -80,66 +93,66 @@ describe('foreground context', () => {

it('isInForegroundAt should match the focused/burred period', () => {
// first blurred period
expect(foregroundContext.isInForegroundAt(2 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(2 as RelativeTime)).toEqual(false)

// first focused period
expect(foregroundContext.isInForegroundAt(10 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(10 as RelativeTime)).toEqual(true)

// second blurred period
expect(foregroundContext.isInForegroundAt(17 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(17 as RelativeTime)).toEqual(false)

// second focused period
expect(foregroundContext.isInForegroundAt(25 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(25 as RelativeTime)).toEqual(true)

// third blurred period
expect(foregroundContext.isInForegroundAt(32 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(32 as RelativeTime)).toEqual(false)

// current focused periods
expect(foregroundContext.isInForegroundAt(42 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(42 as RelativeTime)).toEqual(true)
})

describe('selectInForegroundPeriodsFor', () => {
it('should have 3 in foreground periods for the whole period', () => {
const periods = foregroundContext.selectInForegroundPeriodsFor(0 as RelativeTime, 50 as Duration)
const periods = selectInForegroundPeriodsFor(0 as RelativeTime, 50 as Duration)

expect(periods).toHaveSize(3)
expect(periods![0]).toEqual({
expect(periods[0]).toEqual({
start: (5 * 1e6) as ServerDuration,
duration: (10 * 1e6) as ServerDuration,
})
expect(periods![1]).toEqual({
expect(periods[1]).toEqual({
start: (20 * 1e6) as ServerDuration,
duration: (10 * 1e6) as ServerDuration,
})
expect(periods![2]).toEqual({
expect(periods[2]).toEqual({
start: (35 * 1e6) as ServerDuration,
duration: (15 * 1e6) as ServerDuration,
})
})

it('should have 2 in foreground periods when in between the two full periods', () => {
const periods = foregroundContext.selectInForegroundPeriodsFor(10 as RelativeTime, 15 as Duration)
const periods = selectInForegroundPeriodsFor(10 as RelativeTime, 15 as Duration)

expect(periods).toHaveSize(2)
expect(periods![0]).toEqual({
expect(periods[0]).toEqual({
start: 0 as ServerDuration,
duration: (5 * 1e6) as ServerDuration,
})
expect(periods![1]).toEqual({
expect(periods[1]).toEqual({
start: (10 * 1e6) as ServerDuration,
duration: (5 * 1e6) as ServerDuration,
})
})

it('should have 2 periods, when in between the the full period and ongoing periods', () => {
const periods = foregroundContext.selectInForegroundPeriodsFor(25 as RelativeTime, 20 as Duration)
const periods = selectInForegroundPeriodsFor(25 as RelativeTime, 20 as Duration)

expect(periods).toHaveSize(2)
expect(periods![0]).toEqual({
expect(periods[0]).toEqual({
start: 0 as ServerDuration,
duration: (5 * 1e6) as ServerDuration,
})
expect(periods![1]).toEqual({
expect(periods[1]).toEqual({
start: (10 * 1e6) as ServerDuration,
duration: (10 * 1e6) as ServerDuration,
})
Expand All @@ -164,56 +177,33 @@ describe('foreground context', () => {
clock.tick(BLUR_PERIOD_LENGTH)
})
it('isInForegroundAt should match the focused/burred period', () => {
expect(foregroundContext.isInForegroundAt(2 as RelativeTime)).toEqual(false)
expect(foregroundContext.isInForegroundAt(10 as RelativeTime)).toEqual(true)
expect(foregroundContext.isInForegroundAt(20 as RelativeTime)).toEqual(true)
expect(foregroundContext.isInForegroundAt(30 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(2 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(10 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(20 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(30 as RelativeTime)).toEqual(false)
})
})

it('should not record anything after reaching the maximum number of focus periods', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've removed this test that is now covered by the ValueHistory used in pageStateHistory.ts

const { clock } = setupBuilder.build()
const start = relativeNow()
for (let i = 0; i < MAX_NUMBER_OF_STORED_FOREGROUND_PERIODS + 1; i++) {
addNewForegroundPeriod()
clock.tick(FOCUS_PERIOD_LENGTH)
closeForegroundPeriod()
clock.tick(BLUR_PERIOD_LENGTH)
}

addNewForegroundPeriod()
clock.tick(FOCUS_PERIOD_LENGTH)

expect(foregroundContext.isInForegroundAt(relativeNow())).toEqual(false)
const duration = (((FOCUS_PERIOD_LENGTH as number) + (BLUR_PERIOD_LENGTH as number)) *
MAX_NUMBER_OF_STORED_FOREGROUND_PERIODS) as Duration
expect(foregroundContext.selectInForegroundPeriodsFor(start, duration)).toHaveSize(
MAX_NUMBER_OF_SELECTABLE_FOREGROUND_PERIODS
)
})

it('should not be in foreground, when the periods is closed twice', () => {
const { clock } = setupBuilder.build()
addNewForegroundPeriod()
clock.tick(FOCUS_PERIOD_LENGTH)
closeForegroundPeriod()
clock.tick(BLUR_PERIOD_LENGTH)
closeForegroundPeriod()
pageStateHistory.addPageState(PageState.PASSIVE)

expect(foregroundContext.isInForegroundAt(relativeNow())).toEqual(false)
expect(isInForegroundAt(relativeNow())).toEqual(false)
})

it('after starting with a blur even, should not be in foreground', () => {
setupBuilder.build()
closeForegroundPeriod()
pageStateHistory.addPageState(PageState.PASSIVE)

expect(foregroundContext.isInForegroundAt(relativeNow())).toEqual(false)
expect(isInForegroundAt(relativeNow())).toEqual(false)
})
})

describe('when the page has focus when starting', () => {
beforeEach(() => {
spyOn(Document.prototype, 'hasFocus').and.callFake(() => true)
pageStateHistory = startPageStateHistory()
})

describe('when there is no focus event', () => {
Expand All @@ -222,15 +212,15 @@ describe('foreground context', () => {
clock.tick(FOCUS_PERIOD_LENGTH)
closeForegroundPeriod()

expect(foregroundContext.isInForegroundAt(2 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(2 as RelativeTime)).toEqual(true)
})

it('should return false after the first focused period', () => {
const { clock } = setupBuilder.build()
clock.tick(FOCUS_PERIOD_LENGTH)
closeForegroundPeriod()

expect(foregroundContext.isInForegroundAt(12 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(12 as RelativeTime)).toEqual(false)
})
})

Expand All @@ -242,7 +232,7 @@ describe('foreground context', () => {
clock.tick(FOCUS_PERIOD_LENGTH / 2)
closeForegroundPeriod()

expect(foregroundContext.isInForegroundAt(2 as RelativeTime)).toEqual(true)
expect(isInForegroundAt(2 as RelativeTime)).toEqual(true)
})

it('should return false after the focused period', () => {
Expand All @@ -252,7 +242,7 @@ describe('foreground context', () => {
clock.tick(FOCUS_PERIOD_LENGTH / 2)
closeForegroundPeriod()

expect(foregroundContext.isInForegroundAt(12 as RelativeTime)).toEqual(false)
expect(isInForegroundAt(12 as RelativeTime)).toEqual(false)
})
})
})
Expand Down
Loading