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.

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

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

Merged
merged 3 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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