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-1176] collect other console logs new #1316

Merged
merged 30 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e552db1
Add console observable
amortemousque Feb 4, 2022
bf39e51
🔥 Delete trackConsoleError
amortemousque Feb 4, 2022
4ba65d7
Add console observable to rum
amortemousque Feb 4, 2022
3ef80e6
Add console observable to Logs
amortemousque Feb 4, 2022
a1c5cbb
👌 Add trackConsoleError
amortemousque Feb 7, 2022
933269a
👌 Update console log event observable
amortemousque Feb 7, 2022
eed7e68
👌 Move source computation to Log and RUM
amortemousque Feb 7, 2022
e6a6b43
Use an enum + keep current behavior
amortemousque Feb 7, 2022
03a9de3
Add feature flag forward-logs
amortemousque Feb 7, 2022
60ea273
✅ Add tests
amortemousque Feb 8, 2022
7ff6239
✨ Add forwardConsoleLogs init config option
amortemousque Feb 8, 2022
1063bad
Fix rum console error tracking
amortemousque Feb 8, 2022
cf3a4cd
Merge branch 'main' into aymeric/collect-other-console-logs-new
amortemousque Feb 8, 2022
7fc700c
✅ Add test for forwardErrorsToLogs: 'all'
amortemousque Feb 8, 2022
4abf4f2
👌 Namings
amortemousque Feb 9, 2022
9a874c3
👌 Ensure that only allowed api are instrumented.
amortemousque Feb 9, 2022
32a4393
👌 Remove CONSOLE_APIS
amortemousque Feb 9, 2022
4b8ea23
👌 Clearer cast comment
amortemousque Feb 9, 2022
197dadc
Update error observable naming
amortemousque Feb 9, 2022
d10b0dd
👌 Move console observable into domain
amortemousque Feb 9, 2022
5cd4d91
👌 Add mergeObservables tests
amortemousque Feb 9, 2022
18b2dbe
✅ Update test to ensure the api is instrumented once
amortemousque Feb 9, 2022
f88969c
Make removeDuplicates IE compatible
amortemousque Feb 9, 2022
aa2669c
Merge branch 'main' into aymeric/collect-other-console-logs-new
amortemousque Feb 10, 2022
d3792a5
👌 Code style
amortemousque Feb 10, 2022
3c3176c
👌 Add removeDuplicates test
amortemousque Feb 10, 2022
fc3d57e
Fix naming
amortemousque Feb 10, 2022
7e26428
Fix test flackyness
amortemousque Feb 11, 2022
e35baea
Merge branch 'main' into aymeric/collect-other-console-logs-new
amortemousque Feb 15, 2022
109ce1f
Simplify validateAndBuildLogsConfiguration
amortemousque Feb 15, 2022
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
61 changes: 61 additions & 0 deletions packages/core/src/browser/consoleObservable.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* eslint-disable no-console */
import type { Subscription } from '../tools/observable'
import { initConsoleObservable } from './consoleObservable'

// TODO cover other console APIs
describe('console error observable', () => {
let consoleErrorStub: jasmine.Spy
let consoleSubscription: Subscription
let notifyError: jasmine.Spy

beforeEach(() => {
consoleErrorStub = spyOn(console, 'error')
notifyError = jasmine.createSpy('notifyError')

consoleSubscription = initConsoleObservable(['error']).subscribe(notifyError)
})

afterEach(() => {
consoleSubscription.unsubscribe()
})

it('should keep original behavior', () => {
console.error('foo', 'bar')

expect(consoleErrorStub).toHaveBeenCalledWith('foo', 'bar')
})

it('should notify error', () => {
console.error('foo', 'bar')

expect(notifyError).toHaveBeenCalledWith({
message: 'console error: foo bar',
stack: undefined,
handlingStack: jasmine.any(String),
startClocks: jasmine.any(Object),
status: 'error',
source: 'console',
})
})

it('should generate a handling stack', () => {
function triggerError() {
console.error('foo', 'bar')
}
triggerError()
const consoleLog = notifyError.calls.mostRecent().args[0]
expect(consoleLog.handlingStack).toMatch(/^Error:\s+at triggerError (.|\n)*$/)
})

it('should allow multiple callers', () => {
const notifyOtherCaller = jasmine.createSpy('notifyOtherCaller')
const otherConsoleSubscription = initConsoleObservable(['error']).subscribe(notifyOtherCaller)

console.error('foo', 'bar')

expect(notifyError).toHaveBeenCalledTimes(1)
expect(notifyOtherCaller).toHaveBeenCalledTimes(1)

otherConsoleSubscription.unsubscribe()
})
})
82 changes: 82 additions & 0 deletions packages/core/src/browser/consoleObservable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { callMonitored } from '../domain/internalMonitoring'
import { computeStackTrace } from '../domain/tracekit'
import { createHandlingStack, formatErrorMessage, toStackTraceString } from '../tools/error'
import { mergeObservables, Observable } from '../tools/observable'
import { find, jsonStringify } from '../tools/utils'

export const ConsoleApiName = {
log: 'log',
debug: 'debug',
info: 'info',
warn: 'warn',
error: 'error',
} as const

type ConsoleApiNameType = typeof ConsoleApiName[keyof typeof ConsoleApiName]

