From 3d0c53009f4d24d32b913a7c9c82a38d2fe931db Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Wed, 23 Nov 2022 12:58:15 -0800 Subject: [PATCH 1/4] Handle LD feature flags besides `featureService.overwrites` --- .../launchdarkly/LDExperimentService.tsx | 64 +++++++++++++------ 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx index 0c8881f9e3cc..ffae85c3d0b4 100644 --- a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx +++ b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx @@ -12,20 +12,33 @@ import { useAnalyticsService } from "hooks/services/Analytics"; import { useAppMonitoringService, AppActionCodes } from "hooks/services/AppMonitoringService"; import { ExperimentProvider, ExperimentService } from "hooks/services/Experiment"; import type { Experiments } from "hooks/services/Experiment/experiments"; -import { FeatureSet, useFeatureService } from "hooks/services/Feature"; +import { FeatureSet, FeatureItem, useFeatureService } from "hooks/services/Feature"; import { User } from "packages/cloud/lib/domain/users"; import { useAuthService } from "packages/cloud/services/auth/AuthService"; import { rejectAfter } from "utils/promises"; +type LDFeatureName = `${FeatureItem}` | "overwrites"; +type RawLDFeatureName = `${typeof FEATURE_FLAG_PREFIX}.${LDFeatureName}`; +type LDInitState = "initializing" | "failed" | "initialized"; + /** * The maximum time in milliseconds we'll wait for LaunchDarkly to finish initialization, * before running disabling it. */ const INITIALIZATION_TIMEOUT = 5000; -const FEATURE_FLAG_EXPERIMENT = "featureService.overwrites"; - -type LDInitState = "initializing" | "failed" | "initialized"; +// Originally, all feature toggles were contained within one single feature flag in +// LaunchDarkly, using internal conventions to represent multiple feature toggles from a +// single variant. `FEATURE_FLAG_EXPERIMENT` is the LaunchDarkly key for that shared +// feature flag. +// +// This service hardcodes the convention that all feature overrides in LaunchDarkly use +// keys that follow the format `${FEATURE_FLAG_PREFIX}.${FeatureItem}`. The primary +// benefit of requiring the prefix in code is to provide a reliable search term which can +// be used in LaunchDarkly to retrieve the complete set of flags which can be used to +// dynamically toggle UI features. +const FEATURE_FLAG_PREFIX = "featureService"; +const FEATURE_FLAG_EXPERIMENT: RawLDFeatureName = "featureService.overwrites"; function mapUserToLDUser(user: User | null, locale: string): LDClient.LDUser { return user @@ -73,14 +86,30 @@ const LDInitializationWrapper: React.FC { - const featureSet = featureOverwriteString.split(",").reduce((featureSet, featureString) => { - const [key, enabled] = featureString.startsWith("-") ? [featureString.slice(1), false] : [featureString, true]; - return { - ...featureSet, - [key]: enabled, - }; - }, {} as FeatureSet); + const updateFeatureOverwrites = () => { + const { [FEATURE_FLAG_EXPERIMENT]: sharedFeatureOverwrites, ...independentFeatureOverwritesRaw } = + Object.fromEntries( + Object.entries(ldClient.current?.allFlags() ?? {}).filter(([id]) => id.startsWith(FEATURE_FLAG_PREFIX)) + ); + + const sharedFeaturesSet = sharedFeatureOverwrites + .split(",") + .reduce((featureSet: FeatureSet, featureString: string) => { + const [key, enabled] = featureString.startsWith("-") ? [featureString.slice(1), false] : [featureString, true]; + return { + ...featureSet, + [key]: enabled, + }; + }, {} as FeatureSet); + + const independentFeaturesSet = Object.fromEntries( + Object.entries(independentFeatureOverwritesRaw).map(([flag, value]) => [ + flag.replace(`${FEATURE_FLAG_PREFIX}.`, ""), + value, + ]) + ); + + const featureSet: FeatureSet = { ...sharedFeaturesSet, ...independentFeaturesSet }; setFeatureOverwrites(featureSet); }; @@ -98,7 +127,7 @@ const LDInitializationWrapper: React.FC { // If the promise fails, either because LaunchDarkly service fails to initialize, or @@ -112,20 +141,13 @@ const LDInitializationWrapper: React.FC { - const onFeatureServiceCange = (newOverwrites: string) => { - updateFeatureOverwrites(newOverwrites); - }; - ldClient.current?.on(`change:${FEATURE_FLAG_EXPERIMENT}`, onFeatureServiceCange); - return () => ldClient.current?.off(`change:${FEATURE_FLAG_EXPERIMENT}`, onFeatureServiceCange); - }); - useEffectOnce(() => { const onFeatureFlagsChanged = () => { // Update analytics context whenever a flag changes analyticsService.setContext({ experiments: JSON.stringify(ldClient.current?.allFlags()) }); // Check for overwritten i18n messages updateI18nMessages(); + updateFeatureOverwrites(); }; ldClient.current?.on("change", onFeatureFlagsChanged); return () => ldClient.current?.off("change", onFeatureFlagsChanged); From 5c6a1732c91c8a753313bdab64286d614b234413 Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Mon, 28 Nov 2022 20:24:22 -0800 Subject: [PATCH 2/4] Handle 3rd state (use app default) from feature flags --- .../thirdParty/launchdarkly/LDExperimentService.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx index ffae85c3d0b4..2daef2ba666a 100644 --- a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx +++ b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx @@ -102,11 +102,14 @@ const LDInitializationWrapper: React.FC [ - flag.replace(`${FEATURE_FLAG_PREFIX}.`, ""), - value, - ]) + Object.entries(independentFeatureOverwritesRaw) + .map(([flag, { overwrite }]) => [flag.replace(`${FEATURE_FLAG_PREFIX}.`, ""), overwrite]) + .filter(([_, value]) => (typeof value === "undefined" ? false : true)) ); const featureSet: FeatureSet = { ...sharedFeaturesSet, ...independentFeaturesSet }; From 89eb7b73a7b2490ac498a530aeb1efa380a9ad9a Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Tue, 29 Nov 2022 12:05:16 -0800 Subject: [PATCH 3/4] Remove feature toggling based on shared LD flag --- .../launchdarkly/LDExperimentService.tsx | 78 +++++++++---------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx index 2daef2ba666a..1c6a7336eba1 100644 --- a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx +++ b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx @@ -17,8 +17,32 @@ import { User } from "packages/cloud/lib/domain/users"; import { useAuthService } from "packages/cloud/services/auth/AuthService"; import { rejectAfter } from "utils/promises"; -type LDFeatureName = `${FeatureItem}` | "overwrites"; -type RawLDFeatureName = `${typeof FEATURE_FLAG_PREFIX}.${LDFeatureName}`; +/** + * This service hardcodes two conventions about the format of the LaunchDarkly feature + * flags we use to override feature settings: + * 1) each feature flag's key (a unique string which is used as the flag's field name in + * LaunchDarkly's JSON payloads) is a string satisfying the LDFeatureName type. + * 2) for each feature flag, LaunchDarkly will return a JSON blob satisfying the + * LDFeatureToggle type. + * + * The primary benefit of programmatically requiring a specific prefix is to provide a + * reliable search term which can be used in LaunchDarkly to filter the list of feature + * flags to all of, and only, the ones which can dynamically toggle features in the UI. + * + * LDFeatureToggle objects can take three forms, representing the three possible decision + * states LaunchDarkly can provide for a user/feature pair: + * |--------------------------+-----------------------------------------------| + * | `{}` | use the application's default feature setting | + * | `{ "overwrite": true }` | enable the feature | + * | `{ "overwrite": false }` | disable the feature | + * |--------------------------+-----------------------------------------------| + */ +const FEATURE_FLAG_PREFIX = "featureService"; +type LDFeatureName = `${typeof FEATURE_FLAG_PREFIX}.${FeatureItem}`; +interface LDFeatureToggle { + overwrite?: boolean; +} +type LDFeatureFlagResponse = Record; type LDInitState = "initializing" | "failed" | "initialized"; /** @@ -27,19 +51,6 @@ type LDInitState = "initializing" | "failed" | "initialized"; */ const INITIALIZATION_TIMEOUT = 5000; -// Originally, all feature toggles were contained within one single feature flag in -// LaunchDarkly, using internal conventions to represent multiple feature toggles from a -// single variant. `FEATURE_FLAG_EXPERIMENT` is the LaunchDarkly key for that shared -// feature flag. -// -// This service hardcodes the convention that all feature overrides in LaunchDarkly use -// keys that follow the format `${FEATURE_FLAG_PREFIX}.${FeatureItem}`. The primary -// benefit of requiring the prefix in code is to provide a reliable search term which can -// be used in LaunchDarkly to retrieve the complete set of flags which can be used to -// dynamically toggle UI features. -const FEATURE_FLAG_PREFIX = "featureService"; -const FEATURE_FLAG_EXPERIMENT: RawLDFeatureName = "featureService.overwrites"; - function mapUserToLDUser(user: User | null, locale: string): LDClient.LDUser { return user ? { @@ -82,37 +93,20 @@ const LDInitializationWrapper: React.FC { - const { [FEATURE_FLAG_EXPERIMENT]: sharedFeatureOverwrites, ...independentFeatureOverwritesRaw } = - Object.fromEntries( - Object.entries(ldClient.current?.allFlags() ?? {}).filter(([id]) => id.startsWith(FEATURE_FLAG_PREFIX)) - ); - - const sharedFeaturesSet = sharedFeatureOverwrites - .split(",") - .reduce((featureSet: FeatureSet, featureString: string) => { - const [key, enabled] = featureString.startsWith("-") ? [featureString.slice(1), false] : [featureString, true]; - return { - ...featureSet, - [key]: enabled, - }; - }, {} as FeatureSet); - - // by convention, feature flags return one of three payloads, each with its own meaning: - // 1) `{ "overwrite": true }` :: enable the feature - // 2) `{ "overwrite": false }` :: disable the feature - // 3) `{}` :: `overwrite` is `undefined`, i.e. continue to use the application's default feature state - const independentFeaturesSet = Object.fromEntries( - Object.entries(independentFeatureOverwritesRaw) - .map(([flag, { overwrite }]) => [flag.replace(`${FEATURE_FLAG_PREFIX}.`, ""), overwrite]) - .filter(([_, value]) => (typeof value === "undefined" ? false : true)) + const allFlags = (ldClient.current?.allFlags() ?? {}) as LDFeatureFlagResponse; + const featureSet: FeatureSet = Object.fromEntries( + Object.entries(allFlags) + .filter(([flagName]) => flagName.startsWith(FEATURE_FLAG_PREFIX)) + .map(([flagName, { overwrite }]) => [flagName.replace(`${FEATURE_FLAG_PREFIX}.`, ""), overwrite]) + .filter(([_, overwrite]) => typeof overwrite !== "undefined") ); - const featureSet: FeatureSet = { ...sharedFeaturesSet, ...independentFeaturesSet }; setFeatureOverwrites(featureSet); }; From d3d5441a51363591d73298cfb8769c7c6cdc1faf Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Tue, 29 Nov 2022 15:04:41 -0800 Subject: [PATCH 4/4] s/overwrite/enabled/ in the new LD flag payloads The purpose of the value and the contextual meaning of `true` and `false` just read more easily with this phrasing. --- .../launchdarkly/LDExperimentService.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx index 1c6a7336eba1..f30becf73f6c 100644 --- a/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx +++ b/airbyte-webapp/src/packages/cloud/services/thirdParty/launchdarkly/LDExperimentService.tsx @@ -33,14 +33,14 @@ import { rejectAfter } from "utils/promises"; * states LaunchDarkly can provide for a user/feature pair: * |--------------------------+-----------------------------------------------| * | `{}` | use the application's default feature setting | - * | `{ "overwrite": true }` | enable the feature | - * | `{ "overwrite": false }` | disable the feature | + * | `{ "enabled": true }` | enable the feature | + * | `{ "enabled": false }` | disable the feature | * |--------------------------+-----------------------------------------------| */ const FEATURE_FLAG_PREFIX = "featureService"; type LDFeatureName = `${typeof FEATURE_FLAG_PREFIX}.${FeatureItem}`; interface LDFeatureToggle { - overwrite?: boolean; + enabled?: boolean; } type LDFeatureFlagResponse = Record; type LDInitState = "initializing" | "failed" | "initialized"; @@ -93,18 +93,18 @@ const LDInitializationWrapper: React.FC { const allFlags = (ldClient.current?.allFlags() ?? {}) as LDFeatureFlagResponse; const featureSet: FeatureSet = Object.fromEntries( Object.entries(allFlags) .filter(([flagName]) => flagName.startsWith(FEATURE_FLAG_PREFIX)) - .map(([flagName, { overwrite }]) => [flagName.replace(`${FEATURE_FLAG_PREFIX}.`, ""), overwrite]) - .filter(([_, overwrite]) => typeof overwrite !== "undefined") + .map(([flagName, { enabled }]) => [flagName.replace(`${FEATURE_FLAG_PREFIX}.`, ""), enabled]) + .filter(([_, enabled]) => typeof enabled !== "undefined") ); setFeatureOverwrites(featureSet);