Skip to content

fix: [openfeature] allow empty targeting key in exposure events#4361

Merged
dd-mergequeue[bot] merged 4 commits into
mainfrom
leo.romanovsky/openfeature-allow-empty-targeting-key
Jan 16, 2026
Merged

fix: [openfeature] allow empty targeting key in exposure events#4361
dd-mergequeue[bot] merged 4 commits into
mainfrom
leo.romanovsky/openfeature-allow-empty-targeting-key

Conversation

@leoromanovsky

@leoromanovsky leoromanovsky commented Jan 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

The exposure schema (exposure.json) requires subject.id to be a string but does not enforce a minimum length, meaning empty strings are valid.

Previously, the exposure hook would skip logging exposures when the targeting key was empty. This prevented server-side flag evaluations (which often don't have a user context) from being tracked.

SDK Specification | EXP.5

-->

What does this PR do?

This change removes that restriction, allowing exposures with empty targeting keys to be sent to the event platform.

system test relevant to this: DataDog/system-tests#6040

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@leoromanovsky
openfeature: allow empty targeting key in exposure events
6efb359 
The exposure schema (exposure.json) requires subject.id to be a string
but does not enforce a minimum length, meaning empty strings are valid.

Previously, the exposure hook would skip logging exposures when the
targeting key was empty. This prevented server-side flag evaluations
(which often don't have a user context) from being tracked.

This change removes that restriction, allowing exposures with empty
targeting keys to be sent to the event platform.
@leoromanovsky leoromanovsky changed the title openfeature: allow empty targeting key in exposure events fix: [openfeature] allow empty targeting key in exposure events Jan 16, 2026
@codecov

codecov Bot commented Jan 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.00%. Comparing base (bcf07d5) to head (62210ff).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
openfeature/exposure_hook.go 90.76% <ø> (+12.82%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter

pr-commenter Bot commented Jan 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-01-16 16:27:55

Comparing candidate commit 62210ff in PR branch leo.romanovsky/openfeature-allow-empty-targeting-key with baseline commit bcf07d5 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 158 metrics, 6 unstable metrics.

@leoromanovsky
openfeature: add unit tests for exposureHook.After
4b4e29e 
Add comprehensive tests for the exposure hook's After method:
- Test with non-empty targeting key (standard case)
- Test with empty targeting key (server-side evaluations)
- Test missing allocation key (should skip exposure)
- Test doLog=false (should skip exposure)
- Test nil metadata (should skip exposure)
- Test empty targeting key with attributes (verifies attributes are preserved)
- Test cancelled context (should return error and skip exposure)
dd-oleksii
dd-oleksii reviewed Jan 16, 2026
View reviewed changes
Comment thread openfeature/exposure_hook.go Outdated
@leoromanovsky
openfeature: simplify - always include targeting key in flatContext
fb7ea37 
Empty targeting key is valid, no need to conditionally exclude it.
@leoromanovsky leoromanovsky marked this pull request as ready for review January 16, 2026 14:10
@leoromanovsky leoromanovsky requested review from a team as code owners January 16, 2026 14:11
@leoromanovsky

leoromanovsky commented Jan 16, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jan 16, 2026 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-01-16 15:08:38 UTC ℹ️ Start processing command /merge

2026-01-16 15:09:08 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 39m (p90).

2026-01-16 15:34:38 UTC ❌ MergeQueue: The checks failed on this merge request

Tests failed on this commit 77695fc:

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.

@leoromanovsky

leoromanovsky commented Jan 16, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jan 16, 2026 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-01-16 16:30:36 UTC ℹ️ Start processing command /merge

2026-01-16 16:31:21 UTC ℹ️ MergeQueue: waiting for PR to be ready

This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals. View in MergeQueue UI.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2026-01-16 16:32:54 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in main is approximately 39m (p90).

2026-01-16 16:59:12 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue Bot merged commit e5d6064 into main Jan 16, 2026
186 checks passed
@dd-mergequeue dd-mergequeue Bot deleted the leo.romanovsky/openfeature-allow-empty-targeting-key branch January 16, 2026 16:59
@leoromanovsky leoromanovsky mentioned this pull request Jan 16, 2026
Merged
5 tasks
eliottness pushed a commit that referenced this pull request Jan 26, 2026
@leoromanovsky @eliottness
fix: [openfeature] allow empty targeting key in exposure events (#4361)
48b3b2d 
Co-authored-by: leo.romanovsky <leo.romanovsky@datadoghq.com>
@eliottness eliottness mentioned this pull request Jan 26, 2026
Merged
7 tasks
leoromanovsky added a commit to DataDog/system-tests that referenced this pull request Jan 26, 2026
@leoromanovsky
Enable OF.7 empty targeting key test for Java and Go
932416d 
The OpenFeature Java SDK 1.20.1 fixed the empty targeting key issue
(open-feature/java-sdk#1807), and dd-trace-go main also has the fix
(DataDog/dd-trace-go#4361).

Only Node.js (FFL-1730) still needs the skip.
@leoromanovsky leoromanovsky mentioned this pull request Jan 26, 2026
Draft
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dd-oleksii dd-oleksii dd-oleksii approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@leoromanovsky @dd-oleksii