export interface ConsoleLog {
message: string
apiName: ConsoleApiNameType
stack?: string
handlingStack?: string
}

const consoleObservables: { [k in ConsoleApiNameType]?: Observable<ConsoleLog> } = {}

export function initConsoleObservable(apis: ConsoleApiNameType[]) {
const observables = apis.map((api) => {
if (!consoleObservables[api]) {
consoleObservables[api] = createConsoleObservable(api)
}
return consoleObservables[api]!
})

return mergeObservables<ConsoleLog>(...observables)
}

/* eslint-disable no-console */
function createConsoleObservable(api: ConsoleApiNameType) {
const observable = new Observable<ConsoleLog>(() => {
const originalConsoleApi = console[api]

console[api] = (...params: unknown[]) => {
originalConsoleApi.apply(console, params)
const handlingStack = createHandlingStack()

callMonitored(() => {
observable.notify(buildConsoleLog(params, api, handlingStack))
})
}

return () => {
console[api] = originalConsoleApi
}
})

return observable
}

function buildConsoleLog(params: unknown[], apiName: ConsoleApiNameType, handlingStack: string): ConsoleLog {
const log: ConsoleLog = {
message: [`console ${apiName}:`, ...params].map((param) => formatConsoleParameters(param)).join(' '),
apiName,
}

if (apiName === ConsoleApiName.error) {
const firstErrorParam = find(params, (param: unknown): param is Error => param instanceof Error)
log.stack = firstErrorParam ? toStackTraceString(computeStackTrace(firstErrorParam)) : undefined
log.handlingStack = handlingStack
amortemousque marked this conversation as resolved.
Show resolved Hide resolved
}

return log
}

function formatConsoleParameters(param: unknown) {
if (typeof param === 'string') {
return param
}
if (param instanceof Error) {
return formatErrorMessage(computeStackTrace(param))
}
return jsonStringify(param, undefined, 2)
}
91 changes: 0 additions & 91 deletions packages/core/src/domain/error/trackConsoleError.spec.ts

This file was deleted.

70 changes: 0 additions & 70 deletions packages/core/src/domain/error/trackConsoleError.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export {
updateExperimentalFeatures,
resetExperimentalFeatures,
} from './domain/configuration'
export { trackConsoleError } from './domain/error/trackConsoleError'
export { trackRuntimeError } from './domain/error/trackRuntimeError'
export { computeStackTrace, StackTrace } from './domain/tracekit'
export { BuildEnv, BuildMode, defineGlobal, makePublicApi } from './boot/init'
Expand Down Expand Up @@ -56,6 +55,7 @@ export { Context, ContextArray, ContextValue } from './tools/context'
export { areCookiesAuthorized, getCookie, setCookie, deleteCookie, COOKIE_ACCESS_DELAY } from './browser/cookie'
export { initXhrObservable, XhrCompleteContext, XhrStartContext } from './browser/xhrObservable'
export { initFetchObservable, FetchCompleteContext, FetchStartContext, FetchContext } from './browser/fetchObservable'
export { initConsoleObservable, ConsoleLog, ConsoleApiName } from './browser/consoleObservable'
export { BoundedBuffer } from './tools/boundedBuffer'
export { catchUserErrors } from './tools/catchUserErrors'
export { createContextManager } from './tools/contextManager'
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/tools/observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,14 @@ export class Observable<T> {
this.observers.forEach((observer) => observer(data))
}
}

export function mergeObservables<T>(...observables: Array<Observable<T>>) {
const globalObservable = new Observable<T>(() => {
const subscriptions: Subscription[] = observables.map((observable) =>
observable.subscribe((data) => globalObservable.notify(data))
)
return () => subscriptions.forEach((subscription) => subscription.unsubscribe())
})

return globalObservable
}
amortemousque marked this conversation as resolved.
Show resolved Hide resolved
13 changes: 11 additions & 2 deletions packages/logs/src/boot/startLogs.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Context, RawError, RelativeTime, TimeStamp } from '@datadog/browser-core'
import type { ConsoleLog, Context, RawError, RelativeTime, TimeStamp } from '@datadog/browser-core'
import {
ErrorSource,
noop,
Expand Down Expand Up @@ -61,6 +61,7 @@ describe('logs', () => {
let sessionIsTracked: boolean
let server: sinon.SinonFakeServer
let errorObservable: Observable<RawError>
let consoleObservable: Observable<ConsoleLog>
const sessionManager: LogsSessionManager = {
findTrackedSession: () => (sessionIsTracked ? { id: SESSION_ID } : undefined),
}
Expand All @@ -69,12 +70,20 @@ describe('logs', () => {
configuration: configurationOverrides,
}: { errorLogger?: Logger; configuration?: Partial<LogsConfiguration> } = {}) => {
const configuration = { ...baseConfiguration, ...configurationOverrides }
return doStartLogs(configuration, errorObservable, internalMonitoring, sessionManager, errorLogger)
return doStartLogs(
configuration,
errorObservable,
consoleObservable,
internalMonitoring,
sessionManager,
errorLogger
)
}

beforeEach(() => {
sessionIsTracked = true
errorObservable = new Observable<RawError>()
consoleObservable = new Observable<ConsoleLog>()
server = sinon.fakeServer.create()
})

Expand Down
Loading