Skip to content

Commit 82e864f

Browse files
leoromanovskyleoromanovsky
committed
Address agentless configuration source feedback
1 parent 23c0b22 commit 82e864f

5 files changed

Lines changed: 124 additions & 81 deletions

File tree

‎dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@ public final class ConfigDefaults {
4949
static final boolean DEFAULT_INJECT_DATADOG_ATTRIBUTE = true;
5050
static final String DEFAULT_SITE = "datadoghq.com";
5151

52-
public static final String DEFAULT_FLAGGING_CONFIGURATION_SOURCE = "agentless";
53-
public static final double DEFAULT_FLAGGING_CONFIGURATION_SOURCE_POLL_INTERVAL_SECONDS = 30.0D;
54-
public static final double DEFAULT_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS = 2.0D;
52+
public static final String DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE = "agentless";
53+
public static final double DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_POLL_INTERVAL_SECONDS =
54+
30.0D;
55+
public static final double DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS =
56+
2.0D;
5557

5658
static final boolean DEFAULT_CODE_ORIGIN_FOR_SPANS_INTERFACE_SUPPORT = false;
5759
static final int DEFAULT_CODE_ORIGIN_MAX_USER_FRAMES = 8;

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

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@
8686
import static datadog.trace.api.ConfigDefaults.DEFAULT_ELASTICSEARCH_BODY_ENABLED;
8787
import static datadog.trace.api.ConfigDefaults.DEFAULT_ELASTICSEARCH_PARAMS_ENABLED;
8888
import static datadog.trace.api.ConfigDefaults.DEFAULT_EXPERIMENTATAL_JEE_SPLIT_BY_DEPLOYMENT;
89-
import static datadog.trace.api.ConfigDefaults.DEFAULT_FLAGGING_CONFIGURATION_SOURCE;
90-
import static datadog.trace.api.ConfigDefaults.DEFAULT_FLAGGING_CONFIGURATION_SOURCE_POLL_INTERVAL_SECONDS;
91-
import static datadog.trace.api.ConfigDefaults.DEFAULT_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS;
89+
import static datadog.trace.api.ConfigDefaults.DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE;
90+
import static datadog.trace.api.ConfigDefaults.DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_POLL_INTERVAL_SECONDS;
91+
import static datadog.trace.api.ConfigDefaults.DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS;
9292
import static datadog.trace.api.ConfigDefaults.DEFAULT_GRPC_CLIENT_ERROR_STATUSES;
9393
import static datadog.trace.api.ConfigDefaults.DEFAULT_GRPC_SERVER_ERROR_STATUSES;
9494
import static datadog.trace.api.ConfigDefaults.DEFAULT_HEALTH_METRICS_ENABLED;
@@ -1221,11 +1221,11 @@ public static String getHostName() {
12211221

12221222
private final int remoteConfigMaxExtraServices;
12231223

1224-
private final String flaggingConfigurationSource;
1225-
private final String flaggingConfigurationSourceBaseUrl;
1226-
private final double flaggingConfigurationSourcePollIntervalSeconds;
1227-
private final double flaggingConfigurationSourceRequestTimeoutSeconds;
1228-
private final Map<String, String> flaggingConfigurationSourceExtraHeaders;
1224+
private final String featureFlaggingConfigurationSource;
1225+
private final String featureFlaggingConfigurationSourceBaseUrl;
1226+
private final double featureFlaggingConfigurationSourcePollIntervalSeconds;
1227+
private final double featureFlaggingConfigurationSourceRequestTimeoutSeconds;
1228+
private final Map<String, String> featureFlaggingConfigurationSourceExtraHeaders;
12291229

12301230
private final boolean dbmInjectSqlBaseHash;
12311231
private final String dbmPropagationMode;
@@ -2849,23 +2849,24 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
28492849
configProvider.getInteger(
28502850
REMOTE_CONFIG_MAX_EXTRA_SERVICES, DEFAULT_REMOTE_CONFIG_MAX_EXTRA_SERVICES);
28512851

2852-
flaggingConfigurationSource =
2853-
normalizeFlaggingConfigurationSource(
2852+
featureFlaggingConfigurationSource =
2853+
normalizeFeatureFlaggingConfigurationSource(
28542854
configProvider.getString(
2855-
FEATURE_FLAGS_CONFIGURATION_SOURCE, DEFAULT_FLAGGING_CONFIGURATION_SOURCE));
2856-
flaggingConfigurationSourceBaseUrl =
2855+
FEATURE_FLAGS_CONFIGURATION_SOURCE, DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE));
2856+
featureFlaggingConfigurationSourceBaseUrl =
28572857
configProvider.getStringNotEmpty(
28582858
FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL, null);
2859-
flaggingConfigurationSourcePollIntervalSeconds =
2859+
featureFlaggingConfigurationSourcePollIntervalSeconds =
28602860
configProvider.getDouble(
28612861
FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS,
2862-
DEFAULT_FLAGGING_CONFIGURATION_SOURCE_POLL_INTERVAL_SECONDS);
2863-
flaggingConfigurationSourceRequestTimeoutSeconds =
2862+
DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_POLL_INTERVAL_SECONDS);
2863+
featureFlaggingConfigurationSourceRequestTimeoutSeconds =
28642864
configProvider.getDouble(
28652865
FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS,
2866-
DEFAULT_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS);
2867-
flaggingConfigurationSourceExtraHeaders =
2868-
configProvider.getMergedMap(FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_EXTRA_HEADERS, '=');
2866+
DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS);
2867+
featureFlaggingConfigurationSourceExtraHeaders =
2868+
configProvider.getMergedMap(
2869+
FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_EXTRA_HEADERS, '=');
28692870

