fix(flags): dedupe exposures by latest assignment#3526
Conversation
9daf3ee to
695e6ef
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3526 +/- ##
===========================================
- Coverage 72.22% 72.19% -0.03%
===========================================
Files 965 965
Lines 35635 35630 -5
Branches 5947 5947
===========================================
- Hits 25736 25723 -13
- Misses 8282 8289 +7
- Partials 1617 1618 +1
🚀 New features to boost your workflow:
|
695e6ef to
53af82b
Compare
|
🐑 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bed9e6ad17
ℹ️ 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".
| val lastSentValue = exposuresSentCache[cacheKey] | ||
| if (lastSentValue != cacheValue) { | ||
| @Suppress("UnsafeThirdPartyFunctionCall") // safe - non-null key and value | ||
| exposuresSentCache.put(cacheKey, cacheValue) |
There was a problem hiding this comment.
Keep cache updates ordered with exposure writes
When the same subject/flag is evaluated concurrently with different assignments, this cache update can happen before the previous exposure is actually written. For example, thread A caches A and pauses, thread B caches and writes B, then thread A writes A; the cache still says B, so the next B exposure is suppressed even though the latest emitted assignment was A. The new latest-assignment contract only holds if updating the cache and emitting the exposure remain ordered for a given cache key.
Useful? React with 👍 / 👎.
Motivation
Android flags exposure deduplication currently treats every previously seen
targetingKey + flagName + allocationKey + variationKeytuple as permanently sent. That suppresses valid exposure events when the same subject and flag cycles assignments, for exampleA -> B -> A, because the finalAwas seen earlier.Related iOS PR: DataDog/dd-sdk-ios#2987 applies the same mobile exposure-cache contract.
Changes
targetingKey + flagName.allocationKey + variationKeyas the cache value.LruCache, but make it entry-count based with a 5,000-entry limit instead of using approximate object-size accounting.A -> B -> Awrites three exposure events.Decisions
sizeOf; AndroidXLruCachedefaults to one unit per entry whensizeOfis not overridden.JAVA_HOME=/opt/homebrew/Cellar/openjdk@17/17.0.19/libexec/openjdk.jdk/Contents/Home ANDROID_HOME=/opt/homebrew/share/android-commandlinetools ./gradlew :features:dd-sdk-android-flags:testDebugUnitTestpassed.