From 1115733b9a70dad332c6b1bdce1d04fc623713ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Wed, 16 Oct 2024 13:08:54 +0200 Subject: [PATCH 1/4] [Feature flags example] Apply FF naming conventions --- examples/feature_flags_example/common/feature_flags.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/feature_flags_example/common/feature_flags.ts b/examples/feature_flags_example/common/feature_flags.ts index fcff25bbd2c42..5ee0a9c6b50dc 100644 --- a/examples/feature_flags_example/common/feature_flags.ts +++ b/examples/feature_flags_example/common/feature_flags.ts @@ -7,6 +7,6 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -export const FeatureFlagExampleBoolean = 'example-boolean'; -export const FeatureFlagExampleString = 'example-string'; -export const FeatureFlagExampleNumber = 'example-number'; +export const FeatureFlagExampleBoolean = 'featureFlagsExample.exampleBoolean'; +export const FeatureFlagExampleString = 'featureFlagsExample.exampleString'; +export const FeatureFlagExampleNumber = 'featureFlagsExample.exampleNumber'; From b30566b462562efa5ae9fac993231c7c1f29ec63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Wed, 16 Oct 2024 13:24:59 +0200 Subject: [PATCH 2/4] Support dotted feature flag names in config overrides --- packages/core/feature-flags/README.mdx | 6 +++--- .../src/feature_flags_service.test.ts | 15 ++++++++++++++- .../src/feature_flags_service.ts | 6 ++++-- .../src/feature_flags_service.test.ts | 19 ++++++++++++++++++- .../src/feature_flags_service.ts | 6 ++++-- 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/packages/core/feature-flags/README.mdx b/packages/core/feature-flags/README.mdx index 6765ef31afca9..61a86d41b4872 100644 --- a/packages/core/feature-flags/README.mdx +++ b/packages/core/feature-flags/README.mdx @@ -32,7 +32,7 @@ import type { PluginInitializerContext } from '@kbn/core-plugins-server'; export const featureFlags: FeatureFlagDefinitions = [ { - key: 'my-cool-feature', + key: 'myPlugin.myCoolFeature', name: 'My cool feature', description: 'Enables the cool feature to auto-hide the navigation bar', tags: ['my-plugin', 'my-service', 'ui'], @@ -118,7 +118,7 @@ async (context, request, response) => { const { featureFlags } = await context.core; return response.ok({ body: { - number: await featureFlags.getNumberValue('example-number', 1), + number: await featureFlags.getNumberValue('myPlugin.exampleNumber', 1), }, }); } @@ -142,7 +142,7 @@ provider. In the `kibana.yml`, the following config sets the overrides: ```yaml feature_flags.overrides: - my-feature-flag: 'my-forced-value' + myPlugin.myFeatureFlag: 'my-forced-value' ``` > [!WARNING] diff --git a/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.test.ts b/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.test.ts index 596d64c7b77ae..3f14a2dd92269 100644 --- a/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.test.ts +++ b/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.test.ts @@ -188,7 +188,11 @@ describe('FeatureFlagsService Browser', () => { beforeEach(async () => { addHandlerSpy = jest.spyOn(featureFlagsClient, 'addHandler'); injectedMetadata.getFeatureFlags.mockReturnValue({ - overrides: { 'my-overridden-flag': true }, + overrides: { + 'my-overridden-flag': true, + 'myPlugin.myOverriddenFlag': true, + myDestructuredObjPlugin: { myOverriddenFlag: true }, + }, }); featureFlagsService.setup({ injectedMetadata }); startContract = await featureFlagsService.start(); @@ -288,5 +292,14 @@ describe('FeatureFlagsService Browser', () => { expect(getBooleanValueSpy).toHaveBeenCalledTimes(1); expect(getBooleanValueSpy).toHaveBeenCalledWith('another-flag', false); }); + + test('overrides with dotted names', async () => { + const getBooleanValueSpy = jest.spyOn(featureFlagsClient, 'getBooleanValue'); + expect(startContract.getBooleanValue('myPlugin.myOverriddenFlag', false)).toEqual(true); + expect( + startContract.getBooleanValue('myDestructuredObjPlugin.myOverriddenFlag', false) + ).toEqual(true); + expect(getBooleanValueSpy).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.ts b/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.ts index 0f7e572ef5ce0..e282cc52ddd4e 100644 --- a/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.ts +++ b/packages/core/feature-flags/core-feature-flags-browser-internal/src/feature_flags_service.ts @@ -20,6 +20,7 @@ import { apm } from '@elastic/apm-rum'; import { type Client, ClientProviderEvents, OpenFeature } from '@openfeature/web-sdk'; import deepMerge from 'deepmerge'; import { filter, map, startWith, Subject } from 'rxjs'; +import { get } from 'lodash'; /** * setup method dependencies @@ -172,9 +173,10 @@ export class FeatureFlagsService { flagName: string, fallbackValue: T ): T { + const override = get(this.overrides, flagName); // using lodash get because flagName can come with dots and the config parser might structure it in objects. const value = - typeof this.overrides[flagName] !== 'undefined' - ? (this.overrides[flagName] as T) + typeof override !== 'undefined' + ? (override as T) : // We have to bind the evaluation or the client will lose its internal context evaluationFn.bind(this.featureFlagsClient)(flagName, fallbackValue); apm.addLabels({ [`flag_${flagName}`]: value }); diff --git a/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.test.ts b/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.test.ts index 7bad676b9528b..f3059571570bc 100644 --- a/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.test.ts +++ b/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.test.ts @@ -27,6 +27,8 @@ describe('FeatureFlagsService Server', () => { atPath: { overrides: { 'my-overridden-flag': true, + 'myPlugin.myOverriddenFlag': true, + myDestructuredObjPlugin: { myOverriddenFlag: true }, }, }, }), @@ -251,10 +253,25 @@ describe('FeatureFlagsService Server', () => { expect(getBooleanValueSpy).toHaveBeenCalledTimes(1); expect(getBooleanValueSpy).toHaveBeenCalledWith('another-flag', false); }); + + test('overrides with dotted names', async () => { + const getBooleanValueSpy = jest.spyOn(featureFlagsClient, 'getBooleanValue'); + await expect( + startContract.getBooleanValue('myPlugin.myOverriddenFlag', false) + ).resolves.toEqual(true); + await expect( + startContract.getBooleanValue('myDestructuredObjPlugin.myOverriddenFlag', false) + ).resolves.toEqual(true); + expect(getBooleanValueSpy).not.toHaveBeenCalled(); + }); }); test('returns overrides', () => { const { getOverrides } = featureFlagsService.setup(); - expect(getOverrides()).toStrictEqual({ 'my-overridden-flag': true }); + expect(getOverrides()).toStrictEqual({ + 'my-overridden-flag': true, + 'myPlugin.myOverriddenFlag': true, + myDestructuredObjPlugin: { myOverriddenFlag: true }, + }); }); }); diff --git a/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.ts b/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.ts index 7b01ebde731fe..f5cb714f56234 100644 --- a/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.ts +++ b/packages/core/feature-flags/core-feature-flags-server-internal/src/feature_flags_service.ts @@ -24,6 +24,7 @@ import { } from '@openfeature/server-sdk'; import deepMerge from 'deepmerge'; import { filter, switchMap, startWith, Subject } from 'rxjs'; +import { get } from 'lodash'; import { type FeatureFlagsConfig, featureFlagsConfig } from './feature_flags_config'; /** @@ -165,9 +166,10 @@ export class FeatureFlagsService { flagName: string, fallbackValue: T ): Promise { + const override = get(this.overrides, flagName); // using lodash get because flagName can come with dots and the config parser might structure it in objects. const value = - typeof this.overrides[flagName] !== 'undefined' - ? (this.overrides[flagName] as T) + typeof override !== 'undefined' + ? (override as T) : // We have to bind the evaluation or the client will lose its internal context await evaluationFn.bind(this.featureFlagsClient)(flagName, fallbackValue); apm.addLabels({ [`flag_${flagName}`]: value }); From 7b35edf95adc5d3b426297a216db2a50e79ffdc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Wed, 16 Oct 2024 17:16:20 +0200 Subject: [PATCH 3/4] Add rule of thumb for when NOT to use feature flags --- packages/core/feature-flags/README.mdx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/core/feature-flags/README.mdx b/packages/core/feature-flags/README.mdx index 61a86d41b4872..37953b8b2ee6a 100644 --- a/packages/core/feature-flags/README.mdx +++ b/packages/core/feature-flags/README.mdx @@ -3,7 +3,7 @@ id: kibFeatureFlagsService slug: /kibana-dev-docs/tutorials/feature-flags-service title: Feature Flags service description: The Feature Flags service provides the necessary APIs to evaluate dynamic feature flags. -date: 2024-07-26 +date: 2024-10-16 tags: ['kibana', 'dev', 'contributor', 'api docs', 'a/b testing', 'feature flags', 'flags'] --- @@ -14,6 +14,11 @@ The Feature Flags service provides the necessary APIs to evaluate dynamic featur The service is always enabled, however, it will return the fallback value if a feature flags provider hasn't been attached. Kibana only registers a provider when running on Elastic Cloud Hosted/Serverless. +:warning: Feature Flags are considered dynamic configuration and cannot be used for settings that require restarting Kibana. +One example of invalid use cases are settings used during the `setup` lifecycle of the plugin, such as settings that define +if an HTTP route is registered or not. Instead, you should always register the route, and return `404 - Not found` in the route +handler if the feature flag returns a _disabled_ state. + For a code example, refer to the [Feature Flags Example plugin](../../../examples/feature_flags_example) ## Registering a feature flag From 37ae3bc199a9841f70af0a4419c37a5cd71f3f9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Wed, 16 Oct 2024 18:45:09 +0200 Subject: [PATCH 4/4] Add callout for airgapped environments --- packages/core/feature-flags/README.mdx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/feature-flags/README.mdx b/packages/core/feature-flags/README.mdx index 37953b8b2ee6a..9d33e5cd53300 100644 --- a/packages/core/feature-flags/README.mdx +++ b/packages/core/feature-flags/README.mdx @@ -12,7 +12,8 @@ tags: ['kibana', 'dev', 'contributor', 'api docs', 'a/b testing', 'feature flags The Feature Flags service provides the necessary APIs to evaluate dynamic feature flags. The service is always enabled, however, it will return the fallback value if a feature flags provider hasn't been attached. -Kibana only registers a provider when running on Elastic Cloud Hosted/Serverless. +Kibana only registers a provider when running on Elastic Cloud Hosted/Serverless. And even in those scenarios, we expect that some customers might +have network restrictions that might not allow the flags to evaluate. The fallback value must provide a non-broken experience to users. :warning: Feature Flags are considered dynamic configuration and cannot be used for settings that require restarting Kibana. One example of invalid use cases are settings used during the `setup` lifecycle of the plugin, such as settings that define