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 4813] Remove feature flag for view specific context #3031

Merged
merged 3 commits into from
Sep 25, 2024
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
1 change: 0 additions & 1 deletion packages/core/src/tools/experimentalFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export enum ExperimentalFeature {
REMOTE_CONFIGURATION = 'remote_configuration',
UPDATE_VIEW_NAME = 'update_view_name',
LONG_ANIMATION_FRAME = 'long_animation_frame',
VIEW_SPECIFIC_CONTEXT = 'view_specific_context',
}

const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()
Expand Down
11 changes: 0 additions & 11 deletions packages/rum-core/src/boot/rumPublicApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,6 @@ describe('rum public api', () => {
})

it('should set view specific context with setViewContext', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])

rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
/* eslint-disable @typescript-eslint/no-unsafe-call */
;(rumPublicApi as any).setViewContext({ foo: 'bar' })
Expand All @@ -901,20 +899,11 @@ describe('rum public api', () => {
})

it('should set view specific context with setViewContextProperty', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])

rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
/* eslint-disable @typescript-eslint/no-unsafe-call */
;(rumPublicApi as any).setViewContextProperty('foo', 'bar')

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

it('should not expose view specific context when ff is disabled', () => {
rumPublicApi = makeRumPublicApi(noopStartRum, noopRecorderApi)
rumPublicApi.init(DEFAULT_INIT_CONFIGURATION)
expect((rumPublicApi as any).setViewContext).toBeUndefined()
expect((rumPublicApi as any).setViewContextProperty).toBeUndefined()
})
})
})
46 changes: 24 additions & 22 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ export interface RumPublicApi extends PublicApi {
* See [User tracking consent](https://docs.datadoghq.com/real_user_monitoring/browser/advanced_configuration/#user-tracking-consent) for further information.
*/
setTrackingConsent: (trackingConsent: TrackingConsent) => void

/**
* Set View Context.
*
* Enable to manually set the context of the current view.
* @param context context of the view
*/
setViewContext: (context: Context) => void
/**
* Set View Context Property.
*
* Enable to manually set a property of the context of the current view.
* @param key key of the property
* @param value value of the property
*/
setViewContextProperty: (key: string, value: any) => void
/**
* Set the global context information to all events, stored in `@context`
*
Expand Down Expand Up @@ -425,35 +441,21 @@ export function makeRumPublicApi(
strategy.updateViewName(name)
})
}
if (isExperimentalFeatureEnabled(ExperimentalFeature.VIEW_SPECIFIC_CONTEXT)) {
/**
* Set View Context.
*
* Enable to manually set the context of the current view.
* @param context context of the view
*/
;(rumPublicApi as any).setViewContext = monitor((context: Context) => {
strategy.setViewContext(context)
})

/**
* Set View Context Property.
*
* Enable to manually set a property of the context of the current view.
* @param key key of the property
* @param value value of the property
*/
;(rumPublicApi as any).setViewContextProperty = monitor((key: string, value: any) => {
strategy.setViewContextProperty(key, value)
})
}
}),

setTrackingConsent: monitor((trackingConsent) => {
trackingConsentState.update(trackingConsent)
addTelemetryUsage({ feature: 'set-tracking-consent', tracking_consent: trackingConsent })
}),

setViewContext: monitor((context: Context) => {
strategy.setViewContext(context)
}),

setViewContextProperty: monitor((key: string, value: any) => {
strategy.setViewContextProperty(key, value)
}),

