fix(openfeature): preserve unsupported configuration sources

Author: leoromanovsky

integration-tests/openfeature/openfeature-configuration-sources.spec.js

@@ -65,7 +65,7 @@ const AGENTLESS_RESPONSE = zlib.gzipSync(JSON.stringify({
 }))
 
 /** @typedef {'absent'|'true'|'false'} BooleanSetting */
-/** @typedef {'agentless'|'remote_config'|'disabled'} Delivery */
+/** @typedef {'agentless'|'remote_config'|'disabled'|'error'} Delivery */
 /** @typedef {{ name: string, value?: string }} SourceSetting */
 /**
  * @typedef {object} ConfigurationCase
@@ -135,9 +135,10 @@ describe('OpenFeature configuration source contract', () => {
 
     assert.strictEqual(configurationCases.length, 63)
     assert.deepStrictEqual(countDeliveries(configurationCases), {
-      agentless: 16,
-      remote_config: 16,
-      disabled: 31,
+      agentless: 12,
+      remote_config: 12,
+      disabled: 27,
+      error: 12,
     })
 
     await runCases(configurationCases)
@@ -241,12 +242,19 @@ async function runCase (testCase) {
         waitForObservation(agentlessRequests, testCase.identifier, 'agentless', hasObservation),
         sendCommand(proc, accessCommand),
       ])
+    } else if (testCase.expected === 'error') {
+      await assert.rejects(
+        sendCommand(proc, accessCommand),
+        /Unsupported Feature Flagging configuration source: (offline|unsupported)/
+      )
     } else {
       await sendCommand(proc, accessCommand)
     }
 
-    const { details } = await sendCommand(proc, { command: 'evaluate' })
-    assertEvaluation(testCase, details)
+    if (testCase.expected !== 'error') {
+      const { details } = await sendCommand(proc, { command: 'evaluate' })
+      assertEvaluation(testCase, details)
+    }
 
     await waitForObservation(
       remoteConfigRequests,
@@ -639,6 +647,7 @@ function countDeliveries (cases) {
     agentless: 0,
     remote_config: 0,
     disabled: 0,
+    error: 0,
   }
   for (const testCase of cases) counts[testCase.expected]++
   return counts
@@ -677,6 +686,7 @@ function expectedDelivery (stable, source, legacy) {
   if (stable === 'false') return 'disabled'
   if (source.name === 'agentless') return 'agentless'
   if (source.name === 'remote_config') return 'remote_config'
+  if (source.name === 'offline' || source.name === 'invalid') return 'error'
   if (legacy === 'true') return 'remote_config'
   if (legacy === 'false') return 'disabled'
   return 'agentless'

packages/dd-trace/src/config/defaults.js

@@ -32,13 +32,14 @@ const parseErrors = new Map()
  * @param {string} source - The source of the value.
  * @param {string} baseMessage - The base message to use for the warning.
  * @param {Error} [error] - An error that was thrown while parsing the value.
+ * @param {boolean} [pickedDefault] - Whether the invalid value was discarded in favor of a fallback.
  */
