Skip to content

Send Java SDK identity with Feature Flagging EVP events #12575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leoromanovsky wants to merge 5 commits into from
Open

Send Java SDK identity with Feature Flagging EVP events #12575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96a3427
Identify Java feature flag events on local and direct EVP routes
leoromanovsky Sep 19, 2026
1ac2d95
Merge reviewed EVP transport repairs into Java identity
leoromanovsky Oct 12, 2026
923a274
Merge final EVP hardening base into Java identity
leoromanovsky Oct 12, 2026
ceb8629
Merge lock-free exposure shutdown into Java identity
leoromanovsky Oct 12, 2026
8ad35c4
Merge bounded shutdown coverage fix into EVP identity
leoromanovsky Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 44 additions & 12 deletions communication/src/main/java/datadog/communication/BackendApiFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
package datadog.communication;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;

import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
import datadog.communication.ddagent.SharedCommunicationObjects;
import datadog.communication.http.HttpRetryPolicy;
import datadog.trace.api.Config;
import datadog.trace.api.intake.Intake;
import datadog.trace.util.throwable.FatalAgentMisconfigurationError;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import okhttp3.HttpUrl;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -22,24 +27,41 @@ public class BackendApiFactory {

private final Config config;
private final SharedCommunicationObjects sharedCommunicationObjects;
private final Map<String, String> requestHeaders;
private final boolean sendOnce;

public BackendApiFactory(Config config, SharedCommunicationObjects sharedCommunicationObjects) {
this(config, sharedCommunicationObjects, false);
this(config, sharedCommunicationObjects, emptyMap());
}

public BackendApiFactory(

@vjfridge vjfridge Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Non-blocking cleanup — dead API surface

Two of the four public constructors have no independent caller outside tests. Delete both 3-argument overloads now, while the API is new and nothing in this repository depends on its shape.

The one worth removing on its own merits is (Config, SCO, Map). It implicitly selects sendOnce = false. The default explicit HTTP policy then permits up to five retries — new HttpRetryPolicy.Factory(5, 100, 2.0, true) at line 298, whose first parameter is maxRetries, allowing up to six explicit attempts before accounting for retries internal to OkHttp — while the supplied OkHttp client's own retryOnConnectionFailure setting is left as it is. EVP callers can override the explicit policy, including with NEVER_RETRY. When retries are enabled, a payload with no idempotency key can produce duplicate events after an ambiguous failure, for example when the intake processes the request but the response times out. Nothing in the signature warns about it, and the Javadoc that explains sendOnce sits on the 4-argument form only.

Evidence — caller census (repo-wide grep)
Overload Independent callers outside tests
(Config, SCO) LogsIntakeSystem:22, CiVisibilityServices:84,85, 2 JMH benchmarks
(Config, SCO, Map) none — new in this PR; reached only through the 2-argument form and BackendApiFactoryTest:389
(Config, SCO, boolean) none — this PR orphaned it; serves only BackendApiFactoryTest:323, :512
(Config, SCO, Map, boolean) FeatureFlagBackendApiFactory:49

Two honest caveats. The JMH benchmarks are not production code, they just live outside src/test. And the map overload is not dead execution — the 2-argument constructor delegates to it today. The proposed change re-points that delegation at the 4-argument form.

Evidence — the deletion compiles and passes (executed)

I applied the change in a scratch worktree, updated the three test call sites, and added import static java.util.Collections.emptyMap; to the test file:

  • :communication:test — 235/235 ✅
  • :products:feature-flagging:feature-flagging-lib:test — 279/279 ✅

:dd-java-agent:agent-ci-visibility:compileJava could not build in my offline worktree. It fails identically on the clean tree (unresolvable scala-library / kotlin-stdlib), and it uses only the retained 2-argument form.

Scope note. This removes a footgun from a new API; it does not fix a live bug. The only production caller that needs sendOnce already passes true explicitly (FeatureFlagBackendApiFactory:49), and the retained 2-argument form still defaults to false for the existing CI Visibility and Logs Intake callers, which is correct for them.

Suggested change — delete lines 37-47 and collapse the 2-argument form:

  public BackendApiFactory(Config config, SharedCommunicationObjects sharedCommunicationObjects) {
    this(config, sharedCommunicationObjects, emptyMap(), false);
  }

Then make the three test call sites explicit, keeping each one's existing arguments:

// BackendApiFactoryTest:322-323 — was (Config, SCO, boolean)
new BackendApiFactory(
    Config.get(), sharedCommunicationObjects(discovery, agent.url("/")), emptyMap(), true);

// BackendApiFactoryTest:512 — was (Config, SCO, boolean)
new BackendApiFactory(
    Config.get(), sharedCommunicationObjects(discovery, null), emptyMap(), true);

// BackendApiFactoryTest:389-392 — was (Config, SCO, Map)
new BackendApiFactory(
    config,
    sharedCommunicationObjects(new FakeFeaturesDiscovery(null), null),
    singletonMap(ORIGIN_HEADER, JAVA_TRACING_LIBRARY),
    false);

Config config,
SharedCommunicationObjects sharedCommunicationObjects,
Map<String, String> requestHeaders) {
this(config, sharedCommunicationObjects, requestHeaders, false);
}

public BackendApiFactory(
Config config, SharedCommunicationObjects sharedCommunicationObjects, boolean sendOnce) {
this(config, sharedCommunicationObjects, emptyMap(), sendOnce);
}

/**
* Creates a backend factory with optional send-once transport semantics.
* Creates a backend factory with per-request headers and optional send-once transport semantics.
*
* <p>When {@code sendOnce} is true, automatic connection retries and redirects are disabled, and
* the default HTTP retry policy is {@link HttpRetryPolicy.Factory#NEVER_RETRY}. Explicit caller
* policies may retry proven pre-send failures, but must not replay an ambiguously accepted event
* payload without an idempotency key.
*/
public BackendApiFactory(
Config config, SharedCommunicationObjects sharedCommunicationObjects, boolean sendOnce) {
Config config,
SharedCommunicationObjects sharedCommunicationObjects,
Map<String, String> requestHeaders,
boolean sendOnce) {
this.config = config;
this.sharedCommunicationObjects = sharedCommunicationObjects;
this.requestHeaders = unmodifiableMap(new HashMap<>(requestHeaders));
this.sendOnce = sendOnce;
}

Expand Down Expand Up @@ -182,7 +204,7 @@ private static boolean isAsciiLetterOrDigit(final char character) {
* Creates an EVP proxy client after Agent discovery, optionally forcing a fresh discovery and
* requiring the Agent to advertise every specified forwarding header.
*
* <p>Callers supply the forwarding capabilities their event protocol requires.
* <p>Required forwarding capabilities are independent of any headers configured on this factory.

@vjfridge vjfridge Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ Nit — documentation preference, strictly optional

The replaced sentence said who decides what to pass. The replacement says what does not feed the gate. Both are useful; one line holds both.

  • Before: Callers supply the forwarding capabilities their event protocol requires.
  • After: Required forwarding capabilities are independent of any headers configured on this factory.

The new sentence answers the question this PR creates — do the configured request headers feed the gate? They do not, and that is worth stating. To be fair to the edit, the retained summary at line 204 already describes the mechanism ("requiring the Agent to advertise every specified forwarding header"), so nothing essential was lost. What went is the hint that the caller's event protocol is what determines the list.

Suggested change:

  /**
   * Creates an EVP proxy client after Agent discovery, optionally forcing a fresh discovery and
   * requiring the Agent to advertise every specified forwarding header.
   *
   * <p>Callers supply the forwarding capabilities their event protocol requires. These are
   * independent of any headers configured on this factory.
   *
   * <p>The {@code forceDiscovery} form is intended for bounded unavailable-route recovery probes.
   */

*
* <p>The {@code forceDiscovery} form is intended for bounded unavailable-route recovery probes.
*/
Expand Down Expand Up @@ -250,14 +272,24 @@ private BackendApi createEvpProxyApi(
}

OkHttpClient configureHttpClient(final OkHttpClient httpClient) {
return sendOnce
? httpClient
.newBuilder()
.retryOnConnectionFailure(false)
.followRedirects(false)
.followSslRedirects(false)
.build()
: httpClient;
if (requestHeaders.isEmpty() && !sendOnce) {
return httpClient;
}
final OkHttpClient.Builder builder = httpClient.newBuilder();
if (sendOnce) {
builder.retryOnConnectionFailure(false).followRedirects(false).followSslRedirects(false);
}
if (!requestHeaders.isEmpty()) {
builder.addInterceptor(
chain -> {
final Request.Builder requestBuilder = chain.request().newBuilder();
for (Map.Entry<String, String> header : requestHeaders.entrySet()) {
requestBuilder.header(header.getKey(), header.getValue());
}
return chain.proceed(requestBuilder.build());
});
}
return builder.build();
}

private HttpRetryPolicy.Factory retryPolicyFactory() {
Expand Down
3 changes: 3 additions & 0 deletions communication/src/main/java/datadog/communication/EvpProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ public final class EvpProxy {
/** Identifies the version of the SDK that produced an EVP request. */
public static final String ORIGIN_VERSION_HEADER = "DD-EVP-ORIGIN-VERSION";

/** Origin header value identifying this tracing library. */
public static final String JAVA_TRACING_LIBRARY = "dd-trace-java";

/**
* Default SDK-side target for uncompressed EVP request bodies. Writers may split batches at or
* below this size to keep Agent proxy requests comfortably bounded.
Expand Down
69 changes: 65 additions & 4 deletions communication/src/test/java/datadog/communication/BackendApiFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package datadog.communication;

import static datadog.communication.EvpProxy.JAVA_TRACING_LIBRARY;
import static datadog.communication.EvpProxy.ORIGIN_HEADER;
import static datadog.communication.EvpProxy.ORIGIN_VERSION_HEADER;
import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V2_EVP_PROXY_ENDPOINT;
import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V4_EVP_PROXY_ENDPOINT;
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_URL;
import static datadog.trace.api.config.GeneralConfig.API_KEY;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonMap;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand All @@ -22,7 +26,10 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Stream;
import okhttp3.HttpUrl;
import okhttp3.MediaType;
Expand Down Expand Up @@ -327,15 +334,18 @@ void discoveryRejectsMissingOrPartialIdentityCapabilities(final String capabilit
}

@Test
void featureFlagProxyRequestNeverCarriesDirectCredentials() throws Exception {
void featureFlagProxyRequestCarriesSdkIdentityWithoutDirectCredentials() throws Exception {
final MockWebServer agent = new MockWebServer();
agent.enqueue(new MockResponse().setResponseCode(200).setBody("{}"));
agent.start();
try {
final FakeFeaturesDiscovery discovery = new FakeFeaturesDiscovery(V4_EVP_PROXY_ENDPOINT);
final BackendApiFactory factory =
new BackendApiFactory(
Config.get(), sharedCommunicationObjects(discovery, agent.url("/")), true);
Config.get(),
sharedCommunicationObjects(discovery, agent.url("/")),
sdkHeaders(),
true);
final BackendApi api =
factory.createEvpProxyApi(
Intake.EVENT_PLATFORM,
Expand All @@ -355,6 +365,8 @@ void featureFlagProxyRequestNeverCarriesDirectCredentials() throws Exception {
final RecordedRequest request = agent.takeRequest();
assertEquals("/evp_proxy/v4/api/v2/exposures", request.getPath());
assertEquals("event-platform-intake", request.getHeader("X-Datadog-EVP-Subdomain"));
assertEquals(JAVA_TRACING_LIBRARY, request.getHeader(ORIGIN_HEADER));
assertEquals("test-version", request.getHeader(ORIGIN_VERSION_HEADER));
assertNull(request.getHeader("DD-API-KEY"));
assertEquals(1, agent.getRequestCount());
} finally {
Expand All @@ -363,15 +375,55 @@ void featureFlagProxyRequestNeverCarriesDirectCredentials() throws Exception {
}

@Test
void featureFlagDirectRequestCarriesCredentialsExactlyOnce() throws Exception {
void directIntakeSendsConfiguredRequestHeaders() throws Exception {
final MockWebServer intake = new MockWebServer();
intake.enqueue(new MockResponse().setResponseCode(200).setBody("{}"));
intake.start();
try {
final Properties properties = new Properties();
properties.setProperty(API_KEY, "api-key");
properties.setProperty(
CIVISIBILITY_AGENTLESS_URL, intake.url("/").toString().replaceAll("/$", ""));
final Config config = Config.get(properties);
final BackendApiFactory factory =
new BackendApiFactory(
config,
sharedCommunicationObjects(new FakeFeaturesDiscovery(null), null),
singletonMap(ORIGIN_HEADER, JAVA_TRACING_LIBRARY));

// followRedirects=false mirrors the feature-flagging caller, so this also covers the
// interaction between the redirect-scoped client and the header interceptor.
final BackendApi api = factory.createDirectIntakeApi(Intake.API, false, false);

assertNotNull(api);
api.post(
"flagevaluation",
RequestBody.create(JSON, "{}".getBytes(StandardCharsets.UTF_8)),
stream -> null,
null,
false);

final RecordedRequest request = intake.takeRequest();
assertEquals(JAVA_TRACING_LIBRARY, request.getHeader(ORIGIN_HEADER));
assertEquals("api-key", request.getHeader("DD-API-KEY"));
} finally {

@vjfridge vjfridge Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ Nit — test hygiene

This test leaves one idle socket open to a dead port. The sibling test 40 lines down evicts its pool; this one does not. One line fixes it.

Because requestHeaders is non-empty and followRedirects=false, createDirectIntakeApi derives a fresh OkHttpClient, which opens a connection that stays in the pool. The test shuts down only the MockWebServer.

Evidence — measured effect (executed) 
P5 derived client == parent               = false
P5 shares parent dispatcher               = true
P5 shares parent connection pool          = true
P5 idle connections left in pool          = 1
P5 idle connections after server shutdown = 1
P5 non-daemon OkHttp threads              = none
P5 dispatcher runningCalls                = 0

So: one idle socket to a dead port, held for the OkHttp keep-alive window. No non-daemon thread, so it cannot block JVM exit, and no leaked call. This is hygiene, not a leak.

Two things the obvious fix gets wrong

The dispatcher line is a no-op here. Copying the sibling's teardown verbatim would add dispatcher().executorService().shutdownNow(). getIntakeHttpClient() builds its client with new Dispatcher(RejectingExecutorService.INSTANCE) (OkHttpUtils.java:178), and that executor's shutdownNow() returns an empty list and does nothing (RejectingExecutorService.java:23). Only evictAll() does work on this client.

Worth knowing: the sibling's own dispatcher line buys little either. Its request goes through the synchronous newCall(request).execute() path (OkHttpUtils.java:428), and in the pinned OkHttp 3.12.15 Dispatcher.executorService() builds its ThreadPoolExecutor lazily on first call — I checked the bytecode. So the teardown allocates a pool that never ran a task, then shuts it down. Do not copy it here on the theory that it is doing cleanup there.

evictAll() does not stop the pool cleanup daemon thread. OkHttp documents that it can keep running after eviction. The socket closes; the thread may stay TIMED_WAITING until it times out. That is expected and harmless.

The derivation is not what allocates the pool. The derived client shares the parent's pool by reference, so the same idle socket would exist if the test used the parent directly. The reason to fix it is consistency with the sibling, not a cost introduced by this PR.

Suggested change — compiled and run; :communication:test 235/235 ✅.

Hoist the SharedCommunicationObjects out of the try so the finally can reach the parent client:

    final SharedCommunicationObjects sco =
        sharedCommunicationObjects(new FakeFeaturesDiscovery(null), null);
    try {
      // …
      final BackendApiFactory factory =
          new BackendApiFactory(config, sco, singletonMap(ORIGIN_HEADER, JAVA_TRACING_LIBRARY));
      // …
    } finally {
      // The derived client shares the parent connection pool by reference, so evicting the
      // parent pool closes the idle socket this test opened.
      sco.getIntakeHttpClient().connectionPool().evictAll();
      intake.shutdown();
    }

intake.shutdown();
}
}

@Test
void featureFlagDirectRequestCarriesCredentialsAndIdentityExactlyOnce() throws Exception {
final MockWebServer intake = new MockWebServer();
intake.enqueue(new MockResponse().setResponseCode(500).setBody("failed"));
intake.enqueue(new MockResponse().setResponseCode(200).setBody("{}"));
intake.start();
final OkHttpClient sharedClient = new OkHttpClient.Builder().build();
final BackendApiFactory factory =
new BackendApiFactory(
Config.get(), sharedCommunicationObjects(new FakeFeaturesDiscovery(null), null), true);
Config.get(),
sharedCommunicationObjects(new FakeFeaturesDiscovery(null), null),
sdkHeaders(),
true);
final OkHttpClient directClient = factory.configureHttpClient(sharedClient);
try {
final IntakeApi api =
Expand All @@ -396,6 +448,8 @@ void featureFlagDirectRequestCarriesCredentialsExactlyOnce() throws Exception {
final RecordedRequest request = intake.takeRequest();
assertEquals("/api/v2/flagevaluation", request.getPath());
assertEquals("api-key", request.getHeader("DD-API-KEY"));
assertEquals("dd-trace-java", request.getHeader("DD-EVP-ORIGIN"));
assertEquals("test-version", request.getHeader("DD-EVP-ORIGIN-VERSION"));
assertNull(request.getHeader("X-Datadog-EVP-Subdomain"));
assertEquals(1, intake.getRequestCount());
assertEquals(false, directClient.retryOnConnectionFailure());
Expand Down Expand Up @@ -523,6 +577,13 @@ private static SharedCommunicationObjects sharedCommunicationObjects(
return sco;
}

private static Map<String, String> sdkHeaders() {
final Map<String, String> headers = new HashMap<>(2);
headers.put("DD-EVP-ORIGIN", "dd-trace-java");
headers.put("DD-EVP-ORIGIN-VERSION", "test-version");
return headers;
}

private static final class TestSharedCommunicationObjects extends SharedCommunicationObjects {
private final DDAgentFeaturesDiscovery discovery;

Expand Down
24 changes: 20 additions & 4 deletions ...ture-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagBackendApiFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
package com.datadog.featureflag;

import static datadog.communication.EvpProxy.JAVA_TRACING_LIBRARY;
import static datadog.communication.EvpProxy.ORIGIN_HEADER;
import static datadog.communication.EvpProxy.ORIGIN_VERSION_HEADER;
import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V2_EVP_PROXY_ENDPOINT;
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_AGENTLESS;
import static java.util.Collections.emptyList;
import static java.util.Arrays.asList;
import static java.util.Collections.unmodifiableMap;

import datadog.communication.BackendApi;
import datadog.communication.BackendApiFactory;
import datadog.communication.ddagent.SharedCommunicationObjects;
import datadog.communication.ddagent.TracerVersion;
import datadog.communication.http.HttpRetryPolicy;
import datadog.trace.api.Config;
import datadog.trace.api.intake.Intake;
import java.net.ConnectException;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import okhttp3.Response;
import org.slf4j.Logger;
Expand All @@ -20,6 +27,7 @@
final class FeatureFlagBackendApiFactory {

private static final Logger LOGGER = LoggerFactory.getLogger(FeatureFlagBackendApiFactory.class);
private static final Map<String, String> REQUEST_HEADERS = requestHeaders();

/** Retries connection refusal only; automatic retries and redirects remain disabled. */
static final HttpRetryPolicy.Factory PRE_SEND_RETRY =
Expand Down Expand Up @@ -71,7 +79,7 @@ public boolean shouldRetry(@Nullable final Response response) {
final FeatureFlagRouteSelector routeSelector) {
this(
config,
new BackendApiFactory(config, sharedCommunicationObjects, true),
new BackendApiFactory(config, sharedCommunicationObjects, REQUEST_HEADERS, true),
Comment thread
leoromanovsky marked this conversation as resolved.

@aarsilv aarsilv Oct 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

[P2] Please add a test that sends an exposure through the real writer and checks both identity headers on the wire. Today no test protects the identity headers that this line passes.

If this line passes an empty map instead of REQUEST_HEADERS, the communication and feature-flagging-lib tests still pass (235/235 and 279/279). Production then sends no identity.

The open thread at line 112 suggests a smoke test for this wiring. A unit test can do it. ExposureWriterTests already builds ExposureWriterImpl with a real SharedCommunicationObjects. Its requests go through this line, a real BackendApiFactory, and the fixed EVP v2 route to the test server. No test reads their headers.

1. Measured: three mutants pass the current tests, and the test below fails on each
Tree communication feature-flagging-lib
this line passes an empty map 235/235 279/279
requestHeaders() builds an empty map 235/235 279/279
createEvpProxyApiForEndpoint builds its client from the shared agentHttpClient 235/235 279/279
positive control: configureHttpClient never adds the interceptor 232/235 279/279
PR head + test below 235/235 280/280
test below + each of the first three mutants 235/235 279/280, the new test fails

The third mutant matters more after you restack onto #12477's head 4218559309. There, every Feature Flagging proxy send uses createEvpProxyApiForEndpoint. The communication tests check the proxy-route headers only through the five-argument createEvpProxyApi.

2. Suggested test: for ExposureWriterTests; it passes spotless and also passes on a local merge onto 4218559309

Add import datadog.communication.ddagent.TracerVersion; and this test:

  @Test
  void testExposureRequestsCarrySdkIdentity() throws Exception {
    // The production constructor builds a real BackendApiFactory, so this covers the identity
    // headers from FeatureFlagBackendApiFactory to the wire on the fixed EVP v2 route.
    Config config = mockConfig("identity-service");

    try (ExposureWriterImpl writer =
        new ExposureWriterImpl(1 << 4, 100, MILLISECONDS, sharedCommunicationObjects, config)) {
      writer.init();
      writer.accept(buildExposure());

      poll.eventually(
          () -> {
            assertNotNull(server.getLastRequest());
            assertEquals(EXPOSURES_ENDPOINT, server.getLastRequest().getPath());
            assertEquals("dd-trace-java", server.getLastRequest().getHeader("DD-EVP-ORIGIN"));
            assertEquals(
                TracerVersion.TRACER_VERSION,
                server.getLastRequest().getHeader("DD-EVP-ORIGIN-VERSION"));
          });
    }
  }

In this test JVM, TracerVersion.TRACER_VERSION is "0.0.0", so the version check cannot tell the loader's fallback from a hardcoded "0.0.0". The dd-java-agent.version test resource suggested in the thread at line 112 closes that gap with a non-fallback value.

@leoromanovsky leoromanovsky Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1ac2d95866 adds both-header assertions through the production factory and real socket, including its fixed-endpoint transport. The requested real ExposureWriter assertion and non-fallback version fixture are still missing; leaving this open.

eventType,
routeSelector);
}
Expand Down Expand Up @@ -127,14 +135,22 @@ BackendApi create() {
@Nullable
private String discoverProxyEndpoint(final boolean forceDiscovery) {
try {
// Identity-forwarding requirements belong with SDK identity emission in the identity PR.
return backendApiFactory.discoverEvpProxyEndpoint(forceDiscovery, emptyList());
// A local route must preserve the identity headers that this sender emits on every batch.
return backendApiFactory.discoverEvpProxyEndpoint(
forceDiscovery, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER));
} catch (final RuntimeException exception) {
LOGGER.debug("Could not discover the local Feature Flagging route", exception);
return null;
}
}

private static Map<String, String> requestHeaders() {

@vjfridge vjfridge Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Non-blocking suggestion — test coverage

Delete the two headers.put calls in this method and both test suites stay green. This method builds the payload the PR is named after, and no test pins its contents.

The transport is covered; the contents are not. BackendApiFactoryTest supplies its own header map, so it proves the interceptor works. Every test in FeatureFlagBackendApiFactoryTest passes mock(BackendApiFactory.class), so the argument at line 49 is never read.

Evidence — mutation results (executed)
# Mutation :communication:test :products:…:feature-flagging-lib:test
M1 both headers.put calls behind a never-true guard 235/235 ✅ 279/279 ✅
M2 — positive control configureHttpClient skips addInterceptor 3 ❌ 279/279 ✅

M2 proves the harness can go red. It just does not watch this method.

Evidence — without a fixture, the version assertion misses a hardcoded "0.0.0"

TracerVersion.TRACER_VERSION reads dd-java-agent.version from the system classloader and falls back to "0.0.0" (TracerVersion.java:11). I printed the value inside the feature-flagging-lib test JVM:

TRACER_VERSION=[0.0.0]

So assertEquals(TracerVersion.TRACER_VERSION, …) compares "0.0.0" with "0.0.0". The assertion still detects a missing or otherwise wrong version header; what it cannot do is tell the version loader's fallback apart from a hardcoded "0.0.0". The origin and map-size assertions are unaffected.

Adding src/test/resources/dd-java-agent.version fixes it. Executed results:

Tree state ff-lib suite
Fix + fixture 280/280 ✅
Fix + fixture, version source replaced by literal "0.0.0" 1 ❌
Fix + fixture, M1 re-applied 1 ❌

One local caveat: build.gradle.kts:77-82 sets normalization { runtimeClasspath { ignore("**/*.version") } }. A change to the fixture alone does not invalidate the test task cache key. Use --no-build-cache --rerun-tasks while you iterate — that is how I ran all three rows above.

Scope — what this test does and does not cover

It covers the contents of the map. It does not cover the wiring: the test still passes if line 49 stops passing REQUEST_HEADERS to the constructor. Closing that needs a real BackendApiFactory, which this test class does not use.

A smoke test can close it. OpenFeatureProviderSmokeTest runs DD_FEATURE_FLAGS_CONFIGURATION_SOURCE=remote_config (line 46), which takes the fixed EVP v2 route and so bypasses the capability gate, and decodedEvpProxyMessageCallback() (line 56) already receives the RequestApi. RequestApi.getHeader(String) exists (TestHttpServer.groovy:450). Capturing request.getHeader('DD-EVP-ORIGIN') and asserting it in 'test open feature exposures' would cover the wiring end to end. I did not write that test.

With the current AbstractSmokeTest /info response, agentless mode rejects the discovered local proxy: AbstractSmokeTest.groovy:94-100 advertises only Content-Type, Accept-Encoding, Content-Encoding, User-Agent, DD-CI-PROVIDER-NAME, so the capability gate fails. Exercising that proxy route would need the mock advertisement updated. Direct intake is a separate path and is not blocked by this.

Why coverage metrics will not flag this

The static initializer at line 28 runs requestHeaders() whenever the class loads, so line coverage reports the method as covered although no assertion reads the values. Separately, datadog.communication.BackendApiFactory sits in excludedClassesCoverage (communication/build.gradle.kts:53).

Suggested change — compiled and run; kills M1.

FeatureFlagBackendApiFactory.java:112 — widen the method:

  static Map<String, String> requestHeaders() {

New file products/feature-flagging/feature-flagging-lib/src/test/resources/dd-java-agent.version:

TEST-SNAPSHOT~test

FeatureFlagBackendApiFactoryTest.java — add four imports (static org.junit.jupiter.api.Assertions.assertEquals, static org.junit.jupiter.api.Assertions.assertNotEquals, datadog.communication.ddagent.TracerVersion, java.util.Map) and the test:

  @Test
  void requestHeadersIdentifyTheJavaTracerAndItsVersion() {
    final Map<String, String> headers = FeatureFlagBackendApiFactory.requestHeaders();

    assertEquals("dd-trace-java", headers.get("DD-EVP-ORIGIN"));
    assertEquals(TracerVersion.TRACER_VERSION, headers.get("DD-EVP-ORIGIN-VERSION"));
    // Guards the premise: without src/test/resources/dd-java-agent.version the line above
    // compares the "0.0.0" fallback with itself.
    assertNotEquals("0.0.0", headers.get("DD-EVP-ORIGIN-VERSION"));
    assertEquals(2, headers.size());
  }

The header name and the origin value stay raw literals on purpose. They pin the wire format from outside the constants.

@leoromanovsky leoromanovsky Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially addressed in 1ac2d95866: the real-factory socket test now asserts both headers and exercises constructor wiring. The non-fallback version fixture is still missing, so I am leaving this coverage request open.

final Map<String, String> headers = new HashMap<>(2);
headers.put(ORIGIN_HEADER, JAVA_TRACING_LIBRARY);
headers.put(ORIGIN_VERSION_HEADER, TracerVersion.TRACER_VERSION);
return unmodifiableMap(headers);
}

private boolean hasDirectCredentials() {
final String apiKey = config.getApiKey();
return apiKey != null && !apiKey.isEmpty();
Expand Down
20 changes: 14 additions & 6 deletions ...gging/feature-flagging-lib/src/test/java/com/datadog/featureflag/ExposureWriterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.datadog.featureflag;

import static datadog.communication.EvpProxy.ORIGIN_HEADER;
import static datadog.communication.EvpProxy.ORIGIN_VERSION_HEADER;
import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V4_EVP_PROXY_ENDPOINT;
import static datadog.trace.api.featureflag.config.FeatureFlaggingConfig.CONFIGURATION_SOURCE_AGENTLESS;
import static java.util.Collections.emptyList;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
Expand Down Expand Up @@ -566,7 +568,8 @@ void testAmbiguousExposureBatchIsNotRetriedOrReplayedDirectly() throws Exception
final BackendApiFactory backendApiFactory = mock(BackendApiFactory.class);
final BackendApi proxyApi = mock(BackendApi.class);
final BackendApi directApi = mock(BackendApi.class);
when(backendApiFactory.discoverEvpProxyEndpoint(false, emptyList()))
when(backendApiFactory.discoverEvpProxyEndpoint(
false, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER)))
.thenReturn(V4_EVP_PROXY_ENDPOINT);
when(backendApiFactory.createEvpProxyApiForEndpoint(
Intake.EVENT_PLATFORM,
Expand Down Expand Up @@ -636,7 +639,8 @@ void idleUnavailableWriterOnlyProbesForANewBatchAndStillCloses() throws Exceptio
final AtomicLong clock = new AtomicLong();
final BackendApiFactory backend = mock(BackendApiFactory.class);
final BackendApi proxy = mock(BackendApi.class);
when(backend.discoverEvpProxyEndpoint(true, emptyList())).thenReturn(V4_EVP_PROXY_ENDPOINT);
when(backend.discoverEvpProxyEndpoint(true, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER)))
.thenReturn(V4_EVP_PROXY_ENDPOINT);
when(backend.createEvpProxyApiForEndpoint(
Intake.EVENT_PLATFORM,
true,
Expand All @@ -653,14 +657,18 @@ void idleUnavailableWriterOnlyProbesForANewBatchAndStillCloses() throws Exceptio
new ExposureWriterImpl(16, 100, MILLISECONDS, factory, config);
try {
writer.init();
poll.eventually(() -> verify(backend).discoverEvpProxyEndpoint(false, emptyList()));
poll.eventually(
() ->
verify(backend)
.discoverEvpProxyEndpoint(false, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER)));
clock.set(10);
MILLISECONDS.sleep(300);
verify(backend, never()).discoverEvpProxyEndpoint(true, emptyList());
verify(backend, never())
.discoverEvpProxyEndpoint(true, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER));
verify(proxy, never()).post(any(), any(), any(), any(), eq(false));
writer.accept(buildExposure());
poll.eventually(() -> verify(proxy).post(eq("exposures"), any(), any(), any(), eq(false)));
verify(backend).discoverEvpProxyEndpoint(true, emptyList());
verify(backend).discoverEvpProxyEndpoint(true, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER));
} finally {
writer.close();
}
Expand Down
Loading
Loading