setGlobalContext: monitor((context) => {
globalContextManager.setContext(context)
addTelemetryUsage({ feature: 'set-global-context' })
Expand Down
17 changes: 0 additions & 17 deletions packages/rum-core/src/domain/assembly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ describe('rum assembly', () => {
})

it('should accept modification on context field for view events', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])
const { lifeCycle, serverRumEvents } = setupAssemblyTestWithDefaults({
partialConfiguration: {
beforeSend: (event) => {
Expand All @@ -213,22 +212,6 @@ describe('rum assembly', () => {
expect(serverRumEvents[0].context).toEqual({ foo: 'bar' })
})

it('should reject modification on context field for view events', () => {
N-Boutaib marked this conversation as resolved.
Show resolved Hide resolved
const { lifeCycle, serverRumEvents } = setupAssemblyTestWithDefaults({
partialConfiguration: {
beforeSend: (event) => {
event.context.foo = 'bar'
},
},
})

notifyRawRumEvent(lifeCycle, {
rawRumEvent: createRawRumEvent(RumEventType.VIEW),
})

expect(serverRumEvents[0].context).toBeUndefined()
})

it('should reject replacing the context field to non-object', () => {
const { lifeCycle, serverRumEvents } = setupAssemblyTestWithDefaults({
partialConfiguration: {
Expand Down
4 changes: 1 addition & 3 deletions packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ export function startRumAssembly(
reportError: (error: RawError) => void
) {
modifiableFieldPathsByEvent = {
[RumEventType.VIEW]: isExperimentalFeatureEnabled(ExperimentalFeature.VIEW_SPECIFIC_CONTEXT)
? assign({}, USER_CUSTOMIZABLE_FIELD_PATHS, VIEW_MODIFIABLE_FIELD_PATHS)
: VIEW_MODIFIABLE_FIELD_PATHS,
[RumEventType.VIEW]: assign({}, USER_CUSTOMIZABLE_FIELD_PATHS, VIEW_MODIFIABLE_FIELD_PATHS),
[RumEventType.ERROR]: assign(
{
'error.message': 'string',
Expand Down
12 changes: 0 additions & 12 deletions packages/rum-core/src/domain/view/trackViews.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,6 @@ describe('view event count', () => {

describe('view specific context', () => {
it('should update view context if startView has context parameter', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])
viewTest = setupViewTest({ lifeCycle })
const { getViewUpdate, startView } = viewTest

Expand All @@ -913,7 +912,6 @@ describe('view event count', () => {
})

it('should replace current context set on view event', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])
viewTest = setupViewTest({ lifeCycle })
const { getViewUpdate, startView } = viewTest

Expand All @@ -924,16 +922,7 @@ describe('view event count', () => {
expect(getViewUpdate(4).context).toEqual({ bar: 'baz' })
})

it('should not update view context if the feature is not enabled', () => {
viewTest = setupViewTest({ lifeCycle })
const { getViewUpdate, startView } = viewTest

startView({ context: { foo: 'bar' } })
expect(getViewUpdate(2).context).toBeUndefined()
})

it('should set view context with setViewContext', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])
viewTest = setupViewTest({ lifeCycle })
const { getViewUpdate, setViewContext } = viewTest

Expand All @@ -942,7 +931,6 @@ describe('view event count', () => {
})

it('should set view context with setViewContextProperty', () => {
mockExperimentalFeatures([ExperimentalFeature.VIEW_SPECIFIC_CONTEXT])
viewTest = setupViewTest({ lifeCycle })
const { getViewUpdate, setViewContextProperty } = viewTest

Expand Down
6 changes: 2 additions & 4 deletions packages/rum-core/src/domain/view/trackViews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ function newView(
name = viewOptions.name
service = viewOptions.service || undefined
version = viewOptions.version || undefined
if (isExperimentalFeatureEnabled(ExperimentalFeature.VIEW_SPECIFIC_CONTEXT) && viewOptions.context) {
if (viewOptions.context) {
context = viewOptions.context
// use ContextManager to update the context so we always sanitize it
contextManager.setContext(context)
Expand Down Expand Up @@ -282,9 +282,7 @@ function newView(
name,
service,
version,
context: isExperimentalFeatureEnabled(ExperimentalFeature.VIEW_SPECIFIC_CONTEXT)
? contextManager.getContext()
: undefined,
context: contextManager.getContext(),
loadingType,
location,
startClocks,
Expand Down
6 changes: 2 additions & 4 deletions test/e2e/scenario/rum/init.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,9 @@ describe('API calls and events around init', () => {
.withRumInit((configuration) => {
window.DD_RUM!.init(configuration)
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
;(window.DD_RUM as any).setViewContext({ foo: 'bar' })
window.DD_RUM!.setViewContext({ foo: 'bar' })
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
;(window.DD_RUM as any).setViewContextProperty('bar', 'foo')
window.DD_RUM!.setViewContextProperty('bar', 'foo')

// context should populate the context of the children events
window.DD_RUM!.addAction('custom action')
Expand Down Expand Up @@ -182,7 +182,6 @@ describe('API calls and events around init', () => {
describe('beforeSend', () => {
createTest('allows to edit events context with feature flag')
.withRum({
enableExperimentalFeatures: ['view_specific_context'],
beforeSend: (event: any) => {
event.context!.foo = 'bar'
return true
Expand All @@ -200,7 +199,6 @@ describe('beforeSend', () => {

createTest('allows to replace events context')
.withRum({
enableExperimentalFeatures: ['view_specific_context'],
beforeSend: (event) => {
event.context = { foo: 'bar' }
return true
Expand Down