Skip to content

Commit 7647879

Browse files
leoromanovskyleoromanovsky
committed
fix(feature-flags): separate enablement from source selection
1 parent b32988c commit 7647879

8 files changed

Lines changed: 93 additions & 78 deletions

File tree

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,9 +1656,9 @@ private static boolean isFeatureFlaggingEnabled() {
16561656
final Boolean legacyProviderEnabled =
16571657
featureFlaggingBooleanSetting(FeatureFlaggingConfig.EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED);
16581658

1659-
return !FeatureFlaggingConfig.CONFIGURATION_SOURCE_OFFLINE.equals(
1660-
FeatureFlaggingConfig.resolveConfigurationSource(
1661-
providerEnabled, configurationSource, legacyProviderEnabled));
1659+
return FeatureFlaggingConfig.resolveConfiguration(
1660+
providerEnabled, configurationSource, legacyProviderEnabled)
1661+
.isEnabled();
16621662
}
16631663

16641664
@SuppressFBWarnings(

‎internal-api/src/main/java/datadog/trace/api/Config.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -725,15 +725,14 @@
725725
import static datadog.trace.api.config.TracerConfig.WRITER_BAGGAGE_INJECT;
726726
import static datadog.trace.api.config.TracerConfig.WRITER_LINKS_INJECT;
727727
import static datadog.trace.api.config.TracerConfig.WRITER_TYPE;
728-
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_OFFLINE;
729728
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED;
730729
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE;
731730
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL;
732731
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS;
733732
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS;
734733
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_ENABLED;
735734
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.isSupportedConfigurationSource;
736-
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.resolveConfigurationSource;
735+
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.resolveConfiguration;
737736
import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
738737
import static datadog.trace.bootstrap.instrumentation.api.WriterConstants.OTLP_WRITER_TYPE;
739738
import static datadog.trace.util.CollectionUtils.tryMakeImmutableList;
@@ -750,6 +749,7 @@
750749
import datadog.trace.api.config.OtlpConfig;
751750
import datadog.trace.api.config.ProfilingConfig;
752751
import datadog.trace.api.config.TracerConfig;
752+
import datadog.trace.api.featureflag.config.FeatureFlaggingConfig;
753753
import datadog.trace.api.iast.IastContext;
754754
import datadog.trace.api.iast.IastDetectionMode;
755755
import datadog.trace.api.iast.telemetry.Verbosity;
@@ -2864,8 +2864,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
28642864
configProvider.isSet(FEATURE_FLAGS_CONFIGURATION_SOURCE)
28652865
? configProvider.getString(FEATURE_FLAGS_CONFIGURATION_SOURCE)
28662866
: null;
2867-
final String resolvedFeatureFlaggingConfigurationSource =
2868-
resolveConfigurationSource(
2867+
final FeatureFlaggingConfig.Resolution resolvedFeatureFlaggingConfiguration =
2868+
resolveConfiguration(
28692869
configuredFeatureFlaggingProviderEnabled,
28702870
configuredFeatureFlaggingConfigurationSource,
28712871
legacyFeatureFlaggingProviderEnabled);
@@ -2881,9 +2881,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
28812881
"Unsupported Feature Flagging configuration source: {}. Disabling Feature Flagging",
28822882
configuredFeatureFlaggingConfigurationSource);
28832883
}
2884-
featureFlaggingProviderEnabled =
2885-
!CONFIGURATION_SOURCE_OFFLINE.equals(resolvedFeatureFlaggingConfigurationSource);
2886-
featureFlaggingConfigurationSource = resolvedFeatureFlaggingConfigurationSource;
2884+
featureFlaggingProviderEnabled = resolvedFeatureFlaggingConfiguration.isEnabled();
2885+
featureFlaggingConfigurationSource = resolvedFeatureFlaggingConfiguration.getSource();
28872886
featureFlaggingConfigurationSourceAgentlessBaseUrl =
28882887
configProvider.getStringNotEmpty(
28892888
FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL, null);