-function warnInvalidValue (value, optionName, source, baseMessage, error) {
+function warnInvalidValue (value, optionName, source, baseMessage, error, pickedDefault = true) {
   const canonicalName = (optionsTable[optionName]?.canonicalName ?? optionName) + source
   // Lazy load log module to avoid circular dependency
   if (!parseErrors.has(canonicalName)) {
-    // TODO: Rephrase: It will fallback to former source (or default if not set)
-    let message = `${baseMessage}: ${util.inspect(value)} for ${optionName} (source: ${source}), picked default`
+    let message = `${baseMessage}: ${util.inspect(value)} for ${optionName} (source: ${source})`
+    if (pickedDefault) message += ', picked default'
     if (error) {
       error.stack = error.toString()
       message += `\n\n${util.inspect(error)}`
@@ -242,8 +243,10 @@ for (const [canonicalName, entries] of Object.entries(supportedConfigurations))
       const originalTransform = transformer
       transformer = (value, optionName, source) => {
         if (!allowed.test(value)) {
-          warnInvalidValue(value, optionName, source, 'Invalid value')
-          return
+          const preserveInvalidSource = canonicalName === 'DD_FEATURE_FLAGS_CONFIGURATION_SOURCE' &&
+            String(value).trim() !== ''
+          warnInvalidValue(value, optionName, source, 'Invalid value', undefined, !preserveInvalidSource)
+          if (!preserveInvalidSource) return
         }
         if (originalTransform) {
           value = originalTransform(value)

packages/dd-trace/src/config/generated-config-types.d.ts

@@ -432,7 +432,7 @@ export interface GeneratedConfig {
     };
   };
   featureFlags: {
-    DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: "agentless" | "remote_config";
+    DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: "agentless" | "remote_config" | "offline";
     DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL: string | undefined;
     DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS: number;
     DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS: number;
@@ -697,7 +697,7 @@ export interface GeneratedEnvVarConfig {
   DD_EXPERIMENTAL_TEST_OPT_VITEST_NO_WORKER_INIT: boolean | undefined;
   DD_EXPERIMENTAL_TEST_REQUESTS_FS_CACHE: boolean;
   DD_EXTERNAL_ENV: string | undefined;
-  DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: "agentless" | "remote_config";
+  DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: "agentless" | "remote_config" | "offline";
   DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL: string | undefined;
   DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS: number;
   DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS: number;

packages/dd-trace/src/config/supported-configurations.json

@@ -840,8 +840,8 @@
         "implementation": "A",
         "type": "string",
         "default": "agentless",
-        "allowed": "agentless|remote_config",
-        "description": "Experimental: Select where Feature Flagging loads Universal Flag Configuration. Supported values are agentless and remote_config.",
+        "allowed": "agentless|remote_config|offline",
+        "description": "Select where Feature Flagging loads Universal Flag Configuration. Supported values are agentless and remote_config; offline is reserved and currently unsupported.",
         "namespace": "featureFlags",
         "transform": "toLowerCase"
       }
@@ -851,7 +851,7 @@
         "implementation": "A",
         "type": "string",
         "default": null,
-        "description": "Experimental: Override the agentless Feature Flagging UFC endpoint or base URL.",
+        "description": "Override the agentless Feature Flagging UFC endpoint or base URL.",
         "namespace": "featureFlags",
         "sensitive": true
       }
@@ -861,7 +861,7 @@
         "implementation": "A",
         "type": "int",
         "default": "30",
-        "description": "Experimental: Set the agentless Feature Flagging UFC polling interval in seconds, capped at one hour.",
+        "description": "Set the agentless Feature Flagging UFC polling interval in seconds, capped at one hour.",
         "allowed": "[1-9]\\d*",
         "namespace": "featureFlags"
       }
@@ -871,7 +871,7 @@
         "implementation": "A",
         "type": "int",
         "default": "5",
-        "description": "Experimental: Set the agentless Feature Flagging UFC request timeout in seconds.",
+        "description": "Set the agentless Feature Flagging UFC request timeout in seconds.",
         "allowed": "[1-9]\\d*",
         "namespace": "featureFlags"
       }

packages/dd-trace/src/openfeature/configuration_source.js

@@ -23,10 +23,14 @@ function create (config, applyConfiguration) {
     DD_FEATURE_FLAGS_ENABLED: enabled,
   } = config.featureFlags
 
-  if (!enabled || source !== 'agentless') {
+  if (!enabled || source === 'remote_config') {
     return
   }
 
+  if (source !== 'agentless') {
+    throw new Error(`Unsupported Feature Flagging configuration source: ${source}`)
+  }
+
   try {
     if (!config.DD_API_KEY) {
       throw new Error('DD_API_KEY is required for Feature Flagging agentless delivery')

packages/dd-trace/test/config/index.spec.js

@@ -5034,16 +5034,26 @@ rules:
           expected: { enabled: true, source: 'remote_config' },
         },
         {
-          name: 'falls back from an invalid source to legacy enablement',
+          name: 'fails closed for an invalid source',
+          source: 'other',
+          expected: { enabled: true, source: 'other' },
+        },
+        {
+          name: 'fails closed for the reserved offline source',
+          source: 'offline',
+          expected: { enabled: true, source: 'offline' },
+        },
+        {
+          name: 'fails closed for an invalid source despite legacy enablement',
           source: 'other',
           legacyEnabled: 'true',
-          expected: { enabled: true, source: 'remote_config' },
+          expected: { enabled: true, source: 'other' },
         },
         {
-          name: 'falls back from the reserved offline source to legacy enablement',
+          name: 'fails closed for the reserved offline source despite legacy enablement',
           source: 'offline',
           legacyEnabled: 'true',
-          expected: { enabled: true, source: 'remote_config' },
+          expected: { enabled: true, source: 'offline' },
         },
       ]) {
       it(name, () => {
@@ -5069,22 +5079,22 @@ rules:
       })
     }
 
-    it('falls back from an invalid source through calculated legacy precedence', () => {
+    it('preserves an explicit invalid source over calculated legacy precedence', () => {
       process.env.DD_FEATURE_FLAGS_ENABLED = 'true'
       process.env.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = 'offline'
       process.env.DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED = 'true'
 
       const config = getConfig()
 
       assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_ENABLED, true)
-      assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE, 'remote_config')
+      assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE, 'offline')
       assert.strictEqual(config.experimental.flaggingProvider.enabled, true)
       assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_ENABLED'), 'env_var')
-      assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE'), 'calculated')
+      assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE'), 'env_var')
       assert.strictEqual(config.getOrigin('experimental.flaggingProvider.enabled'), 'env_var')
       assertConfigUpdateContains(updateConfig.getCall(0).args[0], [
         { name: 'DD_FEATURE_FLAGS_ENABLED', value: true, origin: 'env_var' },
-        { name: 'DD_FEATURE_FLAGS_CONFIGURATION_SOURCE', value: 'remote_config', origin: 'calculated' },
+        { name: 'DD_FEATURE_FLAGS_CONFIGURATION_SOURCE', value: 'offline', origin: 'env_var' },
         { name: 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED', value: true, origin: 'env_var' },
       ])
 

packages/dd-trace/test/openfeature/configuration_source.spec.js

@@ -211,6 +211,18 @@ describe('OpenFeature configuration source', () => {
     sinon.assert.notCalled(AgentlessConfigurationSource)
   })
 
+  for (const source of ['offline', 'invalid']) {
+    it(`throws for the unsupported ${source} source`, () => {
+      config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = source
+
+      assert.throws(
+        () => configurationSource.create(config, sinon.spy()),
+        new RegExp(`Unsupported Feature Flagging configuration source: ${source}`)
+      )
+      sinon.assert.notCalled(AgentlessConfigurationSource)
+    })
+  }
+
   it('does not create an agentless source when Feature Flags are disabled', () => {
     config.featureFlags.DD_FEATURE_FLAGS_ENABLED = false
     delete config.site