From 81981c0dde3dd98077075fdce94a2b45fa6c610e Mon Sep 17 00:00:00 2001 From: Aymeric Date: Thu, 6 Jun 2024 15:24:05 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Set=20the=20experimental?= =?UTF-8?q?=20feature=20flags=20as=20early=20as=20possible=20(#2796)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../configuration/configuration.spec.ts | 5 ---- .../src/domain/configuration/configuration.ts | 10 ------- packages/core/src/index.ts | 1 + .../src/tools/experimentalFeatures.spec.ts | 28 ++++++++++++++++++- .../core/src/tools/experimentalFeatures.ts | 13 +++++++++ packages/logs/src/boot/preStartLogs.ts | 3 ++ packages/rum-core/src/boot/preStartRum.ts | 4 +++ 7 files changed, 48 insertions(+), 16 deletions(-) diff --git a/packages/core/src/domain/configuration/configuration.spec.ts b/packages/core/src/domain/configuration/configuration.spec.ts index ae5ffcf66b..900420f480 100644 --- a/packages/core/src/domain/configuration/configuration.spec.ts +++ b/packages/core/src/domain/configuration/configuration.spec.ts @@ -35,11 +35,6 @@ describe('validateAndBuildConfiguration', () => { delete (ExperimentalFeature as any).FOO }) - it('updates experimental feature flags', () => { - validateAndBuildConfiguration({ clientToken, enableExperimentalFeatures: ['foo'] }) - expect(isExperimentalFeatureEnabled(TEST_FEATURE_FLAG)).toBeTrue() - }) - it('ignores unknown experimental features', () => { validateAndBuildConfiguration({ clientToken, diff --git a/packages/core/src/domain/configuration/configuration.ts b/packages/core/src/domain/configuration/configuration.ts index bfb63a8728..bd663e97c1 100644 --- a/packages/core/src/domain/configuration/configuration.ts +++ b/packages/core/src/domain/configuration/configuration.ts @@ -1,7 +1,6 @@ import { catchUserErrors } from '../../tools/catchUserErrors' import { DOCS_ORIGIN, display } from '../../tools/display' import type { RawTelemetryConfiguration } from '../telemetry' -import { ExperimentalFeature, addExperimentalFeatures } from '../../tools/experimentalFeatures' import type { Duration } from '../../tools/utils/timeUtils' import { ONE_SECOND } from '../../tools/utils/timeUtils' import { isPercentage } from '../../tools/utils/numberUtils' @@ -260,15 +259,6 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati return } - // Set the experimental feature flags as early as possible, so we can use them in most places - if (Array.isArray(initConfiguration.enableExperimentalFeatures)) { - addExperimentalFeatures( - initConfiguration.enableExperimentalFeatures.filter((flag): flag is ExperimentalFeature => - objectHasValue(ExperimentalFeature, flag) - ) - ) - } - return assign( { beforeSend: diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9c01c3ce50..0976524c07 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -17,6 +17,7 @@ export { addExperimentalFeatures, resetExperimentalFeatures, getExperimentalFeatures, + initFeatureFlags, ExperimentalFeature, } from './tools/experimentalFeatures' export { trackRuntimeError } from './domain/error/trackRuntimeError' diff --git a/packages/core/src/tools/experimentalFeatures.spec.ts b/packages/core/src/tools/experimentalFeatures.spec.ts index 4fdd1a916d..1726165ba5 100644 --- a/packages/core/src/tools/experimentalFeatures.spec.ts +++ b/packages/core/src/tools/experimentalFeatures.spec.ts @@ -1,6 +1,7 @@ -import type { ExperimentalFeature } from './experimentalFeatures' import { + ExperimentalFeature, addExperimentalFeatures, + initFeatureFlags, isExperimentalFeatureEnabled, resetExperimentalFeatures, } from './experimentalFeatures' @@ -32,3 +33,28 @@ describe('experimentalFeatures', () => { expect(isExperimentalFeatureEnabled(TEST_FEATURE_FLAG_TWO)).toBeTrue() }) }) + +describe('initFeatureFlags', () => { + beforeEach(() => { + ;(ExperimentalFeature as any).FOO = TEST_FEATURE_FLAG_ONE + }) + + afterEach(() => { + delete (ExperimentalFeature as any).FOO + resetExperimentalFeatures() + }) + + it('ignores unknown experimental features', () => { + initFeatureFlags(['bar', undefined as any, null as any, 11 as any]) + + expect(isExperimentalFeatureEnabled('bar' as any)).toBeFalse() + expect(isExperimentalFeatureEnabled(undefined as any)).toBeFalse() + expect(isExperimentalFeatureEnabled(null as any)).toBeFalse() + expect(isExperimentalFeatureEnabled(11 as any)).toBeFalse() + }) + + it('updates experimental feature flags', () => { + initFeatureFlags(['foo']) + expect(isExperimentalFeatureEnabled(TEST_FEATURE_FLAG_ONE)).toBeTrue() + }) +}) diff --git a/packages/core/src/tools/experimentalFeatures.ts b/packages/core/src/tools/experimentalFeatures.ts index 2c9e6a79be..c4b40642d9 100644 --- a/packages/core/src/tools/experimentalFeatures.ts +++ b/packages/core/src/tools/experimentalFeatures.ts @@ -9,6 +9,9 @@ */ // We want to use a real enum (i.e. not a const enum) here, to be able to check whether an arbitrary // string is an expected feature flag + +import { objectHasValue } from './utils/objectUtils' + // eslint-disable-next-line no-restricted-syntax export enum ExperimentalFeature { WRITABLE_RESOURCE_GRAPHQL = 'writable_resource_graphql', @@ -19,6 +22,16 @@ export enum ExperimentalFeature { const enabledExperimentalFeatures: Set = new Set() +export function initFeatureFlags(enableExperimentalFeatures?: string[] | undefined) { + if (Array.isArray(enableExperimentalFeatures)) { + addExperimentalFeatures( + enableExperimentalFeatures.filter((flag): flag is ExperimentalFeature => + objectHasValue(ExperimentalFeature, flag) + ) + ) + } +} + export function addExperimentalFeatures(enabledFeatures: ExperimentalFeature[]): void { enabledFeatures.forEach((flag) => { enabledExperimentalFeatures.add(flag) diff --git a/packages/logs/src/boot/preStartLogs.ts b/packages/logs/src/boot/preStartLogs.ts index 36764617b4..db84989f46 100644 --- a/packages/logs/src/boot/preStartLogs.ts +++ b/packages/logs/src/boot/preStartLogs.ts @@ -5,6 +5,7 @@ import { canUseEventBridge, display, displayAlreadyInitializedError, + initFeatureFlags, noop, timeStampNow, } from '@datadog/browser-core' @@ -44,6 +45,8 @@ export function createPreStartStrategy( display.error('Missing configuration') return } + // Set the experimental feature flags as early as possible, so we can use them in most places + initFeatureFlags(initConfiguration.enableExperimentalFeatures) if (canUseEventBridge()) { initConfiguration = overrideInitConfigurationForBridge(initConfiguration) diff --git a/packages/rum-core/src/boot/preStartRum.ts b/packages/rum-core/src/boot/preStartRum.ts index e018135177..25a37dd9c0 100644 --- a/packages/rum-core/src/boot/preStartRum.ts +++ b/packages/rum-core/src/boot/preStartRum.ts @@ -9,6 +9,7 @@ import { clocksNow, assign, getEventBridge, + initFeatureFlags, } from '@datadog/browser-core' import type { TrackingConsentState, DeflateWorker } from '@datadog/browser-core' import { @@ -78,6 +79,9 @@ export function createPreStartStrategy( return } + // Set the experimental feature flags as early as possible, so we can use them in most places + initFeatureFlags(initConfiguration.enableExperimentalFeatures) + const eventBridgeAvailable = canUseEventBridge() if (eventBridgeAvailable) { initConfiguration = overrideInitConfigurationForBridge(initConfiguration)