‎internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3519,7 +3519,7 @@ class ConfigTest extends DDSpecification {
35193519
"" | "agentless"
35203520
" " | "agentless"
35213521
" ReMoTe_ConFiG " | "remote_config"
3522-
"not-a-real-source" | "offline"
3522+
"not-a-real-source" | null
35233523
}
35243524
35253525
def "feature flag configuration applies migration precedence"() {
@@ -3546,12 +3546,12 @@ class ConfigTest extends DDSpecification {
35463546
providerEnabled | source | legacyProviderEnabled | expectedEnabled | expectedSource
35473547
null | null | null | true | "agentless"
35483548
null | null | true | true | "remote_config"
3549-
null | null | false | false | "offline"
3549+
null | null | false | false | null
35503550
null | "agentless" | true | true | "agentless"
35513551
null | "remote_config" | false | true | "remote_config"
3552-
false | "agentless" | true | false | "offline"
3553-
true | null | false | false | "offline"
3554-
null | "offline" | null | false | "offline"
3552+
false | "agentless" | true | false | null
3553+
true | null | false | false | null
3554+
null | "not-a-source" | null | false | null
35553555
}
35563556
35573557
def "agentless feature flag timing falls back for non-positive values"() {

‎products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.datadog.featureflag;
22

33
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_AGENTLESS;
4-
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_OFFLINE;
54
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_REMOTE_CONFIG;
65

76
import datadog.communication.ddagent.SharedCommunicationObjects;
@@ -29,13 +28,13 @@ public static synchronized void start(final SharedCommunicationObjects sco) {
2928
}
3029
LOGGER.debug("Feature Flagging system starting");
3130
final Config config = Config.get();
32-
final String source = config.getFeatureFlaggingConfigurationSource();
3331
STARTED = true;
3432

35-
if (CONFIGURATION_SOURCE_OFFLINE.equals(source)) {
33+
if (!config.isFeatureFlaggingProviderEnabled()) {
3634
LOGGER.debug("Feature Flagging system disabled");
3735
return;
3836
}
37+
final String source = config.getFeatureFlaggingConfigurationSource();
3938
if (CONFIGURATION_SOURCE_AGENTLESS.equals(source)) {
4039
final FeatureFlaggingGateway.ActivationListener activationListener =
4140
() -> activateAgentless(sco, config);
@@ -119,9 +118,8 @@ static ConfigurationSourceService createConfigurationSourceService(
119118
if (CONFIGURATION_SOURCE_AGENTLESS.equals(configurationSource)) {
120119
return new AgentlessConfigurationSource(config);
121120
}
122-
LOGGER.debug(
123-
"Feature Flagging offline configuration source selected; no config service started");
124-
return null;
121+
throw new IllegalArgumentException(
122+
"Unsupported Feature Flagging configuration source: " + configurationSource);
125123
}
126124

127125
public static synchronized void stop() {

‎products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java‎

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIGURATION_ENABLED;
44
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE;
55
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL;
6-
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
76
import static org.junit.jupiter.api.Assertions.assertFalse;
87
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
98
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -121,37 +120,28 @@ void explicitRemoteConfigUsesRemoteConfigService() {
121120
@Test
122121
@WithConfig(key = FEATURE_FLAGS_CONFIGURATION_SOURCE, value = "invalid")
123122
void invalidConfigurationSourceFailsClosed() {
124-
assertNull(
125-
FeatureFlaggingSystem.createConfigurationSourceService(
126-
sharedCommunicationObjects(), Config.get()));
123+
final Config config = Config.get();
124+
assertFalse(config.isFeatureFlaggingProviderEnabled());
125+
assertNull(config.getFeatureFlaggingConfigurationSource());
126+
127+
try {
128+
FeatureFlaggingSystem.start(sharedCommunicationObjects());
129+
assertFalse(FeatureFlaggingSystem.isAwaitingApplicationActivation());
130+
} finally {
131+
FeatureFlaggingSystem.stop();
132+
}
127133
}
128134

129135
@Test
130136
void rejectsUnsupportedNormalizedConfigurationSource() {
131137
Config config = mock(Config.class);
132138
when(config.getFeatureFlaggingConfigurationSource()).thenReturn("invalid");
133139

134-
assertNull(
135-
FeatureFlaggingSystem.createConfigurationSourceService(
136-
sharedCommunicationObjects(), config));
137-
}
138-
139-
@Test
140-
@WithConfig(key = FEATURE_FLAGS_CONFIGURATION_SOURCE, value = "offline")
141-
void offlineConfigurationSourceDoesNotStartNetworkSource() {
142-
assertNull(
143-
FeatureFlaggingSystem.createConfigurationSourceService(
144-
sharedCommunicationObjects(), Config.get()));
145-
}
146-
147-
@Test
148-
@WithConfig(key = FEATURE_FLAGS_CONFIGURATION_SOURCE, value = "offline")
149-
void startWithOfflineConfigurationSourceSkipsConfigService() {
150-
try {
151-
assertDoesNotThrow(() -> FeatureFlaggingSystem.start(sharedCommunicationObjects()));
152-
} finally {
153-
FeatureFlaggingSystem.stop();
154-
}
140+
assertThrows(
141+
IllegalArgumentException.class,
142+
() ->
143+
FeatureFlaggingSystem.createConfigurationSourceService(
144+
sharedCommunicationObjects(), config));
155145
}
156146

157147
@Test

‎products/feature-flagging/feature-flagging-api/README.md‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,4 @@ OTEL_EXPORTER_OTLP_PROTOCOL=grpc
9797
and expects UFC under the JSON:API `data.attributes` response member. It is
9898
intended for supported commercial sites; use an explicit base URL elsewhere.
9999
Agentless responses do not have an SDK-imposed payload-size limit.
100-
`remote_config` uses the existing Agent Remote
101-
Configuration path. `offline` is reserved for startup-provided UFC bytes;
102-
until those bytes are implemented, no network source starts and evaluations
103-
use defaults.
100+
`remote_config` uses the existing Agent Remote Configuration path.

‎products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,14 @@
33
public class FeatureFlaggingConfig {
44

55
public static final String CONFIGURATION_SOURCE_AGENTLESS = "agentless";
6-
public static final String CONFIGURATION_SOURCE_OFFLINE = "offline";
76
public static final String CONFIGURATION_SOURCE_REMOTE_CONFIG = "remote_config";
87

8+
private static final Resolution DISABLED_RESOLUTION = new Resolution(false, null);
9+
private static final Resolution AGENTLESS_CONFIGURATION =
10+
new Resolution(true, CONFIGURATION_SOURCE_AGENTLESS);
11+
private static final Resolution REMOTE_CONFIG_CONFIGURATION =
12+
new Resolution(true, CONFIGURATION_SOURCE_REMOTE_CONFIG);
13+
914
public static final String FEATURE_FLAGS_ENABLED = "feature.flags.enabled";
1015
public static final String EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED =
1116
"experimental.flagging.provider.enabled";
@@ -27,29 +32,27 @@ public class FeatureFlaggingConfig {
2732
public static final String FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS =
2833
"feature.flags.configuration.source.agentless.request.timeout.seconds";
2934

30-
public static String resolveConfigurationSource(
35+
public static Resolution resolveConfiguration(
3136
final Boolean providerEnabled,
3237
final String explicitSource,
3338
final Boolean legacyProviderEnabled) {
3439
if (Boolean.FALSE.equals(providerEnabled)) {
35-
return CONFIGURATION_SOURCE_OFFLINE;
40+
return DISABLED_RESOLUTION;
3641
}
3742
if (explicitSource != null && !explicitSource.trim().isEmpty()) {
3843
final String source = explicitSource.trim();
3944
if (CONFIGURATION_SOURCE_AGENTLESS.equalsIgnoreCase(source)) {
40-
return CONFIGURATION_SOURCE_AGENTLESS;
45+
return AGENTLESS_CONFIGURATION;
4146
}
4247
if (CONFIGURATION_SOURCE_REMOTE_CONFIG.equalsIgnoreCase(source)) {
43-
return CONFIGURATION_SOURCE_REMOTE_CONFIG;
48+
return REMOTE_CONFIG_CONFIGURATION;
4449
}
45-
return CONFIGURATION_SOURCE_OFFLINE;
50+
return DISABLED_RESOLUTION;
4651
}
4752
if (legacyProviderEnabled != null) {
48-
return legacyProviderEnabled
49-
? CONFIGURATION_SOURCE_REMOTE_CONFIG
50-
: CONFIGURATION_SOURCE_OFFLINE;
53+
return legacyProviderEnabled ? REMOTE_CONFIG_CONFIGURATION : DISABLED_RESOLUTION;
5154
}
52-
return CONFIGURATION_SOURCE_AGENTLESS;
55+
return AGENTLESS_CONFIGURATION;
5356
}
5457

5558
public static boolean isSupportedConfigurationSource(final String source) {
@@ -58,8 +61,25 @@ public static boolean isSupportedConfigurationSource(final String source) {
5861
}
5962
final String normalized = source.trim();
6063
return CONFIGURATION_SOURCE_AGENTLESS.equalsIgnoreCase(normalized)
61-
|| CONFIGURATION_SOURCE_REMOTE_CONFIG.equalsIgnoreCase(normalized)
62-
|| CONFIGURATION_SOURCE_OFFLINE.equalsIgnoreCase(normalized);
64+
|| CONFIGURATION_SOURCE_REMOTE_CONFIG.equalsIgnoreCase(normalized);
65+
}
66+
67+
public static final class Resolution {
68+
private final boolean enabled;
69+
private final String source;
70+
71+
private Resolution(final boolean enabled, final String source) {
72+
this.enabled = enabled;
73+
this.source = source;
74+
}
75+
76+
public boolean isEnabled() {
77+
return enabled;
78+
}
79+
80+
public String getSource() {
81+
return source;
82+
}
6383
}
6484

6585
private FeatureFlaggingConfig() {}

‎products/feature-flagging/feature-flagging-config/src/test/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfigTest.java‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package datadog.trace.api.featureflag.config;
22

33
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_AGENTLESS;
4-
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_OFFLINE;
54
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_REMOTE_CONFIG;
65
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.isSupportedConfigurationSource;
7-
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.resolveConfigurationSource;
6+
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.resolveConfiguration;
87
import static org.junit.jupiter.api.Assertions.assertEquals;
98
import static org.junit.jupiter.api.Assertions.assertFalse;
9+
import static org.junit.jupiter.api.Assertions.assertNull;
1010
import static org.junit.jupiter.api.Assertions.assertTrue;
1111

1212
import org.junit.jupiter.api.Test;
@@ -15,19 +15,14 @@ class FeatureFlaggingConfigTest {
1515

1616
@Test
1717
void appliesConfigurationPrecedence() {
18-
assertEquals(CONFIGURATION_SOURCE_AGENTLESS, resolveConfigurationSource(null, null, null));
19-
assertEquals(CONFIGURATION_SOURCE_AGENTLESS, resolveConfigurationSource(null, " ", null));
20-
assertEquals(CONFIGURATION_SOURCE_REMOTE_CONFIG, resolveConfigurationSource(null, null, true));
21-
assertEquals(CONFIGURATION_SOURCE_OFFLINE, resolveConfigurationSource(null, null, false));
22-
assertEquals(
23-
CONFIGURATION_SOURCE_AGENTLESS, resolveConfigurationSource(null, "agentless", true));
24-
assertEquals(
25-
CONFIGURATION_SOURCE_REMOTE_CONFIG,
26-
resolveConfigurationSource(null, " remote_CONFIG ", false));
27-
assertEquals(
28-
CONFIGURATION_SOURCE_OFFLINE, resolveConfigurationSource(false, "agentless", true));
29-
assertEquals(CONFIGURATION_SOURCE_OFFLINE, resolveConfigurationSource(null, "invalid", null));
30-
assertEquals(CONFIGURATION_SOURCE_OFFLINE, resolveConfigurationSource(null, "offline", null));
18+
assertResolution(true, CONFIGURATION_SOURCE_AGENTLESS, null, null, null);
19+
assertResolution(true, CONFIGURATION_SOURCE_AGENTLESS, null, " ", null);
20+
assertResolution(true, CONFIGURATION_SOURCE_REMOTE_CONFIG, null, null, true);
21+
assertResolution(false, null, null, null, false);
22+
assertResolution(true, CONFIGURATION_SOURCE_AGENTLESS, null, "agentless", true);
23+
assertResolution(true, CONFIGURATION_SOURCE_REMOTE_CONFIG, null, " remote_CONFIG ", false);
24+
assertResolution(false, null, false, "agentless", true);
25+
assertResolution(false, null, null, "invalid", null);
3126
}
3227

3328
@Test
@@ -36,7 +31,23 @@ void recognizesSupportedExplicitSources() {
3631
assertTrue(isSupportedConfigurationSource(" "));
3732
assertTrue(isSupportedConfigurationSource("agentless"));
3833
assertTrue(isSupportedConfigurationSource(" REMOTE_CONFIG "));
39-
assertTrue(isSupportedConfigurationSource("OFFLINE"));
4034
assertFalse(isSupportedConfigurationSource("invalid"));
4135
}
36+
37+
private static void assertResolution(
38+
final boolean enabled,
39+
final String source,
40+
final Boolean providerEnabled,
41+
final String explicitSource,
42+
final Boolean legacyProviderEnabled) {
43+
final FeatureFlaggingConfig.Resolution resolution =
44+
resolveConfiguration(providerEnabled, explicitSource, legacyProviderEnabled);
45+
46+
assertEquals(enabled, resolution.isEnabled());
47+
if (source == null) {
48+
assertNull(resolution.getSource());
49+
} else {
50+
assertEquals(source, resolution.getSource());
51+
}
52+
}
4253
}

0 commit comments

Comments
 (0)