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-909] add beforeSend context #883

Merged
merged 21 commits into from
Jun 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2545afe
[RUMF-909] 🏷️ introduce a data type for RawRumEvent collection lifecy…
BenoitZugmeyer Jun 3, 2021
7764baf
[RUMF-909] ♻️ factorize assembly event notification
BenoitZugmeyer Jun 3, 2021
d955ac7
[RUMF-909] introduce an empty domainContext
BenoitZugmeyer Jun 3, 2021
95c6f76
[RUMF-909] implement error domain context
BenoitZugmeyer Jun 3, 2021
4a08cfb
[RUMF-909] implement action domain context
BenoitZugmeyer Jun 3, 2021
77c0916
[RUMF-909] implement long task domain context
BenoitZugmeyer Jun 3, 2021
958e318
[RUMF-909] rename request context 'response' to 'responseText'
BenoitZugmeyer Jun 4, 2021
43b52a5
[RUMF-909] implement resource domain context (WIP)
BenoitZugmeyer Jun 3, 2021
37c1644
[RUMF-909] implement view domain context
BenoitZugmeyer Jun 3, 2021
ca9e9d3
✨ [RUMF-909] pass domain context to the beforeSend callback
BenoitZugmeyer May 26, 2021
d3e2519
🏷️ expose PerformanceEntry without `toJSON` values
BenoitZugmeyer Jun 8, 2021
7ffc2cd
👌 remove parametric types
BenoitZugmeyer Jun 8, 2021
c0ced56
👌 add input, init and error to fetch context
BenoitZugmeyer Jun 8, 2021
3aafc59
👌 rename 'cloneLocation'
BenoitZugmeyer Jun 8, 2021
b6c61cf
👌 add a non-Error test case in errorCollection
BenoitZugmeyer Jun 8, 2021
5be2e79
Merge remote-tracking branch 'origin/main' into benoit/add-beforesend…
BenoitZugmeyer Jun 8, 2021
e61aaee
👌 change ReadonlyLocation back to Readonly<Location>
BenoitZugmeyer Jun 8, 2021
d4ad427
👌 use type inference to improve typings internally
BenoitZugmeyer Jun 9, 2021
d01dcae
👌 remove prefixes from 'fetch' context
BenoitZugmeyer Jun 9, 2021
51063da
use `toJSON` for 'other' resources performanceEntry
BenoitZugmeyer Jun 9, 2021
64f6991
👌 remove `isinstance PerformanceEntry` check for longtask
BenoitZugmeyer Jun 9, 2021
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
13 changes: 8 additions & 5 deletions packages/core/src/browser/fetchProxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('fetch proxy', () => {
expect(request.method).toEqual('GET')
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(500)
expect(request.response).toEqual('fetch error')
expect(request.responseText).toEqual('fetch error')
expect(request.isAborted).toBe(false)
done()
})
Expand All @@ -50,8 +50,9 @@ describe('fetch proxy', () => {
expect(request.method).toEqual('GET')
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(0)
expect(request.response).toMatch(/Error: fetch error/)
expect(request.responseText).toMatch(/Error: fetch error/)
expect(request.isAborted).toBe(false)
expect(request.error).toEqual(new Error('fetch error'))
done()
})
})
Expand All @@ -64,8 +65,9 @@ describe('fetch proxy', () => {
expect(request.method).toEqual('GET')
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(0)
expect(request.response).toContain('AbortError: The user aborted a request')
expect(request.responseText).toContain('AbortError: The user aborted a request')
expect(request.isAborted).toBe(true)
expect(request.error).toEqual(new DOMException('The user aborted a request', 'AbortError'))
done()
})
})
Expand All @@ -79,8 +81,9 @@ describe('fetch proxy', () => {
expect(request.method).toEqual('GET')
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(200)
expect(request.response).toMatch(/Error: locked/)
expect(request.responseText).toMatch(/Error: locked/)
expect(request.isAborted).toBe(false)
expect(request.error).toBeUndefined()
done()
})
})
Expand All @@ -107,7 +110,7 @@ describe('fetch proxy', () => {
expect(request.method).toEqual('GET')
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(400)
expect(request.response).toEqual('Not found')
expect(request.responseText).toEqual('Not found')
expect(request.isAborted).toBe(false)
done()
})
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/browser/fetchProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ export interface FetchStartContext {
export interface FetchCompleteContext extends FetchStartContext {
duration: Duration
status: number
response: string
response?: Response
responseText: string
responseType?: string
isAborted: boolean
error?: Error
}

let fetchProxySingleton: FetchProxy | undefined
Expand Down Expand Up @@ -111,8 +113,9 @@ function afterSend(responsePromise: Promise<Response>, context: FetchStartContex

if ('stack' in response || response instanceof Error) {
context.status = 0
context.response = toStackTraceString(computeStackTrace(response))
context.responseText = toStackTraceString(computeStackTrace(response))
context.isAborted = response instanceof DOMException && response.code === DOMException.ABORT_ERR
context.error = response

onRequestCompleteCallbacks.forEach((callback) => callback(context as FetchCompleteContext))
} else if ('status' in response) {
Expand All @@ -122,7 +125,8 @@ function afterSend(responsePromise: Promise<Response>, context: FetchStartContex
} catch (e) {
text = `Unable to retrieve response: ${e as string}`
}
context.response = text
context.response = response
context.responseText = text
context.responseType = response.type
context.status = response.status
context.isAborted = false
Expand Down
14 changes: 7 additions & 7 deletions packages/core/src/browser/xhrProxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toContain('/ok')
expect(request.response).toBe('ok')
expect(request.responseText).toBe('ok')
expect(request.status).toBe(200)
expect(request.isAborted).toBe(false)
expect(request.startTime).toEqual(jasmine.any(Number))
Expand All @@ -54,7 +54,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toContain('/expected-404')
expect(request.response).toBe('NOT FOUND')
expect(request.responseText).toBe('NOT FOUND')
expect(request.status).toBe(404)
expect(request.isAborted).toBe(false)
expect(request.startTime).toEqual(jasmine.any(Number))
Expand All @@ -75,7 +75,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toContain('/throw')
expect(request.response).toEqual('expected server error')
expect(request.responseText).toEqual('expected server error')
expect(request.status).toBe(500)
expect(request.isAborted).toBe(false)
expect(request.startTime).toEqual(jasmine.any(Number))
Expand All @@ -96,7 +96,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toBe('http://foo.bar/qux')
expect(request.response).toBe('')
expect(request.responseText).toBe('')
expect(request.status).toBe(0)
expect(request.isAborted).toBe(false)
expect(request.startTime).toEqual(jasmine.any(Number))
Expand All @@ -120,7 +120,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toContain('/ok')
expect(request.response).toBe('ok')
expect(request.responseText).toBe('ok')
expect(request.status).toBe(200)
expect(request.startTime).toEqual(jasmine.any(Number))
expect(request.duration).toEqual(jasmine.any(Number))
Expand All @@ -143,7 +143,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toContain('/ok')
expect(request.response).toBeUndefined()
expect(request.responseText).toBeUndefined()
expect(request.status).toBe(0)
expect(request.startTime).toEqual(jasmine.any(Number))
expect(request.duration).toEqual(jasmine.any(Number))
Expand All @@ -166,7 +166,7 @@ describe('xhr proxy', () => {
const request = getRequest(0)
expect(request.method).toBe('GET')
expect(request.url).toContain('/ok')
expect(request.response).toBe('ok')
expect(request.responseText).toBe('ok')
expect(request.status).toBe(200)
expect(request.isAborted).toBe(false)
expect(request.startTime).toEqual(jasmine.any(Number))
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/browser/xhrProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ export interface XhrStartContext {
export interface XhrCompleteContext extends XhrStartContext {
duration: Duration
status: number
response: string | undefined
responseText: string | undefined
isAborted: boolean
xhr: XMLHttpRequest
}

let xhrProxySingleton: XhrProxy | undefined
Expand Down Expand Up @@ -126,8 +127,9 @@ function proxyXhr() {
const xhrCompleteContext: XhrCompleteContext = {
...xhrPendingContext,
duration: elapsed(xhrPendingContext.startClocks.timeStamp, timeStampNow()),
response: this.response as string | undefined,
responseText: this.response as string | undefined,
status: this.status,
xhr: this,
}

onRequestCompleteCallbacks.forEach((callback) => callback(xhrCompleteContext))
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/domain/automaticErrorCollection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ describe('network error tracker', () => {
})
})

it('should add a default error response', (done) => {
it('should add a default error response text', (done) => {
fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, responseText: undefined })

fetchStubManager.whenAllComplete(() => {
Expand All @@ -249,7 +249,7 @@ describe('network error tracker', () => {
})
})

it('should truncate error response', (done) => {
it('should truncate error response text', (done) => {
fetchStub(FAKE_URL).resolveWith({
...DEFAULT_REQUEST,
responseText: 'Lorem ipsum dolor sit amet orci aliquam.',
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/domain/automaticErrorCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function startRuntimeErrorTracking(errorObservable: ErrorObservable) {
type,
source: ErrorSource.SOURCE,
startClocks: clocksNow(),
originalError: errorObject,
})
}
subscribe(traceKitReportHandler)
Expand Down Expand Up @@ -97,7 +98,7 @@ export function trackNetworkError(configuration: Configuration, errorObservable:
url: request.url,
},
source: ErrorSource.NETWORK,
stack: truncateResponse(request.response, configuration) || 'Failed to load',
stack: truncateResponseText(request.responseText, configuration) || 'Failed to load',
startClocks: request.startClocks,
})
}
Expand All @@ -119,11 +120,11 @@ function isServerError(request: { status: number }) {
return request.status >= 500
}

function truncateResponse(response: string | undefined, configuration: Configuration) {
if (response && response.length > configuration.requestErrorResponseLengthLimit) {
return `${response.substring(0, configuration.requestErrorResponseLengthLimit)}...`
function truncateResponseText(responseText: string | undefined, configuration: Configuration) {
if (responseText && responseText.length > configuration.requestErrorResponseLengthLimit) {
return `${responseText.substring(0, configuration.requestErrorResponseLengthLimit)}...`
}
return response
return responseText
}

function format(type: RequestType) {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/domain/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ describe('configuration', () => {
}
}
const configuration = buildConfiguration({ clientToken, beforeSend }, usEnv)
expect(configuration.beforeSend!({ view: { url: '/foo' } })).toBeFalse()
expect(configuration.beforeSend!({ view: { url: '/bar' } })).toBeUndefined()
expect(configuration.beforeSend!({ view: { url: '/foo' } }, {})).toBeFalse()
expect(configuration.beforeSend!({ view: { url: '/bar' } }, {})).toBeUndefined()
})

it('should catch errors and log them', () => {
Expand All @@ -58,7 +58,7 @@ describe('configuration', () => {
}
const configuration = buildConfiguration({ clientToken, beforeSend }, usEnv)
const displaySpy = spyOn(display, 'error')
expect(configuration.beforeSend!(null)).toBeUndefined()
expect(configuration.beforeSend!(null, {})).toBeUndefined()
expect(displaySpy).toHaveBeenCalledWith('beforeSend threw an error:', myError)
})
})
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/domain/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export interface UserConfiguration {
trackInteractions?: boolean
trackViewsManually?: boolean
proxyHost?: string
beforeSend?: (event: any) => void
beforeSend?: BeforeSendCallback

service?: string
env?: string
Expand All @@ -68,6 +68,8 @@ export interface UserConfiguration {
replica?: ReplicaUserConfiguration
}

export type BeforeSendCallback = (event: any, context?: any) => unknown

interface ReplicaUserConfiguration {
applicationId?: string
clientToken: string
Expand All @@ -78,7 +80,7 @@ export type Configuration = typeof DEFAULT_CONFIGURATION &
cookieOptions: CookieOptions

service?: string
beforeSend?: (event: any) => unknown
beforeSend?: BeforeSendCallback

isEnabled: (feature: string) => boolean
}
Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
export { DEFAULT_CONFIGURATION, Configuration, UserConfiguration, buildCookieOptions } from './domain/configuration'
export {
DEFAULT_CONFIGURATION,
Configuration,
UserConfiguration,
buildCookieOptions,
BeforeSendCallback,
} from './domain/configuration'
export { startAutomaticErrorCollection, ErrorObservable } from './domain/automaticErrorCollection'
export { computeStackTrace } from './domain/tracekit'
export {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/tools/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface RawError {
statusCode: number
method: string
}
originalError?: unknown
}

export const ErrorSource = {
Expand Down
4 changes: 2 additions & 2 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ import {
InternalMonitoring,
} from '@datadog/browser-core'
import { ProvidedSource } from '../domain/rumEventsCollection/error/errorCollection'
import { CommonContext, User, ActionType } from '../rawRumEvent.types'
import { CommonContext, User, ActionType, RumEventDomainContext } from '../rawRumEvent.types'
import { RumEvent } from '../rumEvent.types'
import { buildEnv } from './buildEnv'
import { startRum } from './startRum'

export interface RumUserConfiguration extends UserConfiguration {
applicationId: string
beforeSend?: (event: RumEvent) => void | boolean
beforeSend?: (event: RumEvent, context: RumEventDomainContext) => void | boolean
}

export type RumPublicApi = ReturnType<typeof makeRumPublicApi>
Expand Down
2 changes: 2 additions & 0 deletions packages/rum-core/src/browser/performanceCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle'
import { FAKE_INITIAL_DOCUMENT, isAllowedRequestUrl } from '../domain/rumEventsCollection/resource/resourceUtils'

import { getDocumentTraceId } from '../domain/tracing/getDocumentTraceId'
import { PerformanceEntryRepresentation } from '../rawRumEvent.types'

export interface RumPerformanceResourceTiming {
entryType: 'resource'
Expand All @@ -42,6 +43,7 @@ export interface RumPerformanceLongTaskTiming {
entryType: 'longtask'
startTime: RelativeTime
duration: Duration
toJSON(): PerformanceEntryRepresentation
}

export interface RumPerformancePaintTiming {
Expand Down
Loading