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

♻️ Custom sanitizer for Context Manager #3290

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 16 additions & 12 deletions packages/core/src/domain/context/contextManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,39 @@ import { noop } from '../../tools/utils/functionUtils'
import { createContextManager } from './contextManager'
import { createCustomerDataTracker } from './customerDataTracker'

function createNoopCustomerDataTracker() {
return createContextManager({ customerDataTracker: createCustomerDataTracker(noop) })
}

describe('createContextManager', () => {
it('starts with an empty context', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
expect(manager.getContext()).toEqual({})
})

it('updates the context', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContext({ bar: 'foo' })

expect(manager.getContext()).toEqual({ bar: 'foo' })
})

it('completely replaces the context', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContext({ a: 'foo' })
expect(manager.getContext()).toEqual({ a: 'foo' })
manager.setContext({ b: 'foo' })
expect(manager.getContext()).toEqual({ b: 'foo' })
})

it('sets a context value', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContextProperty('foo', 'bar')
expect(manager.getContext()).toEqual({ foo: 'bar' })
})

it('removes a context value', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContext({ a: 'foo', b: 'bar' })
manager.removeContextProperty('a')
expect(manager.getContext()).toEqual({ b: 'bar' })
Expand All @@ -39,39 +43,39 @@ describe('createContextManager', () => {
})

it('should get a clone of the context from getContext', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
expect(manager.getContext()).toEqual(manager.getContext())
expect(manager.getContext()).not.toBe(manager.getContext())
})

it('should set a clone of context via setContext', () => {
const nestedObject = { foo: 'bar' }
const context = { nested: nestedObject }
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContext(context)
expect(manager.getContext().nested).toEqual(nestedObject)
expect(manager.getContext().nested).not.toBe(nestedObject)
})

it('should set a clone of the property via setContextProperty', () => {
const nestedObject = { foo: 'bar' }
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContextProperty('nested', nestedObject)
expect(manager.getContext().nested).toEqual(nestedObject)
expect(manager.getContext().nested).not.toBe(nestedObject)
})

it('should clear context object via clearContext', () => {
const context = { foo: 'bar' }
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContext(context)
expect(manager.getContext()).toEqual(context)
manager.clearContext()
expect(manager.getContext()).toEqual({})
})

