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

✨[RUM] do not snake case user defined contexts #188

Merged
merged 4 commits into from
Dec 2, 2019
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: 1 addition & 5 deletions packages/core/src/internalMonitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ enum StatusType {
}

export interface MonitoringMessage {
entryType: 'internal'
message: string
status: StatusType
error?: {
Expand Down Expand Up @@ -47,8 +46,7 @@ export function startInternalMonitoring(configuration: Configuration) {
referrer: document.referrer,
url: window.location.href,
},
}),
utils.withSnakeCaseKeys
})
)

lodashAssign(monitoringConfiguration, {
Expand Down Expand Up @@ -88,15 +86,13 @@ export function monitor<T extends Function>(fn: T): T {
export function addMonitoringMessage(message: string) {
addToMonitoringBatch({
message,
entryType: 'internal',
status: StatusType.info,
})
}

function addErrorToMonitoringBatch(e: unknown) {
addToMonitoringBatch({
...formatError(e),
entryType: 'internal',
status: StatusType.error,
})
}
Expand Down
8 changes: 2 additions & 6 deletions packages/core/src/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ export class Batch<T> {
private bytesLimit: number,
private maxMessageSize: number,
private flushTimeout: number,
private contextProvider: () => Context,
private messageProcessor?: (message: Context) => Context
private contextProvider: () => Context
) {
this.flushOnVisibilityHidden()
this.flushPeriodically()
Expand Down Expand Up @@ -82,10 +81,7 @@ export class Batch<T> {
}

private process(message: T) {
let contextualizedMessage = lodashMerge({}, this.contextProvider(), message) as Context
if (this.messageProcessor) {
contextualizedMessage = this.messageProcessor(contextualizedMessage)
}
const contextualizedMessage = lodashMerge({}, this.contextProvider(), message) as Context
const processedMessage = jsonStringify(contextualizedMessage)!
const messageBytesSize = this.sizeInBytes(processedMessage)
return { processedMessage, messageBytesSize }
Expand Down
1 change: 0 additions & 1 deletion packages/core/test/internalMonitoring.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ describe('internal monitoring', () => {

expect(JSON.parse(server.requests[0].requestBody)).toEqual({
date: FAKE_DATE,
entry_type: 'internal',
error: jasmine.anything(),
message: 'message',
status: 'error',
Expand Down
20 changes: 0 additions & 20 deletions packages/core/test/transport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,24 +162,4 @@ describe('batch', () => {
expect(transport.send).not.toHaveBeenCalled()
warnStub.restore()
})

it('should allow to add a custom message processor', () => {
batch = new Batch(
transport,
MAX_SIZE,
BATCH_BYTES_LIMIT,
MESSAGE_BYTES_LIMIT,
FLUSH_TIMEOUT,
() => ({}),
(message: Context) => {
message.message = `*** ${message.message} ***`
return message
}
)

batch.add({ message: 'hello' })
batch.flush()

expect(transport.send).toHaveBeenCalledWith(`{"message":"*** hello ***"}`, jasmine.any(Number))
})
})
94 changes: 55 additions & 39 deletions packages/rum/src/rum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,45 +134,32 @@ export function startRum(
): Omit<RumGlobal, 'init'> {
let globalContext: Context = {}

const batch = new Batch<RumEvent>(
new HttpRequest(configuration.rumEndpoint, configuration.batchBytesLimit),
configuration.maxBatchSize,
configuration.batchBytesLimit,
configuration.maxMessageSize,
configuration.flushTimeout,
() =>
lodashMerge(
{
applicationId,
date: new Date().getTime(),
screen: {
// needed for retro compatibility
id: viewId,
url: viewLocation.href,
},
sessionId: session.getId(),
view: {
id: viewId,
referrer: document.referrer,
url: viewLocation.href,
},
},
globalContext
),
withSnakeCaseKeys
const batch = startRumBatch(
configuration,
session,
() => ({
applicationId,
date: new Date().getTime(),
screen: {
// needed for retro compatibility
id: viewId,
url: viewLocation.href,
},
sessionId: session.getId(),
view: {
id: viewId,
referrer: document.referrer,
url: viewLocation.href,
},
}),
() => globalContext
)

const addRumEvent = (event: RumEvent) => {
if (session.isTracked()) {
batch.add(event)
}
}

trackView(batch, window.location, lifeCycle, addRumEvent)
trackErrors(lifeCycle, addRumEvent)
trackRequests(configuration, lifeCycle, session, addRumEvent)
trackPerformanceTiming(configuration, lifeCycle, addRumEvent)
trackUserAction(lifeCycle, addRumEvent)
trackView(window.location, lifeCycle, batch.addRumEvent, batch.beforeFlushOnUnload)
trackErrors(lifeCycle, batch.addRumEvent)
trackRequests(configuration, lifeCycle, session, batch.addRumEvent)
trackPerformanceTiming(configuration, lifeCycle, batch.addRumEvent)
trackUserAction(lifeCycle, batch.addUserEvent)

return {
addRumGlobalContext: monitor((key: string, value: ContextValue) => {
Expand All @@ -196,6 +183,35 @@ export function startRum(
}
}

function startRumBatch(
configuration: Configuration,
session: RumSession,
rumContextProvider: () => Context,
globalContextProvider: () => Context
) {
const batch = new Batch<Context>(
new HttpRequest(configuration.rumEndpoint, configuration.batchBytesLimit),
configuration.maxBatchSize,
configuration.batchBytesLimit,
configuration.maxMessageSize,
configuration.flushTimeout,
() => lodashMerge(withSnakeCaseKeys(rumContextProvider()), globalContextProvider())
)
return {
addRumEvent: (event: RumEvent) => {
if (session.isTracked()) {
batch.add(withSnakeCaseKeys(event as Context))
}
},
addUserEvent: (event: RumUserAction) => {
if (session.isTracked()) {
batch.add(event as Context)
}
},
beforeFlushOnUnload: (handler: () => void) => batch.beforeFlushOnUnload(handler),
}
}

function trackErrors(lifeCycle: LifeCycle, addRumEvent: (event: RumEvent) => void) {
lifeCycle.subscribe(LifeCycleEventType.error, ({ message, context }: ErrorMessage) => {
addRumEvent({
Expand All @@ -211,9 +227,9 @@ function trackErrors(lifeCycle: LifeCycle, addRumEvent: (event: RumEvent) => voi
})
}

function trackUserAction(lifeCycle: LifeCycle, addRumEvent: (event: RumEvent) => void) {
function trackUserAction(lifeCycle: LifeCycle, addUserEvent: (event: RumUserAction) => void) {
lifeCycle.subscribe(LifeCycleEventType.userAction, ({ name, context }) => {
addRumEvent({
addUserEvent({
...context,
evt: {
name,
Expand Down
6 changes: 3 additions & 3 deletions packages/rum/src/viewTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ let documentVersion: number
let viewMeasures: ViewMeasures

export function trackView(
batch: Batch<RumEvent>,
location: Location,
lifeCycle: LifeCycle,
addRumEvent: (event: RumEvent) => void
addRumEvent: (event: RumEvent) => void,
beforeFlushOnUnload: (handler: () => void) => void
) {
const scheduleViewUpdate = throttle(monitor(() => updateView(addRumEvent)), THROTTLE_VIEW_UPDATE_PERIOD, {
leading: false,
Expand All @@ -38,7 +38,7 @@ export function trackView(
trackMeasures(lifeCycle, scheduleViewUpdate)
trackRenewSession(location, lifeCycle, addRumEvent)

batch.beforeFlushOnUnload(() => updateView(addRumEvent))
beforeFlushOnUnload(() => updateView(addRumEvent))
}

function newView(location: Location, addRumEvent: (event: RumEvent) => void) {
Expand Down
38 changes: 36 additions & 2 deletions packages/rum/test/rum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import sinon from 'sinon'

import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle'
import { startPerformanceCollection } from '../src/performanceCollection'
import { handleResourceEntry, RumEvent, RumResourceEvent, startRum } from '../src/rum'
import { handleResourceEntry, RumEvent, RumResourceEvent, startRum, UserAction } from '../src/rum'
import { RumGlobal } from '../src/rum.entry'

function getEntry(addRumEvent: (event: RumEvent) => void, index: number) {
Expand Down Expand Up @@ -143,6 +143,7 @@ describe('rum session', () => {
const FAKE_ERROR: Partial<ErrorMessage> = { message: 'test' }
const FAKE_RESOURCE: Partial<PerformanceEntry> = { name: 'http://foo.com', entryType: 'resource' }
const FAKE_REQUEST: Partial<RequestDetails> = { url: 'http://foo.com' }
const FAKE_USER_ACTION: UserAction = { name: 'action', context: { foo: 'bar' } }
let server: sinon.SinonFakeServer
let original: PerformanceObserver | undefined
let stubBuilder: PerformanceObserverStubBuilder
Expand Down Expand Up @@ -176,8 +177,9 @@ describe('rum session', () => {
stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource')
lifeCycle.notify(LifeCycleEventType.error, FAKE_ERROR as ErrorMessage)
lifeCycle.notify(LifeCycleEventType.request, FAKE_REQUEST as RequestDetails)
lifeCycle.notify(LifeCycleEventType.userAction, FAKE_USER_ACTION)

expect(server.requests.length).toEqual(3)
expect(server.requests.length).toEqual(4)
})

it('when tracked without resources should not track resources', () => {
Expand Down Expand Up @@ -213,6 +215,7 @@ describe('rum session', () => {
stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource')
lifeCycle.notify(LifeCycleEventType.request, FAKE_REQUEST as RequestDetails)
lifeCycle.notify(LifeCycleEventType.error, FAKE_ERROR as ErrorMessage)
lifeCycle.notify(LifeCycleEventType.userAction, FAKE_USER_ACTION)

expect(server.requests.length).toEqual(0)
})
Expand Down Expand Up @@ -337,4 +340,35 @@ describe('rum global context', () => {
expect((getRumMessage(server, 1) as any).foo).toEqual('bar')
expect((getRumMessage(server, 1) as any).bar).toBeUndefined()
})

it('should not be automatically snake cased', () => {
RUM.setRumGlobalContext({ fooBar: 'foo' })
lifeCycle.notify(LifeCycleEventType.error, FAKE_ERROR as ErrorMessage)

expect((getRumMessage(server, 0) as any).fooBar).toEqual('foo')
})
})

describe('rum user action', () => {
let lifeCycle: LifeCycle
let RUM: RumApi
let server: sinon.SinonFakeServer

beforeEach(() => {
const session = {
getId: () => undefined,
isTracked: () => true,
isTrackedWithResource: () => true,
}
server = sinon.fakeServer.create()
lifeCycle = new LifeCycle()
RUM = startRum('appId', lifeCycle, configuration as Configuration, session) as RumApi
server.requests = []
})

it('should not be automatically snake cased', () => {
lifeCycle.notify(LifeCycleEventType.userAction, { name: 'hello', context: { fooBar: 'foo' } })

expect((getRumMessage(server, 0) as any).fooBar).toEqual('foo')
})
})
10 changes: 1 addition & 9 deletions packages/rum/test/viewTracker.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { Batch } from '@browser-sdk/core'

import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle'
import { PerformanceLongTaskTiming, PerformancePaintTiming, RumEvent, RumViewEvent, UserAction } from '../src/rum'
import { trackView, viewId, viewLocation } from '../src/viewTracker'
Expand All @@ -18,13 +16,7 @@ function setup({
fakeLocation.hash = url.hash
})
const fakeLocation: Partial<Location> = { pathname: '/foo' }
const fakeBatch: Partial<Batch<RumEvent>> = { beforeFlushOnUnload: () => undefined }
trackView(
fakeBatch as Batch<RumEvent>,
fakeLocation as Location,
lifeCycle || new LifeCycle(),
addRumEvent || (() => undefined)
)
trackView(fakeLocation as Location, lifeCycle || new LifeCycle(), addRumEvent || (() => undefined), () => undefined)
}

describe('rum track url change', () => {
Expand Down