Skip to content

Commit

Permalink
🐛[RUMF-650] exclude intake request from performance/request collection (
Browse files Browse the repository at this point in the history
#528)

* 🐛[RUMF-650] exclude intake request from performance/request collection

* 👌 introduce dedicated types for beforeSend context
  • Loading branch information
bcaudan authored Sep 11, 2020
1 parent b832cc6 commit b4f0fa2
Show file tree
Hide file tree
Showing 17 changed files with 252 additions and 230 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/errorCollection.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Configuration, isIntakeRequest } from './configuration'
import { FetchContext, resetFetchProxy, startFetchProxy } from './fetchProxy'
import { FetchCompleteContext, resetFetchProxy, startFetchProxy } from './fetchProxy'
import { monitor } from './internalMonitoring'
import { Observable } from './observable'
import { computeStackTrace, Handler, report, StackFrame, StackTrace } from './tracekit'
import { jsonStringify, ONE_MINUTE, RequestType } from './utils'
import { resetXhrProxy, startXhrProxy, XhrContext } from './xhrProxy'
import { resetXhrProxy, startXhrProxy, XhrCompleteContext } from './xhrProxy'

export interface ErrorMessage {
startTime: number
Expand Down Expand Up @@ -157,7 +157,7 @@ export function trackNetworkError(configuration: Configuration, errorObservable:
startXhrProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.XHR, context))
startFetchProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.FETCH, context))