it('should prevent setting non object values', () => {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.setContext(null as any)
expect(manager.getContext()).toEqual({})
manager.setContext(undefined as any)
Expand All @@ -84,7 +88,7 @@ describe('createContextManager', () => {
const customerDataTracker = createCustomerDataTracker(noop)
const updateCustomerDataSpy = spyOn(customerDataTracker, 'updateCustomerData')
const resetCustomerDataSpy = spyOn(customerDataTracker, 'resetCustomerData')
const manager = createContextManager(customerDataTracker)
const manager = createContextManager({ customerDataTracker })

manager.setContextProperty('foo', 'bar')
manager.removeContextProperty('foo')
Expand All @@ -101,7 +105,7 @@ describe('createContextManager', () => {
describe('changeObservable', () => {
it('should notify on context changes', () => {
const changeSpy = jasmine.createSpy('change')
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createNoopCustomerDataTracker()
manager.changeObservable.subscribe(changeSpy)

manager.getContext()
Expand Down
17 changes: 16 additions & 1 deletion packages/core/src/domain/context/contextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import type { CustomerDataTracker } from './customerDataTracker'

export type ContextManager = ReturnType<typeof createContextManager>

export function createContextManager(customerDataTracker?: CustomerDataTracker) {
export function createContextManager({
customerDataTracker,
contextSanitizer,
}: {
customerDataTracker?: CustomerDataTracker
contextSanitizer?: (context: Context) => Context
} = {}) {
let context: Context = {}
const changeObservable = new Observable<void>()

Expand All @@ -17,6 +23,9 @@ export function createContextManager(customerDataTracker?: CustomerDataTracker)
setContext: (newContext: Context) => {
if (getType(newContext) === 'object') {
context = sanitize(newContext)
if (contextSanitizer) {
context = contextSanitizer(context)
}
nulrich marked this conversation as resolved.
Show resolved Hide resolved
customerDataTracker?.updateCustomerData(context)
} else {
contextManager.clearContext()
Expand All @@ -26,12 +35,18 @@ export function createContextManager(customerDataTracker?: CustomerDataTracker)

setContextProperty: (key: string, property: any) => {
context[key] = sanitize(property)
if (contextSanitizer) {
context = contextSanitizer(context)
}
customerDataTracker?.updateCustomerData(context)
changeObservable.notify()
},

removeContextProperty: (key: string) => {
delete context[key]
if (contextSanitizer) {
context = contextSanitizer(context)
}
customerDataTracker?.updateCustomerData(context)
changeObservable.notify()
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('storeContextManager', () => {
productKey?: string
customerDataType?: CustomerDataType
} = {}) {
const manager = createContextManager(createCustomerDataTracker(noop))
const manager = createContextManager({ customerDataTracker: createCustomerDataTracker(noop) })
if (initialContext) {
manager.setContext(initialContext)
}
Expand Down
16 changes: 9 additions & 7 deletions packages/logs/src/boot/logsPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,13 @@ export interface Strategy {

export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {
const customerDataTrackerManager = createCustomerDataTrackerManager()
const globalContextManager = createContextManager(
customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext)
)
const userContextManager = createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User))
const globalContextManager = createContextManager({
customerDataTracker: customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext),
})
const userContextManager = createContextManager({
customerDataTracker: customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User),
contextSanitizer: sanitizeUser,
})
const trackingConsentState = createTrackingConsentState()

function getCommonContext() {
Expand Down Expand Up @@ -240,15 +243,14 @@ export function makeLogsPublicApi(startLogsImpl: StartLogs): LogsPublicApi {

setUser: monitor((newUser) => {
if (checkUser(newUser)) {
userContextManager.setContext(sanitizeUser(newUser as Context))
userContextManager.setContext(newUser as Context)
}
}),

getUser: monitor(() => userContextManager.getContext()),

setUserProperty: monitor((key, property) => {
const sanitizedProperty = sanitizeUser({ [key]: property })[key]
userContextManager.setContextProperty(key, sanitizedProperty)
userContextManager.setContextProperty(key, property)
}),

removeUserProperty: monitor((key) => userContextManager.removeContextProperty(key)),
Expand Down
2 changes: 1 addition & 1 deletion packages/logs/src/domain/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class Logger {
private level: StatusType = StatusType.debug,
loggerContext: object = {}
) {
this.contextManager = createContextManager(customerDataTracker)
this.contextManager = createContextManager({ customerDataTracker })
this.contextManager.setContext(loggerContext as Context)
if (name) {
this.contextManager.setContextProperty('logger', { name })
Expand Down
16 changes: 9 additions & 7 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,13 @@ export function makeRumPublicApi(
options: RumPublicApiOptions = {}
): RumPublicApi {
const customerDataTrackerManager = createCustomerDataTrackerManager(CustomerDataCompressionStatus.Unknown)
const globalContextManager = createContextManager(
customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext)
)
const userContextManager = createContextManager(customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User))
const globalContextManager = createContextManager({
customerDataTracker: customerDataTrackerManager.getOrCreateTracker(CustomerDataType.GlobalContext),
})
const userContextManager = createContextManager({
customerDataTracker: customerDataTrackerManager.getOrCreateTracker(CustomerDataType.User),
contextSanitizer: sanitizeUser,
})
const trackingConsentState = createTrackingConsentState()
const customVitalsState = createCustomVitalsState()

Expand Down Expand Up @@ -516,16 +519,15 @@ export function makeRumPublicApi(

setUser: monitor((newUser) => {
if (checkUser(newUser)) {
userContextManager.setContext(sanitizeUser(newUser as Context))
userContextManager.setContext(newUser as Context)
}
addTelemetryUsage({ feature: 'set-user' })
}),

getUser: monitor(() => userContextManager.getContext()),

setUserProperty: monitor((key, property) => {
const sanitizedProperty = sanitizeUser({ [key]: property })[key]
userContextManager.setContextProperty(key, sanitizedProperty)
userContextManager.setContextProperty(key, property)
addTelemetryUsage({ feature: 'set-user' })
}),

Expand Down
8 changes: 6 additions & 2 deletions packages/rum-core/src/domain/contexts/commonContext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ describe('commonContext', () => {
beforeEach(() => {
isRecording = false
fakeContext = { foo: 'bar' }
const globalContextManager: ContextManager = createContextManager(createCustomerDataTracker(noop))
const userContextManager: ContextManager = createContextManager(createCustomerDataTracker(noop))
const globalContextManager: ContextManager = createContextManager({
customerDataTracker: createCustomerDataTracker(noop),
})
const userContextManager: ContextManager = createContextManager({
customerDataTracker: createCustomerDataTracker(noop),
})
spyOn(globalContextManager, 'getContext').and.callFake(() => fakeContext)
spyOn(userContextManager, 'getContext').and.callFake(() => fakeContext)

Expand Down
Loading