Thread allocationKey into ResolutionDetails flagMetadata#3534
Conversation
🐑 PR Shepherd is maintaining this PRI watch your PR and automatically fix CI failures, rebase your branch, handle flaky tests, and push it to the merge queue when it's ready. More about what I do → Guide To pause me on this PR, add the |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3534 +/- ##
===========================================
+ Coverage 72.22% 72.26% +0.04%
===========================================
Files 966 966
Lines 35644 35654 +10
Branches 5947 5951 +4
===========================================
+ Hits 25741 25763 +22
+ Misses 8281 8274 -7
+ Partials 1622 1617 -5
🚀 New features to boost your workflow:
|
- Add toEvaluationMetadata() helper that builds EvaluationMetadata from flagMetadata via the public Builder API (EvaluationMetadata constructor is internal in kotlin-sdk 0.6.2) - Wire metadata into toProviderEvaluation() - Add test verifying allocationKey round-trips through flagMetadata
|
FFL-2510 |
| val result = resolution.toProviderEvaluation() | ||
|
|
||
| // Then | ||
| assertThat(result.metadata?.getString("allocationKey")).isEqualTo(fakeAllocationKey) |
There was a problem hiding this comment.
Fails CI due to metadata is non-null
e: warnings found and -Werror specified
w: file:///go/src/github.com/DataDog/dd-sdk-android/features/dd-sdk-android-flags-openfeature/src/test/kotlin/com/datadog/android/flags/openfeature/internal/adapters/ConvertersTest.kt:154:35 Unnecessary safe call on a non-null receiver of type EvaluationMetadata
| assertThat(result.metadata?.getString("allocationKey")).isEqualTo(fakeAllocationKey) | |
| assertThat(result.metadata.getString("allocationKey")).isEqualTo(fakeAllocationKey) |
| is String -> builder.putString(key, value) | ||
| is Boolean -> builder.putBoolean(key, value) | ||
| is Int -> builder.putInt(key, value) | ||
| is Double -> builder.putDouble(key, value) |
There was a problem hiding this comment.
question: do we not need to worry about Long with builder.putLong? https://github.com/open-feature/kotlin-sdk/blob/cfe7ac44d92aef0366e4e6cb8bd01ce070ea06d7/kotlin-sdk/src/commonMain/kotlin/dev/openfeature/kotlin/sdk/FlagEvaluationMetadata.kt#L48
The Builder API doesn't have putLong; Long values fall through to putString as-is via the else branch.
|
Verified via |
|
/merge |
|
View all feedbacks in Devflow UI.
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.
The expected merge time in
|
| @BoolForgery fakeValue: Boolean | ||
| ) { | ||
| // Given — Builder has no putLong (only putInt/putDouble/putString/putBoolean); | ||
| // Long values fall through to the else branch and are stored as their toString() representation. |
There was a problem hiding this comment.
long value can be converted to int though if it fits
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
4 participants
What does this PR do?
The
allocationKeyfield from the Precomputed Flags API response (PrecomputedFlag) was not included inResolutionDetails.flagMetadata, making it unavailable to callers who useresolve()for rich evaluation details (e.g. OpenFeature providers).This PR threads
allocationKeyintoflagMetadataso it is accessible alongside otherextraLoggingfields.Motivation
OpenFeature providers (e.g.
dd-openfeature-provider-kotlin) expose evaluation metadata to consumers via theEvaluationMetadatamap.allocationKeyis a key field for attribution and debugging. Without it inflagMetadata, providers cannot surface it to OpenFeature clients.Additional Notes
buildMetadata(PrecomputedFlag)replaces the oldextractMetadata(JSONObject)helper (dead code removed).allocationKeyis only added when non-blank.allocationKeyedge case to verify it is excluded from metadata.Review checklist (to be filled by reviewers)