Skip to content

Commit 23a9e2b

Browse files
committed
fix(openfeature): separate enablement from source selection
1 parent 4f57fe8 commit 23a9e2b

3 files changed

Lines changed: 38 additions & 33 deletions

File tree

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

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
const log = require('../log')
44

55
const CONFIGURATION_SOURCE_AGENTLESS = 'agentless'
6-
const CONFIGURATION_SOURCE_DISABLED = 'disabled'
76
const CONFIGURATION_SOURCE_REMOTE_CONFIG = 'remote_config'
87

8+
const DISABLED_RESOLUTION = Object.freeze({ enabled: false })
9+
const AGENTLESS_CONFIGURATION = Object.freeze({ enabled: true, source: CONFIGURATION_SOURCE_AGENTLESS })
10+
const REMOTE_CONFIG_CONFIGURATION = Object.freeze({ enabled: true, source: CONFIGURATION_SOURCE_REMOTE_CONFIG })
11+
912
const DEFAULT_AGENTLESS_PATH = '/api/v2/feature-flagging/config/rules-based/server'
1013
const DEFAULT_POLL_INTERVAL_SECONDS = 30
1114
const DEFAULT_REQUEST_TIMEOUT_SECONDS = 2
@@ -18,14 +21,14 @@ const MAX_POLL_INTERVAL_SECONDS = 60 * 60
1821
* @returns {object} Resolved source settings.
1922
*/
2023
function resolve (config) {
21-
const mode = resolveMode(config)
24+
const configuration = resolveConfiguration(config)
2225

23-
if (mode !== CONFIGURATION_SOURCE_AGENTLESS) {
24-
return { mode }
26+
if (!configuration.enabled || configuration.source !== CONFIGURATION_SOURCE_AGENTLESS) {
27+
return configuration
2528
}
2629

2730
return {
28-
mode,
31+
...configuration,
2932
endpoint: endpoint(config, config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL),
3033
pollIntervalMs: positiveMilliseconds(
3134
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS,
@@ -61,7 +64,7 @@ function enable (config, getOpenfeatureProxy) {
6164
return
6265
}
6366

64-
if (sourceConfig.mode === CONFIGURATION_SOURCE_AGENTLESS) {
67+
if (sourceConfig.source === CONFIGURATION_SOURCE_AGENTLESS) {
6568
const AgentlessConfigurationSource = require('./agentless_configuration_source')
6669
const source = new AgentlessConfigurationSource(sourceConfig, ufc => {
6770
getOpenfeatureProxy()._setConfiguration(ufc)
@@ -80,7 +83,7 @@ function enable (config, getOpenfeatureProxy) {
8083
*/
8184
function isRemoteConfig (config) {
8285
try {
83-
return resolveMode(config) === CONFIGURATION_SOURCE_REMOTE_CONFIG
86+
return resolveConfiguration(config).source === CONFIGURATION_SOURCE_REMOTE_CONFIG
8487
} catch (error) {
8588
log.error('Unable to configure Feature Flagging configuration source', error)
8689
return false
@@ -97,7 +100,7 @@ function isRemoteConfig (config) {
97100
*/
98101
function isEnabled (config) {
99102
try {
100-
return resolveMode(config) !== CONFIGURATION_SOURCE_DISABLED
103+
return resolveConfiguration(config).enabled
101104
} catch (error) {
102105
log.error('Unable to configure Feature Flagging configuration source', error)
103106
return false
@@ -109,10 +112,10 @@ function isEnabled (config) {
109112
* endpoint or timing configuration.
110113
*
111114
* @param {import('../config/config-base')} config - Tracer configuration.
112-
* @returns {string} Selected configuration-source mode.
115+
* @returns {{enabled: boolean, source?: string}} Resolved enablement and source selection.
113116
*/
114-
function resolveMode (config) {
115-
if (config.DD_FEATURE_FLAGS_ENABLED === false) return CONFIGURATION_SOURCE_DISABLED
117+
function resolveConfiguration (config) {
118+
if (config.DD_FEATURE_FLAGS_ENABLED === false) return DISABLED_RESOLUTION
116119

117120
const value = config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE
118121
const origin = config.getOrigin?.('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE')
@@ -126,16 +129,15 @@ function resolveMode (config) {
126129
: legacyEnabled !== undefined && legacyEnabled !== null
127130

128131
if (hasExplicitLegacySetting) {
129-
if (legacyEnabled === true) return CONFIGURATION_SOURCE_REMOTE_CONFIG
130-
if (legacyEnabled === false) return CONFIGURATION_SOURCE_DISABLED
132+
if (legacyEnabled === true) return REMOTE_CONFIG_CONFIGURATION
133+
if (legacyEnabled === false) return DISABLED_RESOLUTION
131134
}
132135
}
133136

134-
const mode = String(value ?? '').trim().toLowerCase() || CONFIGURATION_SOURCE_AGENTLESS
135-
if (mode !== CONFIGURATION_SOURCE_AGENTLESS && mode !== CONFIGURATION_SOURCE_REMOTE_CONFIG) {
136-
throw new Error(`Unsupported Feature Flagging configuration source: ${mode}`)
137-
}
138-
return mode
137+
const source = String(value ?? '').trim().toLowerCase() || CONFIGURATION_SOURCE_AGENTLESS
138+
if (source === CONFIGURATION_SOURCE_AGENTLESS) return AGENTLESS_CONFIGURATION
139+
if (source === CONFIGURATION_SOURCE_REMOTE_CONFIG) return REMOTE_CONFIG_CONFIGURATION
140+
throw new Error(`Unsupported Feature Flagging configuration source: ${source}`)
139141
}
140142

141143
/**

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4950,6 +4950,8 @@ rules:
49504950
DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS: 2,
49514951
})
49524952
assert.strictEqual(config.experimental.flaggingProvider.enabled, false)
4953+
assert.strictEqual(config.getOrigin('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE'), 'default')
4954+
assert.strictEqual(config.getOrigin('experimental.flaggingProvider.enabled'), 'default')
49534955
})
49544956

49554957
it('reads the stable provider kill switch', () => {
@@ -4958,6 +4960,7 @@ rules:
49584960
const config = getConfig()
49594961

49604962
assert.strictEqual(config.DD_FEATURE_FLAGS_ENABLED, false)
4963+
assert.strictEqual(config.getOrigin('DD_FEATURE_FLAGS_ENABLED'), 'env_var')
49614964
})
49624965

49634966
for (const value of ['true', 'false']) {
@@ -4968,6 +4971,7 @@ rules:
49684971

49694972
assert.strictEqual(config.DD_FEATURE_FLAGS_ENABLED, true)
49704973
assert.strictEqual(config.experimental.flaggingProvider.enabled, value === 'true')
4974+
assert.strictEqual(config.getOrigin('experimental.flaggingProvider.enabled'), 'env_var')
49714975
})
49724976
}
49734977

@@ -4977,6 +4981,7 @@ rules:
49774981
const config = getConfig()
49784982

49794983
assert.strictEqual(config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE, 'remote_config')
4984+
assert.strictEqual(config.getOrigin('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE'), 'env_var')
49804985
})
49814986

49824987
it('reads the canonical agentless environment variables', () => {

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ describe('OpenFeature configuration source', () => {
4646
it(`normalizes ${JSON.stringify(value)} to the default agentless source`, () => {
4747
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = value
4848

49-
assert.strictEqual(configurationSource.resolve(config).mode, 'agentless')
49+
assert.strictEqual(configurationSource.resolve(config).source, 'agentless')
5050
})
5151
}
5252

5353
it('grandfathers the legacy enabled setting onto Remote Config when the source is defaulted', () => {
5454
config.experimental.flaggingProvider.enabled = true
5555
config.getOrigin.withArgs('experimental.flaggingProvider.enabled').returns('env_var')
5656

57-
assert.deepStrictEqual(configurationSource.resolve(config), { mode: 'remote_config' })
57+
assert.deepStrictEqual(configurationSource.resolve(config), { enabled: true, source: 'remote_config' })
5858
assert.strictEqual(configurationSource.isRemoteConfig(config), true)
5959
assert.strictEqual(configurationSource.isEnabled(config), true)
6060
})
@@ -63,7 +63,7 @@ describe('OpenFeature configuration source', () => {
6363
config.experimental.flaggingProvider.enabled = false
6464
config.getOrigin.withArgs('experimental.flaggingProvider.enabled').returns('env_var')
6565

66-
assert.deepStrictEqual(configurationSource.resolve(config), { mode: 'disabled' })
66+
assert.deepStrictEqual(configurationSource.resolve(config), { enabled: false })
6767
assert.strictEqual(configurationSource.isRemoteConfig(config), false)
6868
assert.strictEqual(configurationSource.isEnabled(config), false)
6969
})
@@ -72,7 +72,7 @@ describe('OpenFeature configuration source', () => {
7272
config.experimental.flaggingProvider.enabled = true
7373
config.getOrigin.returns('env_var')
7474

75-
assert.strictEqual(configurationSource.resolve(config).mode, 'agentless')
75+
assert.strictEqual(configurationSource.resolve(config).source, 'agentless')
7676
assert.strictEqual(configurationSource.isRemoteConfig(config), false)
7777
assert.strictEqual(configurationSource.isEnabled(config), true)
7878
})
@@ -82,7 +82,7 @@ describe('OpenFeature configuration source', () => {
8282
config.experimental.flaggingProvider.enabled = false
8383
config.getOrigin.returns('env_var')
8484

85-
assert.deepStrictEqual(configurationSource.resolve(config), { mode: 'remote_config' })
85+
assert.deepStrictEqual(configurationSource.resolve(config), { enabled: true, source: 'remote_config' })
8686
assert.strictEqual(configurationSource.isRemoteConfig(config), true)
8787
assert.strictEqual(configurationSource.isEnabled(config), true)
8888
})
@@ -93,7 +93,7 @@ describe('OpenFeature configuration source', () => {
9393
config.experimental.flaggingProvider.enabled = true
9494
config.getOrigin.returns('env_var')
9595

96-
assert.deepStrictEqual(configurationSource.resolve(config), { mode: 'disabled' })
96+
assert.deepStrictEqual(configurationSource.resolve(config), { enabled: false })
9797
assert.strictEqual(configurationSource.isRemoteConfig(config), false)
9898
assert.strictEqual(configurationSource.isEnabled(config), false)
9999
})
@@ -206,19 +206,17 @@ describe('OpenFeature configuration source', () => {
206206
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = ' REMOTE_CONFIG '
207207
delete config.site
208208

209-
assert.deepStrictEqual(configurationSource.resolve(config), { mode: 'remote_config' })
209+
assert.deepStrictEqual(configurationSource.resolve(config), { enabled: true, source: 'remote_config' })
210210
assert.strictEqual(configurationSource.isRemoteConfig(config), true)
211211
})
212212

213-
for (const value of ['offline', 'other']) {
214-
it(`fails closed for the unsupported ${value} source`, () => {
215-
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = value
213+
it('fails closed for an unsupported source', () => {
214+
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = 'other'
216215

217-
assert.throws(() => configurationSource.resolve(config), /Unsupported Feature Flagging configuration source/)
218-
assert.strictEqual(configurationSource.isRemoteConfig(config), false)
219-
sinon.assert.calledOnce(log.error)
220-
})
221-
}
216+
assert.throws(() => configurationSource.resolve(config), /Unsupported Feature Flagging configuration source/)
217+
assert.strictEqual(configurationSource.isRemoteConfig(config), false)
218+
sinon.assert.calledOnce(log.error)
219+
})
222220

223221
it('falls back to positive timing defaults with warnings', () => {
224222
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS = 0

0 commit comments

Comments
 (0)