diff --git a/.claude/skills/manual-testing/SKILL.md b/.claude/skills/manual-testing/SKILL.md index cb88a6ab6b..9d78cdc068 100644 --- a/.claude/skills/manual-testing/SKILL.md +++ b/.claude/skills/manual-testing/SKILL.md @@ -5,7 +5,7 @@ description: Run a manual test of the current change end-to-end and output repro ## Step 1: Understand the change -Review the current diff to identify what SDK behavior changed and what events/fields need to be verified. +If you don't have the PR changes in memory, review all commits in the PR to identify what SDK behavior changed and what events/fields need to be verified. ## Step 2: Start the dev server @@ -52,6 +52,13 @@ yarn dev-server intake | jq '' Use `yarn dev-server intake --help` to find the right selector. +To evaluate JavaScript on the page (e.g. to inspect `window` state set by `beforeSend`), switch focus back to tab 0 first, then use `agent-browser eval`: + +```bash +agent-browser tab 0 +agent-browser eval 'window.someValue' +``` + ## Step 5: Verify the output matches expectations, then present the test instructions Output a self-contained bash snippet with the exact commands run and the expected output. This goes directly into the PR "Test instructions" section. diff --git a/packages/core/src/browser/fetchObservable.spec.ts b/packages/core/src/browser/fetchObservable.spec.ts index dc07431cba..61e5373e5d 100644 --- a/packages/core/src/browser/fetchObservable.spec.ts +++ b/packages/core/src/browser/fetchObservable.spec.ts @@ -301,8 +301,8 @@ describe('fetch proxy with ResponseBodyAction', () => { }) }) - it('should not collect response body with WAIT or IGNORE action', (done) => { - setupFetchTracking(() => ResponseBodyAction.WAIT) + it('should not collect response body with IGNORE action', (done) => { + setupFetchTracking(() => ResponseBodyAction.IGNORE) fetch(FAKE_URL).resolveWith({ status: 200, responseText: 'response body content' }) @@ -313,7 +313,7 @@ describe('fetch proxy with ResponseBodyAction', () => { }) it('should use the highest priority action when multiple getters are registered', (done) => { - setupFetchTracking(() => ResponseBodyAction.WAIT) + setupFetchTracking(() => ResponseBodyAction.IGNORE) initFetchObservable({ responseBodyAction: () => ResponseBodyAction.COLLECT, diff --git a/packages/core/src/browser/fetchObservable.ts b/packages/core/src/browser/fetchObservable.ts index c810a49a24..cc6fabe5a9 100644 --- a/packages/core/src/browser/fetchObservable.ts +++ b/packages/core/src/browser/fetchObservable.ts @@ -45,10 +45,7 @@ type ResponseBodyActionGetter = (context: FetchResolveContext) => ResponseBodyAc */ export const enum ResponseBodyAction { IGNORE = 0, - // TODO(next-major): Remove the "WAIT" action when `trackEarlyRequests` is removed, as the - // duration of fetch requests will always come from PerformanceResourceTiming - WAIT = 1, - COLLECT = 2, + COLLECT = 1, } const FETCH_BUFFER_LIMIT = 500 @@ -154,14 +151,12 @@ async function afterSend( ResponseBodyAction.IGNORE ) as ResponseBodyAction - if (responseBodyCondition !== ResponseBodyAction.IGNORE) { + if (responseBodyCondition === ResponseBodyAction.COLLECT) { const clonedResponse = tryToClone(response) if (clonedResponse && clonedResponse.body) { try { - const bytes = await readBytesFromStream(clonedResponse.body, { - collectStreamBody: responseBodyCondition === ResponseBodyAction.COLLECT, - }) - context.responseBody = bytes && new TextDecoder().decode(bytes) + const bytes = await readBytesFromStream(clonedResponse.body) + context.responseBody = new TextDecoder().decode(bytes) } catch { // Ignore errors when reading the response body (e.g., stream aborted, network errors) // This is not critical and should not be reported as an SDK error diff --git a/packages/core/src/tools/readBytesFromStream.spec.ts b/packages/core/src/tools/readBytesFromStream.spec.ts index 32eb58238d..2cbd0f6ead 100644 --- a/packages/core/src/tools/readBytesFromStream.spec.ts +++ b/packages/core/src/tools/readBytesFromStream.spec.ts @@ -14,18 +14,9 @@ describe('readBytesFromStream', () => { }) it('should read full stream', async () => { - const bytes = await readBytesFromStream(stream, { - collectStreamBody: true, - }) + const bytes = await readBytesFromStream(stream) - expect(bytes?.length).toBe(27) - }) - - it('should read full stream without body', async () => { - const bytes = await readBytesFromStream(stream, { - collectStreamBody: false, - }) - expect(bytes).toBeUndefined() + expect(bytes.length).toBe(27) }) it('should handle rejection error on read', async () => { @@ -36,9 +27,7 @@ describe('readBytesFromStream', () => { }) try { - await readBytesFromStream(stream, { - collectStreamBody: true, - }) + await readBytesFromStream(stream) fail('Should have thrown an error') } catch (error) { expect(error).toEqual(jasmine.any(Error)) @@ -54,9 +43,7 @@ describe('readBytesFromStream', () => { cancel: () => Promise.reject(new Error('foo')), }) - const bytes = await readBytesFromStream(stream, { - collectStreamBody: true, - }) + const bytes = await readBytesFromStream(stream) expect(bytes).toBeDefined() }) }) diff --git a/packages/core/src/tools/readBytesFromStream.ts b/packages/core/src/tools/readBytesFromStream.ts index 4812a29744..daa60de7b8 100644 --- a/packages/core/src/tools/readBytesFromStream.ts +++ b/packages/core/src/tools/readBytesFromStream.ts @@ -2,17 +2,10 @@ import type { Uint8ArrayBuffer } from './utils/byteUtils' import { concatBuffers } from './utils/byteUtils' import { noop } from './utils/functionUtils' -interface Options { - // TODO(next-major): always collect stream body when `trackEarlyRequests` is removed, as we don't - // need to use this function to just wait for the end of the stream without collecting it - collectStreamBody?: boolean -} - /** * Read bytes from a ReadableStream until the end of the stream. - * Returns the bytes if collectStreamBody is true, otherwise returns undefined. */ -export async function readBytesFromStream(stream: ReadableStream, options: Options) { +export async function readBytesFromStream(stream: ReadableStream) { const reader = stream.getReader() const chunks: Uint8ArrayBuffer[] = [] @@ -22,9 +15,7 @@ export async function readBytesFromStream(stream: ReadableStream { trackViewsManually: true, trackResources: true, trackLongTasks: true, - trackEarlyRequests: true, remoteConfigurationId: '123', remoteConfigurationProxy: 'config', plugins: [{ name: 'foo', getConfigurationTelemetry: () => ({ bar: true }) }], @@ -750,7 +749,6 @@ describe('serializeRumConfiguration', () => { enable_privacy_for_action_name: false, track_resources: true, track_long_task: true, - track_early_requests: true, use_worker_url: true, compress_intake_requests: true, plugins: [{ name: 'foo', bar: true }], diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index c4cfde97ff..fc3c7668dd 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -260,14 +260,6 @@ export interface RumInitConfiguration extends InitConfiguration { */ trackLongTasks?: boolean | undefined - /** - * Enables early request collection before resource timing entries are available. - * - * @category Data Collection - * @defaultValue false - */ - trackEarlyRequests?: boolean | undefined - /** * List of plugins to enable. The plugins API is unstable and experimental, and may change without * notice. Please use only plugins provided by Datadog matching the version of the SDK you are @@ -336,7 +328,6 @@ export interface RumConfiguration extends Configuration { trackResources: boolean trackResourceHeaders: MatchOption[] trackLongTasks: boolean - trackEarlyRequests: boolean subdomain?: string traceContextInjection: TraceContextInjection plugins: RumPlugin[] @@ -408,7 +399,6 @@ export function validateAndBuildRumConfiguration( trackResources: !!(initConfiguration.trackResources ?? true), trackResourceHeaders: validateAndBuildTrackResourceHeaders(initConfiguration), trackLongTasks: !!(initConfiguration.trackLongTasks ?? true), - trackEarlyRequests: !!initConfiguration.trackEarlyRequests, subdomain: initConfiguration.subdomain, defaultPrivacyLevel: objectHasValue(DefaultPrivacyLevel, initConfiguration.defaultPrivacyLevel) ? initConfiguration.defaultPrivacyLevel @@ -586,7 +576,6 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) { track_user_interactions: configuration.trackUserInteractions, track_resources: configuration.trackResources, track_long_task: configuration.trackLongTasks, - track_early_requests: configuration.trackEarlyRequests, plugins: configuration.plugins?.map((plugin) => ({ name: plugin.name, ...plugin.getConfigurationTelemetry?.(), diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index 8ee01ad1e8..c3e25fb790 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -115,7 +115,7 @@ export function trackFetch(lifeCycle: LifeCycle, configuration: RumConfiguration if (findGraphQlConfiguration(context.url, configuration)?.trackResponseErrors) { return ResponseBodyAction.COLLECT } - return ResponseBodyAction.WAIT + return ResponseBodyAction.IGNORE }, }).subscribe((rawContext) => { const context = rawContext as RumFetchResolveContext | RumFetchStartContext diff --git a/packages/rum-core/src/domain/resource/matchRequestResourceEntry.spec.ts b/packages/rum-core/src/domain/resource/matchRequestResourceEntry.spec.ts deleted file mode 100644 index cdc98f22c4..0000000000 --- a/packages/rum-core/src/domain/resource/matchRequestResourceEntry.spec.ts +++ /dev/null @@ -1,157 +0,0 @@ -import type { Duration, RelativeTime } from '@datadog/browser-core' -import { relativeToClocks } from '@datadog/browser-core' -import type { GlobalPerformanceBufferMock } from '../../../test' -import { createPerformanceEntry, mockGlobalPerformanceBuffer } from '../../../test' -import type { RumPerformanceResourceTiming } from '../../browser/performanceObservable' -import { RumPerformanceEntryType } from '../../browser/performanceObservable' -import type { RequestCompleteEvent } from '../requestCollection' - -import { matchRequestResourceEntry } from './matchRequestResourceEntry' - -describe('matchRequestResourceEntry', () => { - const FAKE_URL = 'https://example.com' - const FAKE_REQUEST: Partial = { - url: FAKE_URL, - startClocks: relativeToClocks(100 as RelativeTime), - duration: 500 as Duration, - } - let globalPerformanceObjectMock: GlobalPerformanceBufferMock - - beforeEach(() => { - globalPerformanceObjectMock = mockGlobalPerformanceBuffer() - }) - - it('should match single entry nested in the request ', () => { - const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 200 as RelativeTime, - duration: 300 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(entry.toJSON() as RumPerformanceResourceTiming) - }) - - it('should match single entry nested in the request with error margin', () => { - const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 99 as RelativeTime, - duration: 502 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(entry.toJSON() as RumPerformanceResourceTiming) - }) - - it('should not match single entry outside the request ', () => { - const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 0 as RelativeTime, - duration: 300 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(undefined) - }) - - it('should discard already matched entries when multiple identical requests are done conurently', () => { - const entry1 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 200 as RelativeTime, - duration: 300 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry1) - - const matchingEntry1 = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry1).toEqual(entry1.toJSON() as RumPerformanceResourceTiming) - - const entry2 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 99 as RelativeTime, - duration: 502 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry2) - - const matchingEntry2 = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry2).toEqual(entry2.toJSON() as RumPerformanceResourceTiming) - }) - - it('should not match two not following entries nested in the request ', () => { - const entry1 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 150 as RelativeTime, - duration: 100 as Duration, - }) - const entry2 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 200 as RelativeTime, - duration: 100 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry1) - globalPerformanceObjectMock.addPerformanceEntry(entry2) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(undefined) - }) - - it('should not match multiple entries nested in the request', () => { - const entry1 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 100 as RelativeTime, - duration: 50 as Duration, - }) - const entry2 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 150 as RelativeTime, - duration: 50 as Duration, - }) - const entry3 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - startTime: 200 as RelativeTime, - duration: 50 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry1) - globalPerformanceObjectMock.addPerformanceEntry(entry2) - globalPerformanceObjectMock.addPerformanceEntry(entry3) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(undefined) - }) - - it('should not match entry with invalid duration', () => { - const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - duration: -1 as Duration, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(undefined) - }) - - it('should not match invalid entry nested in the request ', () => { - const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - name: FAKE_URL, - // fetchStart < startTime is invalid - fetchStart: 0 as RelativeTime, - workerStart: 0 as RelativeTime, - startTime: 200 as RelativeTime, - }) - globalPerformanceObjectMock.addPerformanceEntry(entry) - - const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent) - - expect(matchingEntry).toEqual(undefined) - }) -}) diff --git a/packages/rum-core/src/domain/resource/matchRequestResourceEntry.ts b/packages/rum-core/src/domain/resource/matchRequestResourceEntry.ts deleted file mode 100644 index ae6404b667..0000000000 --- a/packages/rum-core/src/domain/resource/matchRequestResourceEntry.ts +++ /dev/null @@ -1,64 +0,0 @@ -import type { Duration, RelativeTime } from '@datadog/browser-core' -import { addDuration } from '@datadog/browser-core' -import type { RumPerformanceResourceTiming } from '../../browser/performanceObservable' -import type { RequestCompleteEvent } from '../requestCollection' -import { hasValidResourceEntryDuration, hasValidResourceEntryTimings } from './resourceUtils' - -interface Timing { - startTime: RelativeTime - duration: Duration -} - -const alreadyMatchedEntries = new WeakSet() - -/** - * Look for corresponding timing in resource timing buffer - * - * Observations: - * - Timing (start, end) are nested inside the request (start, end) - * - Some timing can be not exactly nested, being off by < 1 ms - * - * Strategy: - * - from valid nested entries (with 1 ms error margin) - * - filter out timing that were already matched to a request - * - then, if a single timing match, return the timing - * - otherwise we can't decide, return undefined - */ -export function matchRequestResourceEntry(request: RequestCompleteEvent) { - if (!performance || !('getEntriesByName' in performance)) { - return - } - const sameNameEntries = performance.getEntriesByName(request.url, 'resource') as RumPerformanceResourceTiming[] - - if (!sameNameEntries.length || !('toJSON' in sameNameEntries[0])) { - return - } - - const candidates = sameNameEntries - .filter((entry) => !alreadyMatchedEntries.has(entry)) - .filter((entry) => hasValidResourceEntryDuration(entry) && hasValidResourceEntryTimings(entry)) - .filter((entry) => - isBetween( - entry, - request.startClocks.relative, - endTime({ startTime: request.startClocks.relative, duration: request.duration }) - ) - ) - - if (candidates.length === 1) { - alreadyMatchedEntries.add(candidates[0]) - - return candidates[0].toJSON() as RumPerformanceResourceTiming - } - - return -} - -function endTime(timing: Timing) { - return addDuration(timing.startTime, timing.duration) -} - -function isBetween(timing: Timing, start: RelativeTime, end: RelativeTime) { - const errorMargin = 1 as Duration - return timing.startTime >= start - errorMargin && endTime(timing) <= addDuration(end, errorMargin) -} diff --git a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts index f1cb36ed71..2224bbcbe7 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.spec.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.spec.ts @@ -10,11 +10,10 @@ import { } from '@datadog/browser-core' import { replaceMockable, registerCleanupTask } from '@datadog/browser-core/test' import { resetExperimentalFeatures } from '@datadog/browser-core/src/tools/experimentalFeatures' -import type { RumFetchResourceEventDomainContext, RumXhrResourceEventDomainContext } from '../../domainContext.types' +import type { RumResourceEventDomainContext } from '../../domainContext.types' import { collectAndValidateRawRumEvents, createPerformanceEntry, - mockPageStateHistory, mockPerformanceObserver, mockRumConfiguration, } from '../../../test' @@ -33,11 +32,9 @@ import { retrieveInitialDocumentResourceTiming } from './retrieveInitialDocument const HANDLING_STACK_REGEX = /^Error: \n\s+at @/ const baseConfiguration = mockRumConfiguration() -const pageStateHistory = mockPageStateHistory() describe('resourceCollection', () => { let lifeCycle: LifeCycle - let wasInPageStateDuringPeriodSpy: jasmine.Spy let notifyPerformanceEntries: (entries: RumPerformanceEntry[]) => void let rawRumEvents: Array> = [] let taskQueuePushSpy: jasmine.Spy @@ -48,7 +45,7 @@ describe('resourceCollection', () => { const taskQueue = createTaskQueue() replaceMockable(createTaskQueue, () => taskQueue) taskQueuePushSpy = spyOn(taskQueue, 'push') - const startResult = startResourceCollection(lifeCycle, { ...baseConfiguration, ...partialConfig }, pageStateHistory) + const startResult = startResourceCollection(lifeCycle, { ...baseConfiguration, ...partialConfig }) rawRumEvents = collectAndValidateRawRumEvents(lifeCycle) @@ -59,7 +56,6 @@ describe('resourceCollection', () => { beforeEach(() => { ;({ notifyPerformanceEntries } = mockPerformanceObserver()) - wasInPageStateDuringPeriodSpy = spyOn(pageStateHistory, 'wasInPageStateDuringPeriod') }) it('should create resource from performance entry', () => { @@ -104,6 +100,14 @@ describe('resourceCollection', () => { }) expect(rawRumEvents[0].domainContext).toEqual({ performanceEntry, + isManual: false, + isAborted: false, + handlingStack: undefined, + requestInit: undefined, + requestInput: undefined, + response: undefined, + error: undefined, + xhr: undefined, }) }) @@ -154,6 +158,11 @@ describe('resourceCollection', () => { performanceEntry: jasmine.any(Object), isAborted: false, handlingStack: jasmine.stringMatching(HANDLING_STACK_REGEX), + isManual: false, + requestInit: undefined, + requestInput: undefined, + response: undefined, + error: undefined, }) }) @@ -317,47 +326,53 @@ describe('resourceCollection', () => { }) }) - describe('with trackEarlyRequests enabled', () => { - it('creates a resource from a performance entry without a matching request', () => { - setupResourceCollection({ trackResources: true, trackEarlyRequests: true }) + it('creates a resource from a performance entry without a matching request', () => { + setupResourceCollection({ trackResources: true }) - notifyPerformanceEntries([ - createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { - initiatorType: RequestType.FETCH, - }), - ]) - runTasks() + notifyPerformanceEntries([ + createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { + initiatorType: RequestType.FETCH, + }), + ]) + runTasks() - expect(rawRumEvents.length).toBe(1) - expect(rawRumEvents[0].startClocks.relative).toBe(200 as RelativeTime) - expect(rawRumEvents[0].rawRumEvent).toEqual({ - date: jasmine.any(Number), - resource: { - id: jasmine.any(String), - duration: (100 * 1e6) as ServerDuration, - method: undefined, - status_code: 200, - delivery_type: 'cache', - protocol: 'HTTP/1.0', - type: ResourceType.FETCH, - url: 'https://resource.com/valid', - render_blocking_status: 'non-blocking', - size: undefined, - encoded_body_size: undefined, - decoded_body_size: undefined, - transfer_size: undefined, - download: { duration: 100000000 as ServerDuration, start: 0 as ServerDuration }, - first_byte: { duration: 0 as ServerDuration, start: 0 as ServerDuration }, - graphql: undefined, - }, - type: RumEventType.RESOURCE, - _dd: { - discarded: false, - }, - }) - expect(rawRumEvents[0].domainContext).toEqual({ - performanceEntry: jasmine.any(Object), - }) + expect(rawRumEvents.length).toBe(1) + expect(rawRumEvents[0].startClocks.relative).toBe(200 as RelativeTime) + expect(rawRumEvents[0].rawRumEvent).toEqual({ + date: jasmine.any(Number), + resource: { + id: jasmine.any(String), + duration: (100 * 1e6) as ServerDuration, + method: undefined, + status_code: 200, + delivery_type: 'cache', + protocol: 'HTTP/1.0', + type: ResourceType.FETCH, + url: 'https://resource.com/valid', + render_blocking_status: 'non-blocking', + size: undefined, + encoded_body_size: undefined, + decoded_body_size: undefined, + transfer_size: undefined, + download: { duration: 100000000 as ServerDuration, start: 0 as ServerDuration }, + first_byte: { duration: 0 as ServerDuration, start: 0 as ServerDuration }, + graphql: undefined, + }, + type: RumEventType.RESOURCE, + _dd: { + discarded: false, + }, + }) + expect(rawRumEvents[0].domainContext).toEqual({ + performanceEntry: jasmine.any(Object), + isManual: false, + isAborted: false, + handlingStack: undefined, + requestInit: undefined, + requestInput: undefined, + response: undefined, + error: undefined, + xhr: undefined, }) }) @@ -412,19 +427,6 @@ describe('resourceCollection', () => { }) }) - it('should not have a duration if a frozen state happens during the request and no performance entry matches', () => { - setupResourceCollection() - wasInPageStateDuringPeriodSpy.and.returnValue(true) - - notifyRequest({ - // For now, this behavior only happens when there is no performance entry matching the request - notifyPerformanceEntry: false, - }) - - const rawRumResourceEventFetch = rawRumEvents[0].rawRumEvent as RawRumResourceEvent - expect(rawRumResourceEventFetch.resource.duration).toBeUndefined() - }) - it('should create resource from completed fetch request', () => { setupResourceCollection() const response = new Response() @@ -478,6 +480,8 @@ describe('resourceCollection', () => { error: undefined, isAborted: false, handlingStack: jasmine.stringMatching(HANDLING_STACK_REGEX), + isManual: false, + xhr: undefined, }) }) ;[null, undefined, 42, {}].forEach((input: any) => { @@ -490,7 +494,7 @@ describe('resourceCollection', () => { }) expect(rawRumEvents.length).toBe(1) - expect((rawRumEvents[0].domainContext as RumFetchResourceEventDomainContext).requestInput).toBe(input) + expect((rawRumEvents[0].domainContext as RumResourceEventDomainContext).requestInput).toBe(input) }) }) @@ -956,7 +960,7 @@ describe('resourceCollection', () => { setupResourceCollection() const response = new Response() notifyRequest({ request: { type: RequestType.FETCH, response } }) - const domainContext = rawRumEvents[0].domainContext as RumFetchResourceEventDomainContext + const domainContext = rawRumEvents[0].domainContext as RumResourceEventDomainContext expect(domainContext.handlingStack).toMatch(HANDLING_STACK_REGEX) }) @@ -966,7 +970,7 @@ describe('resourceCollection', () => { const xhr = new XMLHttpRequest() notifyRequest({ request: { type: RequestType.XHR, xhr } }) - const domainContext = rawRumEvents[0].domainContext as RumXhrResourceEventDomainContext + const domainContext = rawRumEvents[0].domainContext as RumResourceEventDomainContext expect(domainContext.handlingStack).toMatch(HANDLING_STACK_REGEX) }) @@ -1024,6 +1028,7 @@ describe('resourceCollection', () => { notifyPerformanceEntries([ createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { initiatorType: requestCompleteEvent.type === RequestType.FETCH ? 'fetch' : 'xmlhttprequest', + name: requestCompleteEvent.url, ...performanceEntryOverrides, }), ]) diff --git a/packages/rum-core/src/domain/resource/resourceCollection.ts b/packages/rum-core/src/domain/resource/resourceCollection.ts index 601a18c34b..3df025ba0e 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.ts @@ -1,9 +1,7 @@ -import type { ClocksState, Duration, MatchOption } from '@datadog/browser-core' +import type { MatchOption } from '@datadog/browser-core' import { combine, generateUUID, - RequestType, - ResourceType, toServerDuration, relativeToClocks, createTaskQueue, @@ -14,25 +12,19 @@ import { safeTruncate, display, addTelemetryDebug, + RequestType, } from '@datadog/browser-core' import type { RumConfiguration } from '../configuration' import type { RumPerformanceResourceTiming } from '../../browser/performanceObservable' import { RumPerformanceEntryType, createPerformanceObservable } from '../../browser/performanceObservable' -import type { - RumXhrResourceEventDomainContext, - RumFetchResourceEventDomainContext, - RumOtherResourceEventDomainContext, -} from '../../domainContext.types' +import type { RumResourceEventDomainContext } from '../../domainContext.types' import type { NetworkHeaders, RawRumResourceEvent, ResourceRequest, ResourceResponse } from '../../rawRumEvent.types' import { RumEventType } from '../../rawRumEvent.types' import type { RawRumEventCollectedData, LifeCycle } from '../lifeCycle' import { LifeCycleEventType } from '../lifeCycle' import type { RequestCompleteEvent } from '../requestCollection' -import type { PageStateHistory } from '../contexts/pageStateHistory' -import { PageState } from '../contexts/pageStateHistory' import { createSpanIdentifier } from '../tracing/identifier' import { startEventTracker } from '../eventTracker' -import { matchRequestResourceEntry } from './matchRequestResourceEntry' import { computeResourceEntryDetails, computeResourceEntryDuration, @@ -51,36 +43,21 @@ import { extractGraphQlMetadata, findGraphQlConfiguration } from './graphql' import type { ManualResourceData } from './trackManualResources' import { trackManualResources } from './trackManualResources' -export function startResourceCollection( - lifeCycle: LifeCycle, - configuration: RumConfiguration, - pageStateHistory: PageStateHistory -) { +export function startResourceCollection(lifeCycle: LifeCycle, configuration: RumConfiguration) { const taskQueue = mockable(createTaskQueue)() - let requestRegistry: RequestRegistry | undefined - const isEarlyRequestCollectionEnabled = configuration.trackEarlyRequests - - if (isEarlyRequestCollectionEnabled) { - requestRegistry = createRequestRegistry(lifeCycle) - } else { - lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, (request: RequestCompleteEvent) => { - handleResource(() => processRequest(request, configuration, pageStateHistory)) - }) - } + const requestRegistry = createRequestRegistry(lifeCycle) const performanceResourceSubscription = createPerformanceObservable(configuration, { type: RumPerformanceEntryType.RESOURCE, buffered: true, }).subscribe((entries) => { for (const entry of entries) { - if (isEarlyRequestCollectionEnabled || !isResourceEntryRequestType(entry)) { - handleResource(() => processResourceEntry(entry, configuration, pageStateHistory, requestRegistry)) - } + handleResource(() => assembleResource(entry, requestRegistry, configuration)) } }) mockable(retrieveInitialDocumentResourceTiming)(configuration, (timing) => { - handleResource(() => processResourceEntry(timing, configuration, pageStateHistory, requestRegistry)) + handleResource(() => assembleResource(timing, requestRegistry, configuration)) }) function handleResource(computeRawEvent: () => RawRumEventCollectedData | undefined) { @@ -106,74 +83,42 @@ export function startResourceCollection( } } -function processRequest( - request: RequestCompleteEvent, - configuration: RumConfiguration, - pageStateHistory: PageStateHistory -): RawRumEventCollectedData | undefined { - const matchingTiming = matchRequestResourceEntry(request) - return assembleResource(matchingTiming, request, pageStateHistory, configuration) -} - -function processResourceEntry( - entry: RumPerformanceResourceTiming, - configuration: RumConfiguration, - pageStateHistory: PageStateHistory, - requestRegistry: RequestRegistry | undefined -): RawRumEventCollectedData | undefined { - const matchingRequest = - isResourceEntryRequestType(entry) && requestRegistry ? requestRegistry.getMatchingRequest(entry) : undefined - return assembleResource(entry, matchingRequest, pageStateHistory, configuration) -} - -// TODO: In the future, the `entry` parameter should be required, making things simpler. function assembleResource( - entry: RumPerformanceResourceTiming | undefined, - request: RequestCompleteEvent | undefined, - pageStateHistory: PageStateHistory, + entry: RumPerformanceResourceTiming, + requestRegistry: RequestRegistry, configuration: RumConfiguration ): RawRumEventCollectedData | undefined { - if (!entry && !request) { - return - } - + const request = isResourceEntryRequestType(entry) ? requestRegistry.getMatchingRequest(entry) : undefined const tracingInfo = request ? computeRequestTracingInfo(request, configuration) - : computeResourceEntryTracingInfo(entry!, configuration) + : computeResourceEntryTracingInfo(entry, configuration) if (!configuration.trackResources && !tracingInfo) { return } - const startClocks = entry ? relativeToClocks(entry.startTime) : request!.startClocks - const duration = entry - ? computeResourceEntryDuration(entry) - : computeRequestDuration(pageStateHistory, startClocks, request!.duration) + const startClocks = relativeToClocks(entry.startTime) + const duration = computeResourceEntryDuration(entry) const networkHeaders = isExperimentalFeatureEnabled(ExperimentalFeature.TRACK_RESOURCE_HEADERS) ? computeNetworkHeaders(request, configuration) : undefined - const graphql = request && computeGraphQlMetaData(request, configuration) - const contentTypeFromPerformanceEntry = entry && computeContentTypeFromPerformanceEntry(entry) - const resourceEvent = combine( { date: startClocks.timeStamp, resource: { id: generateUUID(), duration: toServerDuration(duration), - // TODO: in the future when `entry` is required, we can probably only rely on `computeResourceEntryType` - type: request - ? request.type === RequestType.XHR - ? ResourceType.XHR - : ResourceType.FETCH - : computeResourceEntryType(entry!), - method: request ? request.method : undefined, - status_code: request ? request.status : discardZeroStatus(entry!.responseStatus), - url: request ? sanitizeIfLongDataUrl(request.url) : entry!.name, - protocol: entry && computeResourceEntryProtocol(entry), - delivery_type: entry && computeResourceEntryDeliveryType(entry), - graphql, + type: computeResourceEntryType(entry), + method: request?.method, + status_code: request ? request.status : discardZeroStatus(entry.responseStatus), + url: request ? sanitizeIfLongDataUrl(request.url) : entry.name, + protocol: computeResourceEntryProtocol(entry), + delivery_type: computeResourceEntryDeliveryType(entry), + graphql: request && computeGraphQlMetaData(request, configuration), + render_blocking_status: entry.renderBlockingStatus, + ...computeResourceEntrySize(entry), + ...computeResourceEntryDetails(entry), }, type: RumEventType.RESOURCE, _dd: { @@ -181,8 +126,7 @@ function assembleResource( }, }, tracingInfo, - entry && computeResourceEntryMetrics(entry), - contentTypeFromPerformanceEntry, + computeContentTypeFromPerformanceEntry(entry), networkHeaders ) @@ -227,46 +171,19 @@ function computeContentTypeFromPerformanceEntry( } function getResourceDomainContext( - entry: RumPerformanceResourceTiming | undefined, + entry: RumPerformanceResourceTiming, request: RequestCompleteEvent | undefined -): RumFetchResourceEventDomainContext | RumXhrResourceEventDomainContext | RumOtherResourceEventDomainContext { - if (request) { - const baseDomainContext = { - performanceEntry: entry, - isAborted: request.isAborted, - handlingStack: request.handlingStack, - } - - if (request.type === RequestType.XHR) { - return { - xhr: request.xhr!, - ...baseDomainContext, - } - } - return { - requestInput: request.input as RequestInfo, - requestInit: request.init, - response: request.response, - error: request.error, - ...baseDomainContext, - } - } +): RumResourceEventDomainContext { return { - // Currently, at least one of `entry` or `request` must be defined when calling this function. - // So `entry` is guaranteed to be defined here. In the future, when `entry` is required, we can - // remove the `!` assertion. - performanceEntry: entry!, - } -} - -function computeResourceEntryMetrics(entry: RumPerformanceResourceTiming) { - const { renderBlockingStatus } = entry - return { - resource: { - render_blocking_status: renderBlockingStatus, - ...computeResourceEntrySize(entry), - ...computeResourceEntryDetails(entry), - }, + performanceEntry: entry, + isManual: false, + isAborted: request ? request.isAborted : false, + handlingStack: request?.handlingStack, + requestInit: request?.init, + requestInput: request?.input as RequestInfo | undefined, + response: request?.response, + error: request?.error, + xhr: request?.xhr, } } @@ -298,12 +215,6 @@ function computeResourceEntryTracingInfo(entry: RumPerformanceResourceTiming, co } } -function computeRequestDuration(pageStateHistory: PageStateHistory, startClocks: ClocksState, duration: Duration) { - return !pageStateHistory.wasInPageStateDuringPeriod(PageState.FROZEN, startClocks.relative, duration) - ? duration - : undefined -} - /** * The status is 0 for cross-origin resources without CORS headers, so the status is meaningless, and we shouldn't report it * https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus#cross-origin_response_status_codes diff --git a/packages/rum-core/src/domainContext.types.ts b/packages/rum-core/src/domainContext.types.ts index f4de716dcc..06127f1e02 100644 --- a/packages/rum-core/src/domainContext.types.ts +++ b/packages/rum-core/src/domainContext.types.ts @@ -9,11 +9,7 @@ export type RumEventDomainContext = T extends type : T extends typeof RumEventType.ACTION ? RumActionEventDomainContext : T extends typeof RumEventType.RESOURCE - ? - | RumFetchResourceEventDomainContext - | RumXhrResourceEventDomainContext - | RumOtherResourceEventDomainContext - | RumManualResourceEventDomainContext + ? RumResourceEventDomainContext | RumManualResourceEventDomainContext : T extends typeof RumEventType.ERROR ? RumErrorEventDomainContext : T extends typeof RumEventType.LONG_TASK @@ -32,25 +28,16 @@ export interface RumActionEventDomainContext { handlingStack?: string } -export interface RumFetchResourceEventDomainContext { +export interface RumResourceEventDomainContext { + isManual: false + performanceEntry: PerformanceEntry + xhr: XMLHttpRequest | undefined + isAborted: boolean + handlingStack: string | undefined requestInit: RequestInit | undefined - requestInput: RequestInfo + requestInput: RequestInfo | undefined response: Response | undefined error: Error | undefined - performanceEntry: PerformanceEntry | undefined - isAborted: boolean - handlingStack: string | undefined -} - -export interface RumXhrResourceEventDomainContext { - xhr: XMLHttpRequest - performanceEntry: PerformanceEntry | undefined - isAborted: boolean - handlingStack: string | undefined -} - -export interface RumOtherResourceEventDomainContext { - performanceEntry: PerformanceEntry } export interface RumManualResourceEventDomainContext { diff --git a/packages/rum-core/src/index.ts b/packages/rum-core/src/index.ts index 48b331173f..d104289a56 100644 --- a/packages/rum-core/src/index.ts +++ b/packages/rum-core/src/index.ts @@ -15,10 +15,8 @@ export type { export type { RumLongTaskEventDomainContext, RumErrorEventDomainContext, - RumOtherResourceEventDomainContext, RumManualResourceEventDomainContext, - RumXhrResourceEventDomainContext, - RumFetchResourceEventDomainContext, + RumResourceEventDomainContext, RumActionEventDomainContext, RumViewEventDomainContext, RumEventDomainContext, diff --git a/packages/rum-slim/src/entries/main.ts b/packages/rum-slim/src/entries/main.ts index b218fa5ec1..8363bc0715 100644 --- a/packages/rum-slim/src/entries/main.ts +++ b/packages/rum-slim/src/entries/main.ts @@ -56,9 +56,7 @@ export type { RumErrorEventDomainContext, RumActionEventDomainContext, RumVitalEventDomainContext, - RumFetchResourceEventDomainContext, - RumXhrResourceEventDomainContext, - RumOtherResourceEventDomainContext, + RumResourceEventDomainContext, RumLongTaskEventDomainContext, } from '@datadog/browser-rum-core' export { DefaultPrivacyLevel } from '@datadog/browser-core' diff --git a/packages/rum/src/entries/main.ts b/packages/rum/src/entries/main.ts index aaf2ee75cc..b38bea26fc 100644 --- a/packages/rum/src/entries/main.ts +++ b/packages/rum/src/entries/main.ts @@ -66,9 +66,7 @@ export type { RumErrorEventDomainContext, RumActionEventDomainContext, RumVitalEventDomainContext, - RumFetchResourceEventDomainContext, - RumXhrResourceEventDomainContext, - RumOtherResourceEventDomainContext, + RumResourceEventDomainContext, RumLongTaskEventDomainContext, } from '@datadog/browser-rum-core' diff --git a/test/e2e/scenario/rum/resources.scenario.ts b/test/e2e/scenario/rum/resources.scenario.ts index 9fce4e4782..91e8d8a849 100644 --- a/test/e2e/scenario/rum/resources.scenario.ts +++ b/test/e2e/scenario/rum/resources.scenario.ts @@ -68,13 +68,13 @@ test.describe('rum resources', () => { expectToHaveValidTimings(resourceEvent!) }) - createTest('collect fetch requests made before init with trackEarlyRequests') + createTest('collect fetch requests made before init') .withHead(html` `) - .withRum({ trackEarlyRequests: true }) + .withRum() .run(async ({ intakeRegistry, flushEvents }) => { await flushEvents() const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.type === 'fetch') @@ -95,7 +95,12 @@ test.describe('rum resources', () => { test.describe('XHR abort support', () => { createTest('track aborted XHR') .withRum() - .run(async ({ intakeRegistry, flushEvents, page }) => { + .run(async ({ intakeRegistry, flushEvents, page, browserName }) => { + test.skip( + browserName === 'webkit', + 'WebKit does not reporte a PerformanceResourceTiming entry for aborted XHRs' + ) + await page.evaluate( () => new Promise((resolve) => { @@ -198,7 +203,12 @@ test.describe('rum resources', () => { test.describe('fetch abort support', () => { createTest('track aborted fetch') .withRum() - .run(async ({ intakeRegistry, flushEvents, page }) => { + .run(async ({ intakeRegistry, flushEvents, page, browserName }) => { + test.skip( + browserName === 'webkit', + 'WebKit does not reporte a PerformanceResourceTiming entry for aborted fetches' + ) + await page.evaluate( () => new Promise((resolve) => {