Repository navigation
Conversation
danyal002
added
type: feature
This comment has been minimized.
This comment has been minimized.
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
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. |
danyal002
force-pushed
the
danyal.khan/EX-3815-exposure-hook
branch
from
October 8, 2026 19:03
296c989 to
272a12d
Compare
danyal002
marked this pull request as ready for review
October 9, 2026 18:14
danyal002
requested review from
greghuels,
hhan2024 and
sarahchen6
and removed request for
a team
October 9, 2026 18:14
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 272a12d296
ℹ️ 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".
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Source and test inspection support the documented separation between Datadog logging and callback exposure decisions. Runtime execution and production behavior were not validated.
🤖 Bits Code Review · Commit 272a12d
aarsilv
reviewed
Oct 11, 2026
| | `holdout.key` | The holdout key | | ||
| | `holdout.assignment_group` | `status_quo` or `all_shipped` | | ||
|
|
||
| The map is empty when the split has no such features. Datadog can add destinations later; the |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] Add a table to the README that maps each capability to the dd-java-agent version it needs, and state the oldest agent that this provider supports. With an older agent this map is empty, and nothing tells the reader why.
dd-openfeature and dd-java-agent ship separately, and the provider turns a capability off when the installed agent lacks it. Today the only signal is a log line at startup: a warning for the serial-id checks, a debug message for the feature checks. This stack adds three of these checks. A table puts the rules in one place, and it shows which checks can go once the oldest supported agent reaches their version.
Suggested section — fill in the three placeholders at release time
### Datadog Java agent versions
`dd-openfeature` uses what the installed `dd-java-agent` supports and turns the rest off.
| Capability | `dd-java-agent` version |
|---|---|
| Exposures to Datadog | <oldest supported version> or later |
| Serial id in span enrichment metadata | 1.65.0 or later |
| Flag-evaluation events | 1.66.0 or later |
| Serial id on exposures | 1.67.0 or later |
| `ExposureHook.Evaluation.getFeatures()` and `__dd_feature.*` metadata | <first release with split features> or later |
| Features on exposures and flag-evaluation events | <first release with these events' features> or later |If you take the OpenFeature SDK bump that I suggested on #12774, add one more line under the table: "Sending exposures through OpenFeature's MultiProvider also needs dev.openfeature:sdk 1.23.0 or later."
Where the versions come from — the first release tag that contains each class, field or constructor
| Agent-side change | Commit | First release tag |
|---|---|---|
ExposureEvent (traced across the module move in #10412) |
8aa326fafe |
v1.56.0 |
Split.serialId (#11658) |
05671ce3b0 |
v1.65.0 |
FlagEvalEvent |
95065ee8b2 |
v1.66.0 |
ExposureEvent constructor with a serial id (#12432) |
598bc41cdb |
v1.67.0 |
These tags show when each piece first shipped. I did not check that the current provider runs end to end on each of those agents, so the oldest supported version is for you to fill in.
Contributor
Author
There was a problem hiding this comment.
Added an "Agent versions" table in 9135fa2, checked against the release tags: serial ids on splits need 1.65.0, flag evaluation events 1.66.0, and serial ids on exposures 1.67.0. I left out the oldest supported agent, because I could not confirm it. I will add it once it is settled.
This was replied to by Claude.
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Replace "The first release that includes this change" with a version number before release, and add rows for #12797's two capabilities. As written, that placeholder would ship in the README.
#12797 adds features on exposures and on flag evaluation events, each behind its own agent check (EXPOSURE_FEATURES_SUPPORTED, FlagEvalLoggingHook.FEATURES_SUPPORTED), but it does not change the README. These capabilities can arrive in a later agent release than Split.features, so give each row its own version, even if the numbers end up the same. Leaving out the oldest supported agent until it is confirmed is fine.
Suggested rows — fill in each version at release
| Split features (`getFeatures()`, `__dd_feature.<key>`) | 1.NN.0 | `getFeatures()` is empty |
| Features on exposures sent to Datadog | 1.NN.0 | Exposures are sent without features |
| Features on flag evaluation events | 1.NN.0 | Flag evaluation events are sent without features |
A UFC split can carry a list of key-value features for exposure hooks. Split gains a nullable features field, and the parser reads it leniently: a malformed list or entry is dropped instead of rejecting the flag, so features never change which variation a subject gets.
…coverage Test the factory's type and annotation checks and the read-only adapter, as the other UFC adapters do, and list the Feature model class with the other UFC POJOs that are excluded from coverage verification.
Add a public ExposureHook that hands each completed evaluation to a callback with the provider's exposure decision. shouldSend() is true for exactly the exposures the provider sends to Datadog. getFeatures() returns the selected split's features, which the provider now copies into the flag metadata as __dd_feature.<key>. DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED=false stops sending exposures to Datadog. The exposure cache keeps running, so ExposureHook callbacks get the same decisions either way.
Each feature now lists where SDKs deliver it, such as HOOK, EXPOSURE or EVALUATION. Destinations are open strings: the parser keeps every non-empty string, including ones this SDK does not know, and drops a feature that has no destinations.
Copy a split feature into the __dd_feature.<key> metadata only when its destinations include HOOK. EXPOSURE and EVALUATION are known destinations that are not delivered to hooks. A destination the SDK does not know is ignored, with one warning for each new name.
SpotBugs reports NP_BOOLEAN_RETURN_NULL because getCacheHit() returned a Boolean that is null when the evaluation is not an exposure. A caller that unboxes it gets a NullPointerException. isCacheHit() returns false in that case; isExposure() still tells callers whether it is one. shouldSend() still needs an explicit false stamp, so sending is unchanged.
The warn-once set kept every unknown destination name for the life of the application, so a stream of configurations with new names grew it without bound. It now stops at 64 names; later unknown destinations are still ignored, only without a warning.
danyal002
force-pushed
the
danyal.khan/EX-3815-exposure-hook
branch
from
October 13, 2026 14:31
a7d2222 to
4a665f2
Compare
shouldSend() matches the exposures Datadog receives only when the provider's own hooks run. The Javadoc now says so, names the MultiProvider case before OpenFeature SDK 1.23.0, and says that ExposureHook is optional because the flag metadata carries the same data. ExposureHook now skips the callback when OpenFeature passes no details, and logs the first callback failure at warn level instead of debug. ExposureLoggingHook logs its first LinkageError at warn level instead of swallowing it. Both read flag metadata through its map, because the ImmutableMetadata getters build a debug message for every absent key.
The earlier tests passed false straight to the gate or the hook, so a wrong key would still pass. This forked test sets the real system property and checks that the provider sends nothing to Datadog while ExposureHook keeps its decisions.
The README now says first that the provider puts its exposure decision and the split's features in the flag metadata, and that ExposureHook is a convenience on top of it. It says how to read the metadata, narrows the shouldSend() claim to when the provider's hooks run, corrects the __dd_do_log and __dd_split_serial_id rows, and adds a table of the dd-java-agent version each capability needs.
aarsilv
approved these changes
Oct 13, 2026
aarsilv
left a comment
aarsilv
left a comment
There was a problem hiding this comment.
Thanks for iterating! Have a few more minor test & README changes for you to consider but this is totally shippable as-is.
| | `holdout.key` | The holdout key | | ||
| | `holdout.assignment_group` | `status_quo` or `all_shipped` | | ||
|
|
||
| The map is empty when the split has no such features. Datadog can add destinations later; the |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Replace "The first release that includes this change" with a version number before release, and add rows for #12797's two capabilities. As written, that placeholder would ship in the README.
#12797 adds features on exposures and on flag evaluation events, each behind its own agent check (EXPOSURE_FEATURES_SUPPORTED, FlagEvalLoggingHook.FEATURES_SUPPORTED), but it does not change the README. These capabilities can arrive in a later agent release than Split.features, so give each row its own version, even if the numbers end up the same. Leaving out the oldest supported agent until it is confirmed is fine.
Suggested rows — fill in each version at release
| Split features (`getFeatures()`, `__dd_feature.<key>`) | 1.NN.0 | `getFeatures()` is empty |
| Features on exposures sent to Datadog | 1.NN.0 | Exposures are sent without features |
| Features on flag evaluation events | 1.NN.0 | Flag evaluation events are sent without features |No released dd-java-agent has Split.features yet. The provider must skip reading it there, or every evaluation throws NoSuchFieldError.
It matches only while Datadog exposure logging is on, and a changed serial id counts only with dd-java-agent 1.67.0 or later. The README also gives 1.68.0 for split features, says that the logging setting does not stop flag evaluation events, and that the provider does not read dd-trace-py's DD_FFE_INTAKE_ENABLED.
* Send EXPOSURE and EVALUATION features to Datadog A UFC split feature can list the EXPOSURE and EVALUATION destinations. The evaluator now writes each feature once for each destination it lists, under that destination's metadata prefix. The exposure hook puts EXPOSURE features on the exposure event, and the flag-evaluation hook puts EVALUATION features on the flag-evaluation event, whatever the consent, because they come from the flag configuration. Flag-evaluation buckets now include the features in both tiers, so rows with different features never merge, and each row serializes them. Both events gain a features field with new constructors; with an older agent the provider sends them without features. * Keep the first feature when a split repeats a feature key The FFE_FLAGS schema documents that keys are unique within a split and that SDKs keep the first feature with a repeated key. Flag metadata is a map, so the provider used to keep the last one. It now ignores a later feature with a key it has already seen, for every destination. * Check flag-evaluation feature support inside FlagEvalLoggingHook DDEvaluator resolved FlagEvalEvent.class in its static initializer. Agents released before FlagEvalEvent existed then failed DDEvaluator's initialization with NoClassDefFoundError, which broke the provider. The check now lives in FlagEvalLoggingHook, the only class that uses it, and Provider already guards that hook's loading against LinkageError. * Read split features without DDEvaluator, and scan only when needed - Moves featuresWithPrefix into SplitFeatureMetadata, which depends only on the OpenFeature API. ExposureHook.getFeatures() called it on DDEvaluator, which needs the Datadog Java agent, so it threw NoClassDefFoundError where the agent is absent, such as unit tests. - DDEvaluator marks the metadata with __dd_evaluation_features only when the split routes a feature to EVALUATION. FlagEvalLoggingHook scans the metadata only then, instead of on every evaluation. - Adds an end-to-end test of EVALUATION features for an allocation that does not log exposures. Every earlier provider-backed test used doLog=true, so adding features only for logged allocations passed. - Reflows a broken comment. * Share one copy of each feature set across a flush window Each bucket kept the features map that the provider built for its own evaluation, and each key kept its own canonical string. Full-tier keys include the targeting key, so up to PER_FLAG_CAP subjects of one split held that many equal maps and strings until the flush. The aggregator now keeps one key and map for each distinct feature set, and clears them with the rest of the window. * Test that ExposureHook works without the Datadog Java agent The test classpath always has the agent classes, so no other test sees ExposureHook reach for them. This test loads ExposureHook in a class loader that hides DDEvaluator and the agent classes, as in an application without the agent. * Read flag evaluation metadata from its map, and rename the marker - FlagEvalLoggingHook reads allocationKey, the evaluation timestamp, the consent key and the evaluation-features marker from one map. The ImmutableMetadata getters build a debug message for every absent key, and error and default results carry only the consent key. - Renames the marker to __dd_has_evaluation_features, so that it does not differ from the __dd_evaluation_feature. prefix by one character. - Rewraps a comment that google-java-format had split.
danyal002
merged commit Oct 14, 2026
761bda4
into
danyal.khan/EX-3800-shared-exposure-cache
2 of 3 checks passed
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
…cord them with ExposureHook (#12774) Send exposures only after a successful evaluation The provider now deduplicates exposures itself and sends them from a finally-stage hook instead of at resolution. Resolution only checks the new provider cache and stamps __dd_exposure_cache_hit in the flag metadata. The hook records the exposure and sends it when the allocation logs exposures, the subject is not already exposed, and the evaluation has no error, so an evaluation that an application hook fails no longer sends an exposure or marks the subject as exposed. The agent's writer keeps its own cache for now, because an older provider still sends every evaluation. Cover the exposure hook guards and exclude the cache POJOs from coverage Test each condition under which the exposure hook sends nothing, and list the exposure cache's key and value classes as POJOs excluded from coverage verification, as the writer's cache classes are. Bound the exposure cache with a static map class SpotBugs reports IA_AMBIGUOUS_INVOCATION_OF_INHERITED_OR_OUTER_METHOD because the anonymous LinkedHashMap's size() call could resolve to the cache's own size(). A static nested map class has no outer instance, so the call is unambiguous. Explain why the exposure cache value classes skip coverage Read the split serial id only when the agent's Split has it The exposure cache check read split.serialId before exposureSerialId could apply SPLIT_SERIAL_ID_SUPPORTED. With an older dd-java-agent whose Split has no serialId field, every evaluation of an allocation that logs exposures then threw NoSuchFieldError. The read now sits behind the same guard as the other serial id reads. Require OpenFeature SDK 1.23.0 so exposures work inside a MultiProvider Exposures are now sent from a provider hook. OpenFeature SDK 1.20.1 to 1.22.1 does not run the hooks of providers wrapped in a MultiProvider, so a Datadog provider inside one sent no exposures. SDK 1.23.0 runs them. The README states 1.23.0 as the minimum for MultiProvider, and a test covers the MultiProvider path; it fails on 1.20.1. Read exposure metadata from its map in ExposureLoggingHook The ImmutableMetadata getters build a debug message for every absent key, whether or not debug logging is on. __dd_do_log is absent from most evaluations by default, so the hook built that string on most of them. It now reads the keys from asUnmodifiableMap(). Rename USE_LEGACY_EXPOSURE_API to EXPOSURE_SERIAL_ID_SUPPORTED The flag now names the capability it detects, like SPLIT_SERIAL_ID_SUPPORTED next to it, instead of the case to avoid. Its value is inverted, and every reader is inverted with it. Correct how the provider and writer exposure caches relate They have the same size but refresh entries at different times, so once full they can evict different subjects. The comment said they evict the same ones. Test deduplication with an agent that predates serial-id exposures On dd-java-agent 1.65 and 1.66 the lookup at resolution uses no serial id, so the hook must record none either. Otherwise every repeat of a split with a serial id is sent again. Rewrap the exposure cache size comment google-java-format had split it, leaving a line that held only one word. Let applications record feature flag exposures with ExposureHook (#12775) * Parse features on UFC splits A UFC split can carry a list of key-value features for exposure hooks. Split gains a nullable features field, and the parser reads it leniently: a malformed list or entry is dropped instead of rejecting the flag, so features never change which variation a subject gets. * Cover the features adapter factory and exclude the Feature POJO from coverage Test the factory's type and annotation checks and the read-only adapter, as the other UFC adapters do, and list the Feature model class with the other UFC POJOs that are excluded from coverage verification. * Let applications record feature flag exposures with ExposureHook Add a public ExposureHook that hands each completed evaluation to a callback with the provider's exposure decision. shouldSend() is true for exactly the exposures the provider sends to Datadog. getFeatures() returns the selected split's features, which the provider now copies into the flag metadata as __dd_feature.<key>. DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED=false stops sending exposures to Datadog. The exposure cache keeps running, so ExposureHook callbacks get the same decisions either way. * Parse the destinations of UFC split features Each feature now lists where SDKs deliver it, such as HOOK, EXPOSURE or EVALUATION. Destinations are open strings: the parser keeps every non-empty string, including ones this SDK does not know, and drops a feature that has no destinations. * Deliver only HOOK features to exposure hooks Copy a split feature into the __dd_feature.<key> metadata only when its destinations include HOOK. EXPOSURE and EVALUATION are known destinations that are not delivered to hooks. A destination the SDK does not know is ignored, with one warning for each new name. * Replace getCacheHit() with a primitive isCacheHit() SpotBugs reports NP_BOOLEAN_RETURN_NULL because getCacheHit() returned a Boolean that is null when the evaluation is not an exposure. A caller that unboxes it gets a NullPointerException. isCacheHit() returns false in that case; isExposure() still tells callers whether it is one. shouldSend() still needs an explicit false stamp, so sending is unchanged. * Cap the unknown feature destinations the evaluator remembers The warn-once set kept every unknown destination name for the life of the application, so a stream of configurations with new names grew it without bound. It now stops at 64 names; later unknown destinations are still ignored, only without a warning. * State when shouldSend() matches Datadog, and surface hook failures shouldSend() matches the exposures Datadog receives only when the provider's own hooks run. The Javadoc now says so, names the MultiProvider case before OpenFeature SDK 1.23.0, and says that ExposureHook is optional because the flag metadata carries the same data. ExposureHook now skips the callback when OpenFeature passes no details, and logs the first callback failure at warn level instead of debug. ExposureLoggingHook logs its first LinkageError at warn level instead of swallowing it. Both read flag metadata through its map, because the ImmutableMetadata getters build a debug message for every absent key. * Test that the Datadog exposure logging setting turns sending off The earlier tests passed false straight to the gate or the hook, so a wrong key would still pass. This forked test sets the real system property and checks that the provider sends nothing to Datadog while ExposureHook keeps its decisions. * Lead the exposure README with the flag metadata any hook can read The README now says first that the provider puts its exposure decision and the split's features in the flag metadata, and that ExposureHook is a convenience on top of it. It says how to read the metadata, narrows the shouldSend() claim to when the provider's hooks run, corrects the __dd_do_log and __dd_split_serial_id rows, and adds a table of the dd-java-agent version each capability needs. * Test evaluation with an agent whose Split has no features No released dd-java-agent has Split.features yet. The provider must skip reading it there, or every evaluation throws NoSuchFieldError. * Correct when shouldSend() matches what Datadog receives It matches only while Datadog exposure logging is on, and a changed serial id counts only with dd-java-agent 1.67.0 or later. The README also gives 1.68.0 for split features, says that the logging setting does not stop flag evaluation events, and that the provider does not read dd-trace-py's DD_FFE_INTAKE_ENABLED. * Send EXPOSURE and EVALUATION feature flag features to Datadog (#12797) * Send EXPOSURE and EVALUATION features to Datadog A UFC split feature can list the EXPOSURE and EVALUATION destinations. The evaluator now writes each feature once for each destination it lists, under that destination's metadata prefix. The exposure hook puts EXPOSURE features on the exposure event, and the flag-evaluation hook puts EVALUATION features on the flag-evaluation event, whatever the consent, because they come from the flag configuration. Flag-evaluation buckets now include the features in both tiers, so rows with different features never merge, and each row serializes them. Both events gain a features field with new constructors; with an older agent the provider sends them without features. * Keep the first feature when a split repeats a feature key The FFE_FLAGS schema documents that keys are unique within a split and that SDKs keep the first feature with a repeated key. Flag metadata is a map, so the provider used to keep the last one. It now ignores a later feature with a key it has already seen, for every destination. * Check flag-evaluation feature support inside FlagEvalLoggingHook DDEvaluator resolved FlagEvalEvent.class in its static initializer. Agents released before FlagEvalEvent existed then failed DDEvaluator's initialization with NoClassDefFoundError, which broke the provider. The check now lives in FlagEvalLoggingHook, the only class that uses it, and Provider already guards that hook's loading against LinkageError. * Read split features without DDEvaluator, and scan only when needed - Moves featuresWithPrefix into SplitFeatureMetadata, which depends only on the OpenFeature API. ExposureHook.getFeatures() called it on DDEvaluator, which needs the Datadog Java agent, so it threw NoClassDefFoundError where the agent is absent, such as unit tests. - DDEvaluator marks the metadata with __dd_evaluation_features only when the split routes a feature to EVALUATION. FlagEvalLoggingHook scans the metadata only then, instead of on every evaluation. - Adds an end-to-end test of EVALUATION features for an allocation that does not log exposures. Every earlier provider-backed test used doLog=true, so adding features only for logged allocations passed. - Reflows a broken comment. * Share one copy of each feature set across a flush window Each bucket kept the features map that the provider built for its own evaluation, and each key kept its own canonical string. Full-tier keys include the targeting key, so up to PER_FLAG_CAP subjects of one split held that many equal maps and strings until the flush. The aggregator now keeps one key and map for each distinct feature set, and clears them with the rest of the window. * Test that ExposureHook works without the Datadog Java agent The test classpath always has the agent classes, so no other test sees ExposureHook reach for them. This test loads ExposureHook in a class loader that hides DDEvaluator and the agent classes, as in an application without the agent. * Read flag evaluation metadata from its map, and rename the marker - FlagEvalLoggingHook reads allocationKey, the evaluation timestamp, the consent key and the evaluation-features marker from one map. The ImmutableMetadata getters build a debug message for every absent key, and error and default results carry only the consent key. - Renames the marker to __dd_has_evaluation_features, so that it does not differ from the __dd_evaluation_feature. prefix by one character. - Rewraps a comment that google-java-format had split. Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #12774. Includes the commits of #12772 until it merges.
What Does This Do
ExposureHook. It hands each completed evaluation to a callback, with the provider's exposure decision:shouldSend()is true for exactly the exposures the provider sends to Datadog.isExposure()is true for every successful evaluation of an allocation that logs exposures, including repeats.isCacheHit()tells repeats apart.getFeatures()returns the selected split's features whose destinations includeHOOK, such asholdout.keyandholdout.assignment_group.__dd_feature.<key>.DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED(defaulttrue). Whenfalse, the provider sends no exposures to Datadog. The exposure cache keeps running, soExposureHookcallbacks get the same decisions.dd-openfeatureREADME.Motivation
Some customers use Datadog feature flags but keep exposures in their own data store. They need each exposure, with the holdout key and assignment group, in their own code. They also need to stop sending the same exposures to Datadog.
Additional Notes
dd-java-agent, the split has no features, sogetFeatures()is empty. Exposure decisions still work.metadata/supported-configurations.json.Jira ticket: EX-3815 [EX-3801]