Skip to content

Commit

Permalink
Integrate aymeric/remove-useless-session-check (#2769) into staging-22
Browse files Browse the repository at this point in the history
Integrated commit sha: fabc81b

Co-authored-by: Aymeric Mortemousque <[email protected]>
  • Loading branch information
dd-mergequeue[bot] and amortemousque authored May 27, 2024
2 parents 581c775 + fabc81b commit c6e4758
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 103 deletions.
2 changes: 1 addition & 1 deletion packages/rum-core/src/boot/startRum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function startRumStub(
noopRecorderApi
)

startLongTaskCollection(lifeCycle, configuration, sessionManager)
startLongTaskCollection(lifeCycle, configuration)
return {
stop: () => {
rumEventCollectionStop()
Expand Down
4 changes: 2 additions & 2 deletions packages/rum-core/src/boot/startRum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ export function startRum(
drainPreStartTelemetry()
addTelemetryConfiguration(serializeRumConfiguration(initConfiguration))

startLongTaskCollection(lifeCycle, configuration, session)
startResourceCollection(lifeCycle, configuration, session, pageStateHistory)
startLongTaskCollection(lifeCycle, configuration)
startResourceCollection(lifeCycle, configuration, pageStateHistory)

const {
addTiming,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ describe('long task collection', () => {
sessionManager = createRumSessionManagerMock()
setupBuilder = setup()
.withSessionManager(sessionManager)
.beforeBuild(({ lifeCycle, sessionManager, configuration }) => {
startLongTaskCollection(lifeCycle, { ...configuration, trackLongTasks }, sessionManager)
.beforeBuild(({ lifeCycle, configuration }) => {
startLongTaskCollection(lifeCycle, { ...configuration, trackLongTasks })
})
})

Expand Down
10 changes: 2 additions & 8 deletions packages/rum-core/src/domain/longTask/longTaskCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,16 @@ import type { RawRumLongTaskEvent } from '../../rawRumEvent.types'
import { RumEventType } from '../../rawRumEvent.types'
import type { LifeCycle } from '../lifeCycle'
import { LifeCycleEventType } from '../lifeCycle'
import type { RumSessionManager } from '../rumSessionManager'
import { RumPerformanceEntryType } from '../../browser/performanceCollection'
import type { RumConfiguration } from '../configuration'

export function startLongTaskCollection(
lifeCycle: LifeCycle,
configuration: RumConfiguration,
sessionManager: RumSessionManager
) {
export function startLongTaskCollection(lifeCycle: LifeCycle, configuration: RumConfiguration) {
lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, (entries) => {
for (const entry of entries) {
if (entry.entryType !== RumPerformanceEntryType.LONG_TASK) {
break
}
const session = sessionManager.findTrackedSession(entry.startTime)
if (!session || !configuration.trackLongTasks) {
if (!configuration.trackLongTasks) {
break
}
const startClocks = relativeToClocks(entry.startTime)
Expand Down
120 changes: 51 additions & 69 deletions packages/rum-core/src/domain/resource/resourceCollection.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Duration, RelativeTime, ServerDuration, TimeStamp } from '@datadog/browser-core'
import { isIE, RequestType, ResourceType } from '@datadog/browser-core'
import type { RumFetchResourceEventDomainContext } from '../../domainContext.types'
import { setup, createRumSessionManagerMock, createPerformanceEntry } from '../../../test'
import { setup, createPerformanceEntry } from '../../../test'
import type { TestSetupBuilder } from '../../../test'
import type { RawRumResourceEvent } from '../../rawRumEvent.types'
import { RumEventType } from '../../rawRumEvent.types'
Expand All @@ -19,9 +19,9 @@ describe('resourceCollection', () => {

beforeEach(() => {
trackResources = true
setupBuilder = setup().beforeBuild(({ lifeCycle, sessionManager, pageStateHistory, configuration }) => {
setupBuilder = setup().beforeBuild(({ lifeCycle, pageStateHistory, configuration }) => {
wasInPageStateDuringPeriodSpy = spyOn(pageStateHistory, 'wasInPageStateDuringPeriod')
startResourceCollection(lifeCycle, { ...configuration, trackResources }, sessionManager, pageStateHistory)
startResourceCollection(lifeCycle, { ...configuration, trackResources }, pageStateHistory)
})
})

Expand Down Expand Up @@ -108,76 +108,61 @@ describe('resourceCollection', () => {
})
})

//
;[
{
title: 'when trackResource is false',
trackResources: false,
session: createRumSessionManagerMock(),
},
{
title: 'when the session is not tracked',
trackResources: true,
session: createRumSessionManagerMock().setNotTracked(),
},
].forEach((options) => {
describe(options.title, () => {
beforeEach(() => {
trackResources = options.trackResources
setupBuilder.withSessionManager(options.session)
})
describe('when trackResource is false', () => {
beforeEach(() => {
trackResources = false
})

describe('and resource is not traced', () => {
it('should not collect a resource from a performance entry', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
describe('and resource is not traced', () => {
it('should not collect a resource from a performance entry', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
createPerformanceEntry(RumPerformanceEntryType.RESOURCE),
])
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
createPerformanceEntry(RumPerformanceEntryType.RESOURCE),
])

expect(rawRumEvents.length).toBe(0)
})
expect(rawRumEvents.length).toBe(0)
})

it('should not collect a resource from a completed XHR request', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
lifeCycle.notify(
LifeCycleEventType.REQUEST_COMPLETED,
createCompletedRequest({
type: RequestType.XHR,
})
)
it('should not collect a resource from a completed XHR request', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
lifeCycle.notify(
LifeCycleEventType.REQUEST_COMPLETED,
createCompletedRequest({
type: RequestType.XHR,
})
)

expect(rawRumEvents.length).toBe(0)
})
expect(rawRumEvents.length).toBe(0)
})
})

describe('and resource is traced', () => {
it('should collect a resource from a performance entry', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
describe('and resource is traced', () => {
it('should collect a resource from a performance entry', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { traceId: '1234' }),
])
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [
createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { traceId: '1234' }),
])

expect(rawRumEvents.length).toBe(1)
expect((rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd.discarded).toBeTrue()
})
expect(rawRumEvents.length).toBe(1)
expect((rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd.discarded).toBeTrue()
})

it('should collect a resource from a completed XHR request', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
lifeCycle.notify(
LifeCycleEventType.REQUEST_COMPLETED,
createCompletedRequest({
type: RequestType.XHR,
traceId: new TraceIdentifier(),
spanId: new TraceIdentifier(),
traceSampled: true,
})
)

expect(rawRumEvents.length).toBe(1)
expect((rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd.discarded).toBeTrue()
})
it('should collect a resource from a completed XHR request', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
lifeCycle.notify(
LifeCycleEventType.REQUEST_COMPLETED,
createCompletedRequest({
type: RequestType.XHR,
traceId: new TraceIdentifier(),
spanId: new TraceIdentifier(),
traceSampled: true,
})
)

expect(rawRumEvents.length).toBe(1)
expect((rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd.discarded).toBeTrue()
})
})
})
Expand Down Expand Up @@ -324,15 +309,14 @@ describe('resourceCollection', () => {
})

it('should pull traceSampleRate from config if present', () => {
setupBuilder = setup().beforeBuild(({ lifeCycle, sessionManager, pageStateHistory }) => {
setupBuilder = setup().beforeBuild(({ lifeCycle, pageStateHistory }) => {
startResourceCollection(
lifeCycle,
validateAndBuildRumConfiguration({
clientToken: 'xxx',
applicationId: 'xxx',
traceSampleRate: 60,
})!,
sessionManager,
pageStateHistory
)
})
Expand All @@ -351,14 +335,13 @@ describe('resourceCollection', () => {
})

it('should not define rule_psr if traceSampleRate is undefined', () => {
setupBuilder = setup().beforeBuild(({ lifeCycle, sessionManager, pageStateHistory }) => {
setupBuilder = setup().beforeBuild(({ lifeCycle, pageStateHistory }) => {
startResourceCollection(
lifeCycle,
validateAndBuildRumConfiguration({
clientToken: 'xxx',
applicationId: 'xxx',
})!,
sessionManager,
pageStateHistory
)
})
Expand All @@ -377,15 +360,14 @@ describe('resourceCollection', () => {
})

it('should define rule_psr to 0 if traceSampleRate is set to 0', () => {
setupBuilder = setup().beforeBuild(({ lifeCycle, sessionManager, pageStateHistory }) => {
setupBuilder = setup().beforeBuild(({ lifeCycle, pageStateHistory }) => {
startResourceCollection(
lifeCycle,
validateAndBuildRumConfiguration({
clientToken: 'xxx',
applicationId: 'xxx',
traceSampleRate: 0,
})!,
sessionManager,
pageStateHistory
)
})
Expand Down
28 changes: 7 additions & 21 deletions packages/rum-core/src/domain/resource/resourceCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { RumEventType } from '../../rawRumEvent.types'
import { LifeCycleEventType } from '../lifeCycle'
import type { RawRumEventCollectedData, LifeCycle } from '../lifeCycle'
import type { RequestCompleteEvent } from '../requestCollection'
import type { RumSessionManager } from '../rumSessionManager'
import type { PageStateHistory } from '../contexts/pageStateHistory'
import { PageState } from '../contexts/pageStateHistory'
import { matchRequestTiming } from './matchRequestTiming'
Expand All @@ -35,11 +34,10 @@ import {
export function startResourceCollection(
lifeCycle: LifeCycle,
configuration: RumConfiguration,
sessionManager: RumSessionManager,
pageStateHistory: PageStateHistory
) {
lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, (request: RequestCompleteEvent) => {
const rawEvent = processRequest(request, configuration, sessionManager, pageStateHistory)
const rawEvent = processRequest(request, configuration, pageStateHistory)
if (rawEvent) {
lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, rawEvent)
}
Expand All @@ -48,7 +46,7 @@ export function startResourceCollection(
lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, (entries) => {
for (const entry of entries) {
if (entry.entryType === RumPerformanceEntryType.RESOURCE && !isRequestKind(entry)) {
const rawEvent = processResourceEntry(entry, configuration, sessionManager)
const rawEvent = processResourceEntry(entry, configuration)
if (rawEvent) {
lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, rawEvent)
}
Expand All @@ -60,14 +58,12 @@ export function startResourceCollection(
function processRequest(
request: RequestCompleteEvent,
configuration: RumConfiguration,
sessionManager: RumSessionManager,
pageStateHistory: PageStateHistory
): RawRumEventCollectedData<RawRumResourceEvent> | undefined {
const matchingTiming = matchRequestTiming(request)
const startClocks = matchingTiming ? relativeToClocks(matchingTiming.startTime) : request.startClocks
const shouldIndex = shouldIndexResource(configuration, sessionManager, startClocks)
const tracingInfo = computeRequestTracingInfo(request, configuration)
if (!shouldIndex && !tracingInfo) {
if (!configuration.trackResources && !tracingInfo) {
return
}

Expand All @@ -90,7 +86,7 @@ function processRequest(
},
type: RumEventType.RESOURCE as const,
_dd: {
discarded: !shouldIndex,
discarded: !configuration.trackResources,
},
},
tracingInfo,
Expand All @@ -113,13 +109,11 @@ function processRequest(

function processResourceEntry(
entry: RumPerformanceResourceTiming,
configuration: RumConfiguration,
sessionManager: RumSessionManager
configuration: RumConfiguration
): RawRumEventCollectedData<RawRumResourceEvent> | undefined {
const startClocks = relativeToClocks(entry.startTime)
const shouldIndex = shouldIndexResource(configuration, sessionManager, startClocks)
const tracingInfo = computeEntryTracingInfo(entry, configuration)
if (!shouldIndex && !tracingInfo) {
if (!configuration.trackResources && !tracingInfo) {
return
}

Expand All @@ -137,7 +131,7 @@ function processResourceEntry(
},
type: RumEventType.RESOURCE as const,
_dd: {
discarded: !shouldIndex,
discarded: !configuration.trackResources,
},
},
tracingInfo,
Expand All @@ -152,14 +146,6 @@ function processResourceEntry(
}
}

function shouldIndexResource(
configuration: RumConfiguration,
sessionManager: RumSessionManager,
resourceStart: ClocksState
) {
return configuration.trackResources && sessionManager.findTrackedSession(resourceStart.relative)
}

function computePerformanceEntryMetrics(timing: RumPerformanceResourceTiming) {
const { renderBlockingStatus } = timing
return {
Expand Down

0 comments on commit c6e4758

Please sign in to comment.