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

fix: report requests even without timing #1386

Merged
merged 7 commits into from
Sep 3, 2024
Merged

fix: report requests even without timing #1386

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5344a6d
feat: report requests even without timing
pauldambra Aug 26, 2024
116c715
we're capturing start and end already so can get that if nothing else
pauldambra Aug 26, 2024
673b5c0
wat
pauldambra Aug 26, 2024
a2205fa
double wat
pauldambra Aug 26, 2024
110e3db
Update src/entrypoints/recorder.ts
pauldambra Aug 27, 2024
964cf4f
Merge branch 'main' into feat/report-requests-even-without-timing
pauldambra Aug 30, 2024
efacb0c
Merge branch 'main' into feat/report-requests-even-without-timing
pauldambra Sep 3, 2024
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
117 changes: 77 additions & 40 deletions src/entrypoints/recorder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,17 @@ import { getRecordConsolePlugin } from 'rrweb/es/rrweb/packages/rrweb/src/plugin
// copying here so that we can use it before rrweb adopt it
import type { IWindow, listenerHandler, RecordPlugin } from '@rrweb/types'
import { CapturedNetworkRequest, Headers, InitiatorType, NetworkRecordOptions } from '../types'
import { isArray, isBoolean, isDocument, isFormData, isNull, isNullish, isObject, isString } from '../utils/type-utils'
import {
isArray,
isBoolean,
isDocument,
isFormData,
isNull,
isNullish,
isObject,
isString,
isUndefined,
} from '../utils/type-utils'
import { logger } from '../utils/logger'
import { window } from '../utils/globals'
import { defaultNetworkOptions } from '../extensions/replay/config'
Expand Down Expand Up @@ -68,7 +78,7 @@ function initPerformanceObserver(cb: networkCallback, win: IWindow, options: Req
)
cb({
requests: initialPerformanceEntries.flatMap((entry) =>
prepareRequest(entry, undefined, undefined, {}, true)
prepareRequest({ entry, method: undefined, status: undefined, networkRequest: {}, isInitial: true })
),
isInitial: true,
})
Expand All @@ -92,7 +102,9 @@ function initPerformanceObserver(cb: networkCallback, win: IWindow, options: Req
)

cb({
requests: performanceEntries.flatMap((entry) => prepareRequest(entry, undefined, undefined, {})),
requests: performanceEntries.flatMap((entry) =>
prepareRequest({ entry, method: undefined, status: undefined, networkRequest: {} })
),
})
})
// compat checked earlier
Expand Down Expand Up @@ -139,8 +151,8 @@ async function getRequestPerformanceEntry(
win: IWindow,
initiatorType: string,
url: string,
after?: number,
before?: number,
start?: number,
end?: number,
attempt = 0
): Promise<PerformanceResourceTiming | null> {
if (attempt > 10) {
Expand All @@ -153,12 +165,12 @@ async function getRequestPerformanceEntry(
(entry) =>
isResourceTiming(entry) &&
entry.initiatorType === initiatorType &&
(!after || entry.startTime >= after) &&
(!before || entry.startTime <= before)
(isUndefined(start) || entry.startTime >= start) &&
(isUndefined(end) || entry.startTime <= end)
)
if (!performanceEntry) {
await new Promise((resolve) => setTimeout(resolve, 50 * attempt))
return getRequestPerformanceEntry(win, initiatorType, url, after, before, attempt + 1)
return getRequestPerformanceEntry(win, initiatorType, url, start, end, attempt + 1)
}
return performanceEntry
}
Expand Down Expand Up @@ -242,8 +254,8 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required<Ne
// eslint-disable-next-line compat/compat
const req = new Request(url)
const networkRequest: Partial<CapturedNetworkRequest> = {}
let after: number | undefined
let before: number | undefined
let start: number | undefined
let end: number | undefined

const requestHeaders: Headers = {}
const originalSetRequestHeader = xhr.setRequestHeader.bind(xhr)
Expand All @@ -267,15 +279,15 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required<Ne
) {
networkRequest.requestBody = _tryReadXHRBody({ body, options, url })
}
after = win.performance.now()
start = win.performance.now()
return originalSend(body)
}