28702871
dynamicInstrumentationEnabled =
28712872
configProvider.getBoolean(
@@ -3779,12 +3780,12 @@ public boolean isInferredProxyPropagationEnabled() {
37793780
return traceInferredProxyEnabled;
37803781
}
37813782

3782-
private static String normalizeFlaggingConfigurationSource(final String source) {
3783+
private static String normalizeFeatureFlaggingConfigurationSource(final String source) {
37833784
if (source == null) {
3784-
return DEFAULT_FLAGGING_CONFIGURATION_SOURCE;
3785+
return DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE;
37853786
}
37863787
final String normalized = source.trim().toLowerCase(Locale.ROOT);
3787-
return normalized.isEmpty() ? DEFAULT_FLAGGING_CONFIGURATION_SOURCE : normalized;
3788+
return normalized.isEmpty() ? DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE : normalized;
37883789
}
37893790

37903791
public boolean isBaggageExtract() {
@@ -4694,24 +4695,24 @@ public int getRemoteConfigMaxExtraServices() {
46944695
return remoteConfigMaxExtraServices;
46954696
}
46964697

4697-
public String getFlaggingConfigurationSource() {
4698-
return flaggingConfigurationSource;
4698+
public String getFeatureFlaggingConfigurationSource() {
4699+
return featureFlaggingConfigurationSource;
46994700
}
47004701

4701-
public String getFlaggingConfigurationSourceBaseUrl() {
4702-
return flaggingConfigurationSourceBaseUrl;
4702+
public String getFeatureFlaggingConfigurationSourceBaseUrl() {
4703+
return featureFlaggingConfigurationSourceBaseUrl;
47034704
}
47044705

4705-
public double getFlaggingConfigurationSourcePollIntervalSeconds() {
4706-
return flaggingConfigurationSourcePollIntervalSeconds;
4706+
public double getFeatureFlaggingConfigurationSourcePollIntervalSeconds() {
4707+
return featureFlaggingConfigurationSourcePollIntervalSeconds;
47074708
}
47084709

4709-
public double getFlaggingConfigurationSourceRequestTimeoutSeconds() {
4710-
return flaggingConfigurationSourceRequestTimeoutSeconds;
4710+
public double getFeatureFlaggingConfigurationSourceRequestTimeoutSeconds() {
4711+
return featureFlaggingConfigurationSourceRequestTimeoutSeconds;
47114712
}
47124713

4713-
public Map<String, String> getFlaggingConfigurationSourceExtraHeaders() {
4714-
return flaggingConfigurationSourceExtraHeaders;
4714+
public Map<String, String> getFeatureFlaggingConfigurationSourceExtraHeaders() {
4715+
return featureFlaggingConfigurationSourceExtraHeaders;
47154716
}
47164717

47174718
public boolean isDynamicInstrumentationEnabled() {
@@ -6546,16 +6547,16 @@ public String toString() {
65466547
+ remoteConfigMaxPayloadSize
65476548
+ ", remoteConfigIntegrityCheckEnabled="
65486549
+ remoteConfigIntegrityCheckEnabled
6549-
+ ", flaggingConfigurationSource="
6550-
+ flaggingConfigurationSource
6551-
+ ", flaggingConfigurationSourceBaseUrl="
6552-
+ flaggingConfigurationSourceBaseUrl
6553-
+ ", flaggingConfigurationSourcePollIntervalSeconds="
6554-
+ flaggingConfigurationSourcePollIntervalSeconds
6555-
+ ", flaggingConfigurationSourceRequestTimeoutSeconds="
6556-
+ flaggingConfigurationSourceRequestTimeoutSeconds
6557-
+ ", flaggingConfigurationSourceExtraHeaderNames="
6558-
+ flaggingConfigurationSourceExtraHeaders.keySet()
6550+
+ ", featureFlaggingConfigurationSource="
6551+
+ featureFlaggingConfigurationSource
6552+
+ ", featureFlaggingConfigurationSourceBaseUrl="
6553+
+ featureFlaggingConfigurationSourceBaseUrl
6554+
+ ", featureFlaggingConfigurationSourcePollIntervalSeconds="
6555+
+ featureFlaggingConfigurationSourcePollIntervalSeconds
6556+
+ ", featureFlaggingConfigurationSourceRequestTimeoutSeconds="
6557+
+ featureFlaggingConfigurationSourceRequestTimeoutSeconds
6558+
+ ", featureFlaggingConfigurationSourceExtraHeaderNames="
6559+
+ featureFlaggingConfigurationSourceExtraHeaders.keySet()
65596560
+ ", debuggerEnabled="
65606561
+ dynamicInstrumentationEnabled
65616562
+ ", debuggerUploadTimeout="

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

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
public class FeatureFlaggingSystem {
99

1010
private static final Logger LOGGER = LoggerFactory.getLogger(FeatureFlaggingSystem.class);
11-
private static final String SOURCE_AGENTLESS = "agentless";
12-
private static final String SOURCE_REMOTE_CONFIG = "remote_config";
13-
private static final String SOURCE_OFFLINE = "offline";
1411

1512
private static volatile ConfigurationSourceService CONFIG_SERVICE;
1613
private static volatile ExposureWriter EXPOSURE_WRITER;
@@ -34,22 +31,25 @@ public static void start(final SharedCommunicationObjects sco) {
3431

3532
static ConfigurationSourceService createConfigurationSourceService(
3633
final SharedCommunicationObjects sco, final Config config) {
37-
final String configurationSource = config.getFlaggingConfigurationSource();
34+
final ConfigurationSource configurationSource =
35+
ConfigurationSource.from(config.getFeatureFlaggingConfigurationSource());
3836

39-
if (SOURCE_REMOTE_CONFIG.equals(configurationSource)) {
40-
if (!config.isRemoteConfigEnabled()) {
41-
throw new IllegalStateException("Feature Flagging system started without RC");
42-
}
43-
return new RemoteConfigServiceImpl(sco, config);
44-
} else if (SOURCE_AGENTLESS.equals(configurationSource)) {
45-
return new UfcHttpConfigService(config);
46-
} else if (SOURCE_OFFLINE.equals(configurationSource)) {
47-
LOGGER.debug(
48-
"Feature Flagging offline configuration source selected; no config service started");
49-
return null;
50-
} else {
51-
throw new IllegalArgumentException(
52-
"Unsupported Feature Flagging configuration source: " + configurationSource);
37+
switch (configurationSource) {
38+
case REMOTE_CONFIG:
39+
if (!config.isRemoteConfigEnabled()) {
40+
throw new IllegalStateException("Feature Flagging system started without RC");
41+
}
42+
return new RemoteConfigServiceImpl(sco, config);
43+
case AGENTLESS:
44+
return new UfcHttpConfigService(config);
45+
case OFFLINE:
46+
LOGGER.debug(
47+
"Feature Flagging offline configuration source selected; no config service started");
48+
return null;
49+
default:
50+
throw new IllegalArgumentException(
51+
"Unsupported Feature Flagging configuration source: "
52+
+ config.getFeatureFlaggingConfigurationSource());
5353
}
5454
}
5555

@@ -64,4 +64,26 @@ public static void stop() {
6464
}
6565
LOGGER.debug("Feature Flagging system stopped");
6666
}
67+
68+
private enum ConfigurationSource {
69+
AGENTLESS("agentless"),
70+
REMOTE_CONFIG("remote_config"),
71+
OFFLINE("offline");
72+
73+
private final String value;
74+
75+
ConfigurationSource(final String value) {
76+
this.value = value;
77+
}
78+
79+
private static ConfigurationSource from(final String value) {
80+
for (final ConfigurationSource source : values()) {
81+
if (source.value.equals(value)) {
82+
return source;
83+
}
84+
}
85+
throw new IllegalArgumentException(
86+
"Unsupported Feature Flagging configuration source: " + value);
87+
}
88+
}
6789
}

‎products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UfcHttpConfigService.java‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
final class UfcHttpConfigService implements ConfigurationSourceService {
3131
private static final Logger LOGGER = LoggerFactory.getLogger(UfcHttpConfigService.class);
3232

33-
private static final String SERVER_DISTRIBUTION_PATH =
33+
private static final String DATADOG_API_SERVER_DISTRIBUTION_PATH =
3434
"/api/v2/feature-flagging/config/server-distribution";
3535
private static final int MAX_ATTEMPTS = 3;
3636

@@ -53,10 +53,11 @@ private UfcHttpConfigService(final Config config, final HttpUrl endpoint) {
5353
this(
5454
endpoint,
5555
config,
56-
millis(config.getFlaggingConfigurationSourcePollIntervalSeconds()),
56+
millis(config.getFeatureFlaggingConfigurationSourcePollIntervalSeconds()),
5757
new OkHttpUfcHttpClient(
5858
OkHttpUtils.buildHttpClient(
59-
endpoint, millis(config.getFlaggingConfigurationSourceRequestTimeoutSeconds()))),
59+
endpoint,
60+
millis(config.getFeatureFlaggingConfigurationSourceRequestTimeoutSeconds()))),
6061
Executors.newSingleThreadScheduledExecutor(new UfcHttpThreadFactory()));
6162
}
6263

@@ -69,7 +70,7 @@ endpoint, millis(config.getFlaggingConfigurationSourceRequestTimeoutSeconds())))
6970
this.endpoint = endpoint;
7071
this.config = config;
7172
final Map<String, String> configuredExtraHeaders =
72-
config.getFlaggingConfigurationSourceExtraHeaders();
73+
config.getFeatureFlaggingConfigurationSourceExtraHeaders();
7374
this.extraHeaders =
7475
configuredExtraHeaders == null ? Collections.emptyMap() : configuredExtraHeaders;
7576
this.pollIntervalMillis = pollIntervalMillis;
@@ -175,10 +176,10 @@ private void updateEtag(final String nextEtag) {
175176
}
176177

177178
static HttpUrl endpoint(final Config config) {
178-
final String configuredBaseUrl = config.getFlaggingConfigurationSourceBaseUrl();
179+
final String configuredBaseUrl = config.getFeatureFlaggingConfigurationSourceBaseUrl();
179180
final String endpoint =
180181
configuredBaseUrl == null
181-
? defaultEndpoint(config)
182+
? datadogApiServerDistributionEndpoint(config)
182183
: endpointFromConfiguredUrl(configuredBaseUrl);
183184
final HttpUrl parsed = HttpUrl.parse(endpoint);
184185
if (parsed == null) {
@@ -195,7 +196,11 @@ private static String endpointFromConfiguredUrl(final String configuredUrl) {
195196
"Invalid Feature Flagging HTTP configuration source URL: " + configuredUrl);
196197
}
197198
if (isRootPath(parsed)) {
198-
return parsed.newBuilder().addPathSegments("mock/ufc/config").build().toString();
199+
return parsed
200+
.newBuilder()
201+
.addPathSegments(datadogApiServerDistributionPath())
202+
.build()
203+
.toString();
199204
}
200205
return parsed.toString();
201206
}
@@ -204,18 +209,22 @@ private static boolean isRootPath(final HttpUrl url) {
204209
return "/".equals(url.encodedPath()) || url.encodedPath().isEmpty();
205210
}
206211

207-
private static String defaultEndpoint(final Config config) {
212+
private static String datadogApiServerDistributionEndpoint(final Config config) {
208213
final StringBuilder endpoint =
209214
new StringBuilder("https://api.")
210215
.append(config.getSite().toLowerCase(Locale.ROOT))
211-
.append(SERVER_DISTRIBUTION_PATH);
216+
.append(DATADOG_API_SERVER_DISTRIBUTION_PATH);
212217
final String env = config.getEnv();
213218
if (env != null && !env.isEmpty()) {
214219
endpoint.append("?dd_env=").append(urlEncode(env));
215220
}
216221
return endpoint.toString();
217222
}
218223

224+
private static String datadogApiServerDistributionPath() {
225+
return DATADOG_API_SERVER_DISTRIBUTION_PATH.substring(1);
226+
}
227+
219228
private static String urlEncode(final String value) {
220229
try {
221230
return URLEncoder.encode(value, "UTF-8");

‎products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/UfcHttpConfigServiceTest.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,26 +48,29 @@ void cleanup() {
4848
}
4949

5050
@Test
51-
void appendsMockConfigPathWhenConfiguredUrlIsRoot() {
51+
void appendsDatadogApiServerDistributionPathWhenConfiguredUrlIsRoot() {
5252
final Config config = config("http://mock-backend:8092", "datadoghq.com", "");
5353

5454
assertEquals(
55-
"http://mock-backend:8092/mock/ufc/config",
55+
"http://mock-backend:8092/api/v2/feature-flagging/config/server-distribution",
5656
UfcHttpConfigService.endpoint(config).toString());
5757
}
5858

5959
@Test
6060
void preservesConfiguredFullEndpointUrl() {
6161
final Config config =
62-
config("http://mock-backend:8092/mock/ufc/config?fixture=valid", "datadoghq.com", "");
62+
config(
63+
"http://mock-backend:8092/api/v2/feature-flagging/config/server-distribution?fixture=valid",
64+
"datadoghq.com",
65+
"");
6366

6467
assertEquals(
65-
"http://mock-backend:8092/mock/ufc/config?fixture=valid",
68+
"http://mock-backend:8092/api/v2/feature-flagging/config/server-distribution?fixture=valid",
6669
UfcHttpConfigService.endpoint(config).toString());
6770
}
6871

6972
@Test
70-
void derivesDefaultEndpointFromSiteAndEnv() {
73+
void derivesDatadogApiServerDistributionEndpointFromSiteAndEnv() {
7174
final Config config = config(null, "datad0g.com", "staging env");
7275

7376
assertEquals(
@@ -188,7 +191,7 @@ private static UfcHttpConfigService service(
188191
final FakeClient client, final Map<String, String> extraHeaders) {
189192
final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
190193
return new UfcHttpConfigService(
191-
HttpUrl.get("http://localhost/mock/ufc/config"),
194+
HttpUrl.get("http://localhost/api/v2/feature-flagging/config/server-distribution"),
192195
config("http://localhost", "datadoghq.com", "", extraHeaders),
193196
60_000,
194197
client,
@@ -205,10 +208,16 @@ private static Config config(
205208
final String env,
206209
final Map<String, String> extraHeaders) {
207210
final Config config = mock(Config.class);
208-
lenient().when(config.getFlaggingConfigurationSourceBaseUrl()).thenReturn(baseUrl);
209-
lenient().when(config.getFlaggingConfigurationSourceExtraHeaders()).thenReturn(extraHeaders);
210-
lenient().when(config.getFlaggingConfigurationSourcePollIntervalSeconds()).thenReturn(30.0D);
211-
lenient().when(config.getFlaggingConfigurationSourceRequestTimeoutSeconds()).thenReturn(2.0D);
211+
lenient().when(config.getFeatureFlaggingConfigurationSourceBaseUrl()).thenReturn(baseUrl);
212+
lenient()
213+
.when(config.getFeatureFlaggingConfigurationSourceExtraHeaders())
214+
.thenReturn(extraHeaders);
215+
lenient()
216+
.when(config.getFeatureFlaggingConfigurationSourcePollIntervalSeconds())
217+
.thenReturn(30.0D);
218+
lenient()
219+
.when(config.getFeatureFlaggingConfigurationSourceRequestTimeoutSeconds())
220+
.thenReturn(2.0D);
212221
lenient().when(config.getApiKey()).thenReturn("test-api-key");
213222
lenient().when(config.getSite()).thenReturn(site);
214223
lenient().when(config.getEnv()).thenReturn(env);

0 commit comments

Comments
 (0)