From 6ed851e3a1f806497127800980768dc3503106b6 Mon Sep 17 00:00:00 2001 From: cy-moi Date: Mon, 23 Sep 2024 15:25:08 +0200 Subject: [PATCH 1/2] Remove feature flag for view specific context --- .../core/src/tools/experimentalFeatures.ts | 1 - .../rum-core/src/boot/rumPublicApi.spec.ts | 11 ----- packages/rum-core/src/boot/rumPublicApi.ts | 46 ++++++++++--------- packages/rum-core/src/domain/assembly.spec.ts | 17 ------- packages/rum-core/src/domain/assembly.ts | 4 +- .../src/domain/view/trackViews.spec.ts | 12 ----- .../rum-core/src/domain/view/trackViews.ts | 6 +-- test/e2e/scenario/rum/init.scenario.ts | 2 - 8 files changed, 27 insertions(+), 72 deletions(-) diff --git a/packages/core/src/tools/experimentalFeatures.ts b/packages/core/src/tools/experimentalFeatures.ts index b3049aa85a..77ad3ce389 100644 --- a/packages/core/src/tools/experimentalFeatures.ts +++ b/packages/core/src/tools/experimentalFeatures.ts @@ -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 = new Set() diff --git a/packages/rum-core/src/boot/rumPublicApi.spec.ts b/packages/rum-core/src/boot/rumPublicApi.spec.ts index 9fc99aac5a..0b531184db 100644 --- a/packages/rum-core/src/boot/rumPublicApi.spec.ts +++ b/packages/rum-core/src/boot/rumPublicApi.spec.ts @@ -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' }) @@ -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() - }) }) }) diff --git a/packages/rum-core/src/boot/rumPublicApi.ts b/packages/rum-core/src/boot/rumPublicApi.ts index d92b18d8cc..07463f8264 100644 --- a/packages/rum-core/src/boot/rumPublicApi.ts +++ b/packages/rum-core/src/boot/rumPublicApi.ts @@ -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` * @@ -425,28 +441,6 @@ 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) => { @@ -454,6 +448,14 @@ export function makeRumPublicApi( 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' }) diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index bcbb2f0e93..e7e21e77ee 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -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) => { @@ -213,22 +212,6 @@ describe('rum assembly', () => { expect(serverRumEvents[0].context).toEqual({ foo: 'bar' }) }) - it('should reject modification on context field for view events', () => { - 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: { diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index ba0dd6947d..e46198c0eb 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -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', diff --git a/packages/rum-core/src/domain/view/trackViews.spec.ts b/packages/rum-core/src/domain/view/trackViews.spec.ts index fb45bc1f01..6c89f01e44 100644 --- a/packages/rum-core/src/domain/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/view/trackViews.spec.ts @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/packages/rum-core/src/domain/view/trackViews.ts b/packages/rum-core/src/domain/view/trackViews.ts index 0590d6ef91..48a40deb26 100644 --- a/packages/rum-core/src/domain/view/trackViews.ts +++ b/packages/rum-core/src/domain/view/trackViews.ts @@ -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) @@ -282,9 +282,7 @@ function newView( name, service, version, - context: isExperimentalFeatureEnabled(ExperimentalFeature.VIEW_SPECIFIC_CONTEXT) - ? contextManager.getContext() - : undefined, + context: contextManager.getContext(), loadingType, location, startClocks, diff --git a/test/e2e/scenario/rum/init.scenario.ts b/test/e2e/scenario/rum/init.scenario.ts index 3d3a4e6d9e..79318ed0bb 100644 --- a/test/e2e/scenario/rum/init.scenario.ts +++ b/test/e2e/scenario/rum/init.scenario.ts @@ -125,7 +125,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 @@ -143,7 +142,6 @@ describe('beforeSend', () => { createTest('allows to replace events context') .withRum({ - enableExperimentalFeatures: ['view_specific_context'], beforeSend: (event) => { event.context = { foo: 'bar' } return true From c8b0ecbee69a7e91d31517669ed06015201042d2 Mon Sep 17 00:00:00 2001 From: cy-moi Date: Tue, 24 Sep 2024 14:16:40 +0200 Subject: [PATCH 2/2] Add types in rumPublicApi --- test/e2e/scenario/rum/init.scenario.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/scenario/rum/init.scenario.ts b/test/e2e/scenario/rum/init.scenario.ts index b04c0aa491..f5894cd8b6 100644 --- a/test/e2e/scenario/rum/init.scenario.ts +++ b/test/e2e/scenario/rum/init.scenario.ts @@ -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')