xhr.addEventListener('readystatechange', () => {
if (xhr.readyState !== xhr.DONE) {
return
}
before = win.performance.now()
end = win.performance.now()
const responseHeaders: Headers = {}
const rawHeaders = xhr.getAllResponseHeaders()
const headers = rawHeaders.trim().split(/[\r\n]+/)
Expand All @@ -300,12 +312,16 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required<Ne
) {
networkRequest.responseBody = _tryReadXHRBody({ body: xhr.response, options, url })
}
getRequestPerformanceEntry(win, 'xmlhttprequest', req.url, after, before)
getRequestPerformanceEntry(win, 'xmlhttprequest', req.url, start, end)
.then((entry) => {
if (isNull(entry)) {
return
}
const requests = prepareRequest(entry, req.method, xhr?.status, networkRequest)
const requests = prepareRequest({
entry,
method: req.method,
status: xhr?.status,
networkRequest,
start,
end,
})
cb({ requests })
})
.catch(() => {
Expand All @@ -326,34 +342,50 @@ function initXhrObserver(cb: networkCallback, win: IWindow, options: Required<Ne
* NB PerformanceNavigationTiming extends PerformanceResourceTiming
* Here we don't care which interface it implements as both expose `serverTimings`
*/
const exposesServerTiming = (event: PerformanceEntry): event is PerformanceResourceTiming =>
event.entryType === 'navigation' || event.entryType === 'resource'

function prepareRequest(
entry: PerformanceResourceTiming,
method: string | undefined,
status: number | undefined,
networkRequest: Partial<CapturedNetworkRequest>,
const exposesServerTiming = (event: PerformanceEntry | null): event is PerformanceResourceTiming =>
!!event && (event.entryType === 'navigation' || event.entryType === 'resource')
pauldambra marked this conversation as resolved.
Show resolved Hide resolved

function prepareRequest({
entry,
method,
status,
networkRequest,
isInitial,
start,
end,
}: {
entry: PerformanceResourceTiming | null
method: string | undefined
status: number | undefined
networkRequest: Partial<CapturedNetworkRequest>
isInitial?: boolean
): CapturedNetworkRequest[] {
start?: number
end?: number
Comment on lines +362 to +363
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to make these optional? Looks like this method is only called with the results of window.performance.now() as inputs for these values which always returns a number

Copy link
Member Author

Choose a reason for hiding this comment

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

only that the values start undefined... really you shouldn't ever get to the point of needing them and the value not be present but 🙈

}): CapturedNetworkRequest[] {
start = entry ? entry.startTime : start
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
end = entry ? entry.responseEnd : end

// kudos to sentry javascript sdk for excellent background on why to use Date.now() here
// https://github.com/getsentry/sentry-javascript/blob/e856e40b6e71a73252e788cd42b5260f81c9c88e/packages/utils/src/time.ts#L70
// can't start observer if performance.now() is not available
// eslint-disable-next-line compat/compat
const timeOrigin = Math.floor(Date.now() - performance.now())
// clickhouse can't ingest timestamps that are floats
// (in this case representing fractions of a millisecond we don't care about anyway)
const timestamp = Math.floor(timeOrigin + entry.startTime)
// use timeOrigin if we really can't gather a start time
const timestamp = Math.floor(timeOrigin + (start || 0))

const entryJSON = entry ? entry.toJSON() : {}

const requests: CapturedNetworkRequest[] = [
{
...entry.toJSON(),
startTime: Math.round(entry.startTime),
endTime: Math.round(entry.responseEnd),
...entryJSON,
startTime: isUndefined(start) ? undefined : Math.round(start),
endTime: isUndefined(end) ? undefined : Math.round(end),
timeOrigin,
timestamp,
method: method,
initiatorType: entry.initiatorType as InitiatorType,
initiatorType: entry ? (entry.initiatorType as InitiatorType) : undefined,
status,
requestHeaders: networkRequest.requestHeaders,
requestBody: networkRequest.requestBody,
Expand Down Expand Up @@ -485,8 +517,9 @@ function initFetchObserver(
const req = new Request(url, init)
let res: Response | undefined
const networkRequest: Partial<CapturedNetworkRequest> = {}
let after: number | undefined
let before: number | undefined
let start: number | undefined
let end: number | undefined

try {
const requestHeaders: Headers = {}
req.headers.forEach((value, header) => {
Expand All @@ -506,9 +539,9 @@ function initFetchObserver(
networkRequest.requestBody = await _tryReadRequestBody({ r: req, options, url })
}

after = win.performance.now()
start = win.performance.now()
res = await originalFetch(req)
before = win.performance.now()
end = win.performance.now()

const responseHeaders: Headers = {}
res.headers.forEach((value, header) => {
Expand All @@ -530,12 +563,16 @@ function initFetchObserver(

return res
} finally {
getRequestPerformanceEntry(win, 'fetch', req.url, after, before)
getRequestPerformanceEntry(win, 'fetch', req.url, start, end)
.then((entry) => {
if (isNull(entry)) {
return
}
const requests = prepareRequest(entry, req.method, res?.status, networkRequest)
const requests = prepareRequest({
entry,
method: req.method,
status: res?.status,
networkRequest,
start,
end,
})
cb({ requests })
})
.catch(() => {
Expand Down
Loading