Support agentless OpenFeature CDN polling without the Java agent - #12109
Support agentless OpenFeature CDN polling without the Java agent#12109leoromanovsky wants to merge 16 commits into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 7faa22e | Docs | Datadog PR Page | Give us feedback! |
🔴 Java Benchmark SLOs — Performance SLO breach detected
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8def9ceb42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
The managed CDN endpoint builder leaves the configured environment value unescaped in the query string. An environment such as prod&blue is sent as dd_env=prod plus a separate blue parameter, so standalone applications can poll the wrong configuration; the pre-change agentless client encoded this value correctly.
📊 Validated against 4 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 7faa22e · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| "ufc-server.ff-cdn." + (site == null || site.isEmpty() ? "datadoghq.com" : site), | ||
| -1, | ||
| UFC_PATH, | ||
| env == null || env.isEmpty() ? null : "dd_env=" + env, |
There was a problem hiding this comment.
Standalone users with reserved characters in dd.env can receive no configuration or the wrong environment's feature flags from the CDN.
Assertion details
- Input: Managed CDN mode with
dd.env=prod&blue(or another environment containing&,=,#, or similar query-reserved characters). - Expected: The complete environment string is transmitted as one encoded
dd_envquery parameter, matching the prior OkHttp implementation'saddQueryParameterbehavior. - Actual:
CdnEndpointResolver.resolveconstructs the raw query asdd_env=prod&blue; the resulting URI is...?dd_env=prod&blue, which servers parse asdd_env=prodand an independentblueparameter.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7faa22ee17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| try (BufferedSource source = Okio.buffer(Okio.source(new ByteArrayInputStream(content)))) { | ||
| final JsonReader reader = JsonReader.of(source); | ||
| final boolean jsonApi = requireJsonApiEnvelope || isJsonApi(reader.peekJson()); |
There was a problem hiding this comment.
data field as an envelope
When a raw Remote Configuration UFC document contains a forward-compatible top-level field named data, this check classifies the entire document as JSON:API based only on that field name. parseJsonApi then returns null unless the field has the exact JSON:API resource shape, causing an otherwise valid raw configuration to be rejected instead of ignoring the additive field as intended. Detect the envelope from its resource structure/type, or avoid auto-detection on the raw Remote Configuration parsing path.
Useful? React with 👍 / 👎.
| final URI configured = URI.create(configuredBaseUrl.trim()); | ||
| if (!configured.isAbsolute() || configured.getHost() == null) { |
There was a problem hiding this comment.
When the configured base URL is absolute and has a host but uses an unsupported scheme such as ftp://, this validation accepts it. The asynchronous first poll then throws IllegalArgumentException from HttpRequest.newBuilder; pollOnceSafely swallows that exception, so initialization waits for the full configured timeout and subsequent polls keep repeating the same failure. Validate that the scheme is HTTP or HTTPS here so invalid configuration fails immediately and diagnostically.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Should we also require https for the custom base URL here? Right now the validation only checks that the URI is absolute and has a host, so a plain http:// value would be accepted and the poll would go out over plaintext. Wondering if that's intentional (e.g., to leave room for a local test harness) or something we'd want to gate.
| UFC_PATH, | ||
| env == null || env.isEmpty() ? null : "dd_env=" + env, |
There was a problem hiding this comment.
When DD_ENV contains a reserved query character such as & or =, passing "dd_env=" + env as the URI query does not encode that character as data; for example, prod&preview=true becomes ?dd_env=prod&preview=true. The managed CDN therefore receives a truncated environment plus an unintended parameter and can return configuration for the wrong environment. Encode the environment as a query-parameter value, matching the existing OkHttp path's addQueryParameter behavior.
Useful? React with 👍 / 👎.
|
I'm aware of the "performance" and "tech debt" review agents and was wondering if it ran with the codex review I kick off while the PR was in draft. My codex reports that: I ran them locally now: I'll consider addressing these but posting it here to show evidence of it having run. |
leoromanovsky
added
comp: openfeature
| try { | ||
| pollOnce(); | ||
| } catch (final RuntimeException ignored) { | ||
| if (!closed) { | ||
| status = SourceStatus.ERROR; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there downstream logic to emit PROVIDER_STALE if the polling fails?
https://openfeature.dev/docs/reference/concepts/events/#provider_stale
And then to emit PROVIDER_READY if polling recovers? And similarly reported in the evaluation reasons? https://openfeature.dev/specification/types/#resolution-reason
| from(bootstrapProject.extensions.getByType<SourceSetContainer>().named("main").map { it.output }) { | ||
| include("datadog/trace/api/featureflag/ufc/v1/**") | ||
| } |
There was a problem hiding this comment.
A concern from Codex
This embeds the UFC model in
dd-openfeature.jar, while the same classes are supplied by the agent on the bootstrap classloader. Standard parent-first delegation makes them identical at runtime, but a child-first application classloader could load this embedded copy and make gateway-providedServerConfigurationobjects type-incompatible with the provider’s evaluator. Do we support such classloaders? If so, can we add a child-first classloading test; otherwise, please document the parent-first delegation requirement.
| private static final String UFC = | ||
| "{\"format\":\"SERVER\",\"environment\":{\"name\":\"test\"},\"flags\":{" | ||
| + "\"message\":{\"key\":\"message\",\"enabled\":true,\"variationType\":\"STRING\"," | ||
| + "\"variations\":{\"on\":{\"key\":\"on\",\"value\":\"hello\"}}," | ||
| + "\"allocations\":[{\"key\":\"allocation\",\"rules\":[],\"splits\":[" | ||
| + "{\"variationKey\":\"on\",\"shards\":[]}],\"doLog\":true}]}}}"; | ||
| } |
There was a problem hiding this comment.
The JSON:API envelope is the CDN’s expected wire format, right?
Could this test use data.type plus data.attributes so the packaged-JAR path verifies envelope detection, parsing, storage, and evaluation end to end? The current raw UFC payload doesn’t exercise envelope handling.
vjfridge
left a comment
vjfridge
left a comment
There was a problem hiding this comment.
I'm not a huge domain expert yet, but from my review, this PR seems well-tested and solid compared to existing agent-backed implementation.
I left a few comments from things my Claude helped flag
| final URI configured = URI.create(configuredBaseUrl.trim()); | ||
| if (!configured.isAbsolute() || configured.getHost() == null) { |
There was a problem hiding this comment.
Should we also require https for the custom base URL here? Right now the validation only checks that the URI is absolute and has a host, so a plain http:// value would be accepted and the poll would go out over plaintext. Wondering if that's intentional (e.g., to leave room for a local test harness) or something we'd want to gate.
| private static Boolean booleanSetting(final String property, final String environment) { | ||
| final String value = setting(property, environment); | ||
| return value == null ? null : Boolean.valueOf(value); | ||
| } |
There was a problem hiding this comment.
Small divergence between the agent-side and standalone-side parsing of the enable flags that I think could bite customers on the migration path.
The agent path uses featureFlaggingBooleanSetting:
return Boolean.parseBoolean(value) || "1".equals(value);The standalone path here uses Boolean.valueOf(value).
So for DD_FEATURE_FLAGS_ENABLED (and DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED):
| Env value | Agent path | Standalone path |
|---|---|---|
true / TRUE / True |
✅ enabled | ✅ enabled |
false |
❌ disabled | ❌ disabled |
1 |
✅ enabled | ❌ disabled |
yes |
❌ disabled | ❌ disabled |
| unset | matrix default | matrix default |
A customer who wrote DD_FEATURE_FLAGS_ENABLED=1 in their existing agent-based deployment and then removes -javaagent to try the new agentless mode will silently land in the DISABLED branch and the flag evaluation returns defaults with PROVIDER_NOT_READY.
Two possible directions:
- Match the agent semantics by using the same
Boolean.parseBoolean(value) || "1".equals(value)form here. - Extract a single shared parser into
feature-flagging-config(next toresolveConfiguration) so both paths use it and future changes stay in lock-step.
Related: while looking at this, I noticed two other places where the standalone path re-implements agent-side logic rather than sharing it — worth checking whether the same drift risk
applies:
resolveSource(...)here reimplements the same precedence matrix thatFeatureFlaggingConfig.resolveConfiguration(...)already encodes for the agent path.setting(property, env)on this class reads system property → env var only. The agent'sfeatureFlaggingSettingalso reads Fleet and Local stable-config sources. A customer who shipsdd.feature.flags.configuration.source=agentlessvia Stable Configuration will pick it up under the agent but not in agentless mode. Also the standalone path can't be configured viaapplication_monitoring.yamlfiles, even though the agent-side path can.
3 participants
Motivation
dd-openfeature.jaris the application-facing OpenFeature provider. Before this change, it could not operate withoutdd-java-agent.jar.The Java agent owned most Feature Flags runtime behavior:
The provider received configuration through
FeatureFlaggingGateway, which is available only when the Java agent starts first. Without-javaagent, the provider could not receive configuration or become ready.Before this PR, both configuration sources depended on Java agent startup:
flowchart LR Start["JVM startup<br/>-javaagent required"] --> AgentRuntime RC["Datadog Agent<br/>Remote Configuration"] --> AgentRuntime CDN["Managed UFC CDN"] --> AgentRuntime subgraph Agent["Java agent runtime"] AgentRuntime["dd-java-agent.jar<br/>source selection, polling,<br/>UFC parsing, and lifecycle"] end AgentRuntime --> Gateway["FeatureFlaggingGateway<br/>bootstrap classloader boundary"] subgraph App["Application classloader"] Application["Application"] --> Provider["dd-openfeature.jar<br/>OpenFeature provider"] Provider --> Application end Gateway --> ProviderGCP Cloud Run permits Java agent attachment through
JAVA_TOOL_OPTIONS. Therefore, the existing release can run there. However, the customer must download two JARs, manage an agent path, and change JVM startup options.Other serverless runtimes can impose stricter startup controls. A runtime can own the JVM command, support only a platform-specific layer, or prevent arbitrary
-javaagentattachment. These limits make the provider unusable even when the application can add a normal library dependency.The default CDN mode must therefore work from
dd-openfeature.jaralone. A customer must be able to start the application withjava -jar app.jar.This release scope adds no-agent CDN evaluation and last-known-good behavior. It does not add no-agent exposure delivery or telemetry. Existing agent-backed Remote Configuration and its side effects remain unchanged.
Changes
The PR supports these data flows.
No Java agent: default or explicit
agentlessThe provider now owns the complete CDN evaluation path on the application classloader:
flowchart LR CDN["Managed UFC CDN"] --> Http["JDK HTTP source"] subgraph AppRuntime["Application classloader; no Java agent"] Http --> Parser["Shared UFC parser"] Parser --> Store["Configuration store"] Store --> Evaluator["Shared flag evaluator"] Application["Application"] --> API["OpenFeature API"] API --> Entry["Standalone evaluator entrypoint"] Entry --> Evaluator Evaluator --> Application endJava agent: explicit
agentlessJava agent: explicit or legacy
remote_configSource selection now has these results:
agentlessremote_configtruefalseagentlessremote_configDecisions
This PR makes no net-new platform change. It adds no service, endpoint, protocol, payload schema, or backend dependency.
The implementation reorganizes existing Java FFE code into reusable modules. The standalone entrypoint uses the existing managed CDN endpoint and UFC v1 payload.
UFC parsing remains forward compatible for additive fields. The parser ignores and discards unknown keys at the JSON API and UFC model levels. New enum values are semantic changes. For an unknown operator or variation type, the parser skips only the incompatible flag and keeps valid flags.
The shared evaluator test loads every case file from the pinned
ffe-system-test-datasubmodule. The current pin contains 26 files and 224 cases. Java-only cases cover SDK metadata and malformed input that the universal fixture contract excludes.The implementation has one source model, one UFC parser, and one flag evaluator. It keeps two entrypoints and two HTTP transports:
The existing
FeatureFlaggingGatewayremains the only agent classloader boundary. The PR adds no RPC and no new cross-classloader protocol. Without the agent, the provider runs the shared code on the application classloader.Commit
264a0ca4e1removes the duplicate model and parsers. That cleanup adds 340 lines and deletes 1,121 lines.The complete PR changes 45 files. Production code is
+1847/-659, tests are+1480/-376, and build metadata is+300/-0. The remaining additions implement the standalone HTTP lifecycle and its coverage. They do not add a second evaluator.Validation
System tests
3bcfb76cf3245a7f3e5909e4c0e3757aa42d7254264a0ca4e1ab4a8cf74b37fded7caa7cc4d0ecf1dd-openfeature.jarSHA-256:ee29d96bfeda54919923e303fb9600ec35f809fe3816658dd1dd0a2f4046d319dd-java-agent.jarSHA-256:14592da1f95bce63415f17de9d09b9bc5b25cd7f04669409ccf616a606969cd955a1e57ce493ac1a42af4cb867447e023eda05feAll 33 configuration-source cases passed against these exact artifacts. The parallel run passed 30 cases. Three cases had Docker test-agent setup errors from a local shared-port collision. Each affected case passed in a sequential rerun. No test assertion failed.
The four no-agent cases produced these results:
PROVIDER_READYand evaluatednew-user-onboardingforaliceasgreen.agentlessoverrode the legacy enable setting and produced the samegreenresult.greenresult.remote_configfailed provider startup and made zero CDN requests.The 29 agent-present source-selection and lifecycle cases also passed.
Dogfooding
31e721efa811c817c144de117d835f43cfeb16fbcodex-java-node-parityPROVIDER_READYafter 597 msPROVIDER_READYafter 601 msPROVIDER_READYafter 56 msThe Java agentless process ran
java -jar app.jar. Its health response reportedjavaAgentAttached: false.The Java RC process ran:
Its health response reported
javaAgentAttached: true.The managed configuration produced these live results:
true,TARGETING_MATCH,default-allocation"true",STATIC,allocation-override-9406352060f7"true",STATIC,allocation-override-9406352060f7"variant_2",TARGETING_MATCH,default-allocation"variant_2",STATIC,allocation-override-392dd7c149f8"variant_2",STATIC,allocation-override-392dd7c149f8-5,TARGETING_MATCH,default-allocation"-5",STATIC,allocation-default-c30ce0f47c94"-5",STATIC,allocation-default-c30ce0f47c94-3.14,TARGETING_MATCH,default-allocation"-3.14",STATIC,allocation-override-b84f868b6ff5"-3.14",STATIC,allocation-override-b84f868b6ff5{"ai_model":"openai","enabled":true},TARGETING_MATCH,default-allocation"{\"ai_model\":\"openai\",\"enabled\":true}",STATIC,allocation-default-bad0dcbff776"{\"ai_model\":\"openai\",\"enabled\":true}",STATIC,allocation-default-bad0dcbff776All five evaluator values matched Node.js. The Java dogfood HTTP response serializes
variant.keyas a string. Reason and allocation metadata remain SDK-specific.