function handleCompleteRequest(type: RequestType, request: XhrContext | FetchContext) {
function handleCompleteRequest(type: RequestType, request: XhrCompleteContext | FetchCompleteContext) {
if (!isIntakeRequest(request.url, configuration) && (isRejected(request) || isServerError(request))) {
errorObservable.notify({
context: {
Expand Down
43 changes: 26 additions & 17 deletions packages/core/src/fetchProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,54 @@ import { monitor } from './internalMonitoring'
import { computeStackTrace } from './tracekit'
import { normalizeUrl } from './urlPolyfill'

export interface FetchProxy<T extends FetchContext = FetchContext> {
beforeSend: (callback: (context: Partial<T>) => void) => void
onRequestComplete: (callback: (context: T) => void) => void
export interface FetchProxy<
StartContext extends FetchStartContext = FetchStartContext,
CompleteContext extends FetchCompleteContext = FetchCompleteContext
> {
beforeSend: (callback: (context: StartContext) => void) => void
onRequestComplete: (callback: (context: CompleteContext) => void) => void
}

export interface FetchContext {
export interface FetchStartContext {
method: string
startTime: number
init?: RequestInit
duration: number
url: string
status: number
response: string
responseType?: string

/**
* allow clients to enhance the context
*/
[key: string]: unknown
}

export interface FetchCompleteContext extends FetchStartContext {
duration: number
status: number
response: string
responseType?: string
}

let fetchProxySingleton: FetchProxy | undefined
let originalFetch: typeof window.fetch
const beforeSendCallbacks: Array<(fetch: Partial<FetchContext>) => void> = []
const onRequestCompleteCallbacks: Array<(fetch: FetchContext) => void> = []
const beforeSendCallbacks: Array<(fetch: FetchStartContext) => void> = []
const onRequestCompleteCallbacks: Array<(fetch: FetchCompleteContext) => void> = []

export function startFetchProxy<T extends FetchContext = FetchContext>(): FetchProxy<T> {
export function startFetchProxy<
StartContext extends FetchStartContext = FetchStartContext,
CompleteContext extends FetchCompleteContext = FetchCompleteContext
>(): FetchProxy<StartContext, CompleteContext> {
if (!fetchProxySingleton) {
proxyFetch()
fetchProxySingleton = {
beforeSend(callback: (context: Partial<FetchContext>) => void) {
beforeSend(callback: (context: FetchStartContext) => void) {
beforeSendCallbacks.push(callback)
},
onRequestComplete(callback: (context: FetchContext) => void) {
onRequestComplete(callback: (context: FetchCompleteContext) => void) {
onRequestCompleteCallbacks.push(callback)
},
}
}
return fetchProxySingleton as FetchProxy<T>
return fetchProxySingleton as FetchProxy<StartContext, CompleteContext>
}

export function resetFetchProxy() {
Expand All @@ -66,7 +75,7 @@ function proxyFetch() {
const url = normalizeUrl((typeof input === 'object' && input.url) || (input as string))
const startTime = performance.now()

const context: Partial<FetchContext> = {
const context: FetchStartContext = {
init,
method,
startTime,
Expand All @@ -80,7 +89,7 @@ function proxyFetch() {
context.status = 0
context.response = toStackTraceString(computeStackTrace(response))

onRequestCompleteCallbacks.forEach((callback) => callback(context as FetchContext))
onRequestCompleteCallbacks.forEach((callback) => callback(context as FetchCompleteContext))
} else if ('status' in response) {
let text: string
try {
Expand All @@ -92,7 +101,7 @@ function proxyFetch() {
context.responseType = response.type
context.status = response.status

onRequestCompleteCallbacks.forEach((callback) => callback(context as FetchContext))
onRequestCompleteCallbacks.forEach((callback) => callback(context as FetchCompleteContext))
}
}
beforeSendCallbacks.forEach((callback) => callback(context))
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export { HttpRequest, Batch } from './transport'
export * from './urlPolyfill'
export * from './utils'
export { areCookiesAuthorized, getCookie, setCookie, COOKIE_ACCESS_DELAY } from './cookie'
export { startXhrProxy, XhrContext, XhrProxy } from './xhrProxy'
export { startFetchProxy, FetchContext, FetchProxy } from './fetchProxy'
export { startXhrProxy, XhrCompleteContext, XhrStartContext, XhrProxy } from './xhrProxy'
export { startFetchProxy, FetchCompleteContext, FetchStartContext, FetchProxy } from './fetchProxy'

export * from './specHelper'
40 changes: 25 additions & 15 deletions packages/core/src/xhrProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,56 @@ import { monitor } from './internalMonitoring'
import { normalizeUrl } from './urlPolyfill'

interface BrowserXHR extends XMLHttpRequest {
_datadog_xhr: Partial<XhrContext>
_datadog_xhr: XhrStartContext
}

export interface XhrProxy<T extends XhrContext = XhrContext> {
beforeSend: (callback: (context: Partial<T>, xhr: XMLHttpRequest) => void) => void
onRequestComplete: (callback: (context: T) => void) => void
export interface XhrProxy<
StartContext extends XhrStartContext = XhrStartContext,
CompleteContext extends XhrCompleteContext = XhrCompleteContext
> {
beforeSend: (callback: (context: StartContext, xhr: XMLHttpRequest) => void) => void
onRequestComplete: (callback: (context: CompleteContext) => void) => void
}

export interface XhrContext {
export interface XhrStartContext {
method: string
url: string
startTime: number
duration: number
status: number
response: string | undefined

/**
* allow clients to enhance the context
*/
[key: string]: unknown
}

export interface XhrCompleteContext extends XhrStartContext {
duration: number
status: number
response: string | undefined
}

let xhrProxySingleton: XhrProxy | undefined
const beforeSendCallbacks: Array<(context: Partial<XhrContext>, xhr: XMLHttpRequest) => void> = []
const onRequestCompleteCallbacks: Array<(context: XhrContext) => void> = []
const beforeSendCallbacks: Array<(context: XhrStartContext, xhr: XMLHttpRequest) => void> = []
const onRequestCompleteCallbacks: Array<(context: XhrCompleteContext) => void> = []
let originalXhrOpen: typeof XMLHttpRequest.prototype.open
let originalXhrSend: typeof XMLHttpRequest.prototype.send

export function startXhrProxy<T extends XhrContext = XhrContext>(): XhrProxy<T> {
export function startXhrProxy<
StartContext extends XhrStartContext = XhrStartContext,
CompleteContext extends XhrCompleteContext = XhrCompleteContext
>(): XhrProxy<StartContext, CompleteContext> {
if (!xhrProxySingleton) {
proxyXhr()
xhrProxySingleton = {
beforeSend(callback: (context: Partial<XhrContext>, xhr: XMLHttpRequest) => void) {
beforeSend(callback: (context: XhrStartContext, xhr: XMLHttpRequest) => void) {
beforeSendCallbacks.push(callback)
},
onRequestComplete(callback: (context: XhrContext) => void) {
onRequestComplete(callback: (context: XhrCompleteContext) => void) {
onRequestCompleteCallbacks.push(callback)
},
}
}
return xhrProxySingleton as XhrProxy<T>
return xhrProxySingleton as XhrProxy<StartContext, CompleteContext>
}

export function resetXhrProxy() {
Expand All @@ -62,6 +71,7 @@ function proxyXhr() {
XMLHttpRequest.prototype.open = monitor(function(this: BrowserXHR, method: string, url: string) {
this._datadog_xhr = {
method,
startTime: -1, // computed in send call
url: normalizeUrl(url),
}
return originalXhrOpen.apply(this, arguments as any)
Expand Down Expand Up @@ -94,7 +104,7 @@ function proxyXhr() {
this._datadog_xhr.response = this.response as string | undefined
this._datadog_xhr.status = this.status

onRequestCompleteCallbacks.forEach((callback) => callback(this._datadog_xhr as XhrContext))
onRequestCompleteCallbacks.forEach((callback) => callback(this._datadog_xhr as XhrCompleteContext))
}

this.addEventListener('loadend', monitor(reportXhr))
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/fetchProxy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { FetchContext, FetchProxy, resetFetchProxy, startFetchProxy } from '../src/fetchProxy'
import { FetchCompleteContext, FetchProxy, resetFetchProxy, startFetchProxy } from '../src/fetchProxy'
import { FetchStub, FetchStubManager, FetchStubPromise, isIE, stubFetch } from '../src/specHelper'

describe('fetch proxy', () => {
const FAKE_URL = 'http://fake-url/'
let fetchStub: (input: RequestInfo, init?: RequestInit) => FetchStubPromise
let fetchStubManager: FetchStubManager
let completeSpy: jasmine.Spy<(context: FetchContext) => void>
let completeSpy: jasmine.Spy<(context: FetchCompleteContext) => void>
let fetchProxy: FetchProxy

function getRequest(index: number) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/xhrProxy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { withXhr } from '../src'
import { resetXhrProxy, startXhrProxy, XhrContext, XhrProxy } from '../src/xhrProxy'
import { resetXhrProxy, startXhrProxy, XhrCompleteContext, XhrProxy } from '../src/xhrProxy'

describe('xhr proxy', () => {
let completeSpy: jasmine.Spy<(context: XhrContext) => void>
let completeSpy: jasmine.Spy<(context: XhrCompleteContext) => void>
let xhrProxy: XhrProxy

function getRequest(index: number) {
Expand Down
31 changes: 19 additions & 12 deletions packages/rum/src/performanceCollection.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { DOM_EVENT, getRelativeTime, isNumber, monitor } from '@datadog/browser-core'
import { Configuration, DOM_EVENT, getRelativeTime, isNumber, monitor } from '@datadog/browser-core'

import { getDocumentTraceId } from './getDocumentTraceId'
import { LifeCycle, LifeCycleEventType } from './lifeCycle'
import { FAKE_INITIAL_DOCUMENT } from './resourceUtils'
import { FAKE_INITIAL_DOCUMENT, isAllowedRequestUrl } from './resourceUtils'

interface BrowserWindow extends Window {
PerformanceObserver?: PerformanceObserver
Expand Down Expand Up @@ -67,17 +67,17 @@ function supportPerformanceNavigationTimingEvent() {
)
}

export function startPerformanceCollection(lifeCycle: LifeCycle) {
export function startPerformanceCollection(lifeCycle: LifeCycle, configuration: Configuration) {
retrieveInitialDocumentResourceTiming((timing) => {
handleRumPerformanceEntry(lifeCycle, timing)
handleRumPerformanceEntry(lifeCycle, configuration, timing)
})

if (supportPerformanceObject()) {
handlePerformanceEntries(lifeCycle, performance.getEntries())
handlePerformanceEntries(lifeCycle, configuration, performance.getEntries())
}
if ((window as BrowserWindow).PerformanceObserver) {
const observer = new PerformanceObserver(
monitor((entries) => handlePerformanceEntries(lifeCycle, entries.getEntries()))
monitor((entries) => handlePerformanceEntries(lifeCycle, configuration, entries.getEntries()))
)
const entryTypes = ['resource', 'navigation', 'longtask']

Expand All @@ -96,7 +96,7 @@ export function startPerformanceCollection(lifeCycle: LifeCycle) {
}
if (!supportPerformanceNavigationTimingEvent()) {
retrieveNavigationTiming((timing) => {
handleRumPerformanceEntry(lifeCycle, timing)
handleRumPerformanceEntry(lifeCycle, configuration, timing)
})
}
}
Expand Down Expand Up @@ -171,24 +171,31 @@ function computeRelativePerformanceTiming() {
return result as PerformanceTiming
}

function handlePerformanceEntries(lifeCycle: LifeCycle, entries: PerformanceEntry[]) {
function handlePerformanceEntries(lifeCycle: LifeCycle, configuration: Configuration, entries: PerformanceEntry[]) {
entries.forEach((entry) => {
if (
entry.entryType === 'resource' ||
entry.entryType === 'navigation' ||
entry.entryType === 'paint' ||
entry.entryType === 'longtask'
) {
handleRumPerformanceEntry(lifeCycle, entry as RumPerformanceEntry)
handleRumPerformanceEntry(lifeCycle, configuration, entry as RumPerformanceEntry)
}
})
}

function handleRumPerformanceEntry(lifeCycle: LifeCycle, entry: RumPerformanceEntry) {
// Exclude incomplete navigation entries by filtering out those who have a loadEventEnd at 0
if (entry.entryType === 'navigation' && entry.loadEventEnd <= 0) {
function handleRumPerformanceEntry(lifeCycle: LifeCycle, configuration: Configuration, entry: RumPerformanceEntry) {
if (isIncompleteNavigation(entry) || isForbiddenResource(configuration, entry)) {
return
}

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, entry)
}

function isIncompleteNavigation(entry: RumPerformanceEntry) {
return entry.entryType === 'navigation' && entry.loadEventEnd <= 0
}

function isForbiddenResource(configuration: Configuration, entry: RumPerformanceEntry) {
return entry.entryType === 'resource' && !isAllowedRequestUrl(configuration, entry.name)
}
Loading

0 comments on commit b4f0fa2

Please sign in to comment.