[Feature Flags] Send the split serial id on exposure events - #9088
Open
danyal002 wants to merge 3 commits into
Open
[Feature Flags] Send the split serial id on exposure events#9088danyal002 wants to merge 3 commits into
danyal002 wants to merge 3 commits into
Conversation
BenchmarksBenchmark execution time: 2026-08-21 18:04:32 Comparing candidate commit 227e672 in PR branch Found 0 performance improvements and 4 performance regressions! Performance is the same for 68 metrics, 0 unstable metrics, 65 known flaky benchmarks, 61 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9088) and master.
|
|||||||||||||||||||||||||
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 191.63 ± (191.44 - 192.16) ms | 201.90 ± (202.75 - 204.39) ms | +5.4% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1150.18 ± (1150.15 - 1156.46) ms | 1217.60 ± (1214.62 - 1224.80) ms | +5.9% | ❌⬆️ |
danyal002
added
area:tracer
danyal002
marked this pull request as ready for review
August 20, 2026 17:22
danyal002
requested review from
greghuels and
typotter
and removed request for
a team
August 20, 2026 17:22
The intake uses the serial id to find the holdout that an allocation comes from. The compiler rewrites a holdout into a usual allocation before the tracer receives it, so the exposure event records no other link back to it. The evaluator passes the serial id from the split directly to the exposure dispatch. It does not read it from the flag metadata, because the metadata carries the serial id only when span enrichment is on. The dedup cache now compares the serial id as well. A serial id that appears or changes while the allocation and the variant stay the same was swallowed before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AttributeTests forbids an attribute named argument whose type is not in the core assembly, because such an attribute can throw CustomAttributeFormatException when the owning assembly does not resolve. The JsonProperty NullValueHandling argument is an enum in Datadog.Trace, so it broke that test on every framework. Use the Newtonsoft ShouldSerialize convention instead, as ProbeException.ShouldSerializeStackTrace does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
danyal002
force-pushed
the
danyal.khan/EX-3416
branch
from
August 20, 2026 20:24
0dc148a to
37ae122
Compare
andrewlock
reviewed
Aug 21, 2026
…teOnly The field is internal only so that a test can assert the wire shape. The attribute lets the analyzer stop other types from using it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
typotter
approved these changes
Aug 21, 2026
| public const string ExposurePath = "evp_proxy/v2/api/v2/exposures"; | ||
| private static readonly JsonSerializerSettings SerializerSettings = new() | ||
| [TestingAndPrivateOnly] | ||
| internal static readonly JsonSerializerSettings SerializerSettings = new() |
There was a problem hiding this comment.
Agree with the suggestion to add the attribute:
[TestingAndPrivateOnly]
internal static readonly JsonSerializerSettings SerializerSettings = new()| Assert.True(cache.Add(CreateSerialIdEvent(340132))); | ||
| Assert.True(cache.Add(CreateSerialIdEvent(340133))); | ||
| Assert.True(cache.Add(CreateSerialIdEvent(340132))); | ||
| Assert.Equal(1, cache.Size); |
There was a problem hiding this comment.
All three Add calls return true (not deduplicated) but Size stays 1. Consider adding a comment that this is intended and why size stays 1.
// Size stays 1: the key (flag + subject) is shared across all three events.
// Each Add replaces the stored Value rather than growing the cache.
Assert.Equal(1, cache.Size);
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
3 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.
Summary of changes
Add an optional
serial_idfield to the feature-flag exposure event, and send it when the split carries a serial id.Reason for change
The exposures intake uses the serial id to find the holdout that an allocation comes from. The UFC compiler rewrites a holdout into a usual allocation before the tracer receives the flag configuration, so the exposure event holds no other link back to the holdout.
The tracer already reads the serial id and uses it for APM span enrichment. This change also puts it on the exposure event.
Implementation details
Exposure/Model/ExposureEvent.csSerialIdproperty. It goes on the wire as a top-levelserial_id.FeatureFlagsEvaluator.csExposure/ExposureCache.csExposure/ExposureApi.csSerializerSettingsinternal, so that a test can assert the wire shape.ResolveVariantgives the serial id toDispatchExposuredirectly. It does not read the value from the flag metadata, becauseResolveVariantwrites that metadata entry only when span enrichment is on.The serializer includes nulls, so
ExposureEventgivesSerialIdaShouldSerializeSerialIdmethod. An event without a serial id omits the key instead of sendingnull. AJsonPropertynamed argument cannot do this, becauseAttributeTestsforbids an attribute argument whose type is not in the core assembly, andNullValueHandlingis an enum inDatadog.Trace.ProbeException.ShouldSerializeStackTracesolves the same problem the same way.The tracer does not validate the value. The flag configuration layer validates it.
Test coverage
Nine new test cases in three classes:
ExposureEventTests— the wire shape:serial_idfor340132,serial_idfor0, and no key when the value is absent.FeatureFlagsEvaluatorTests— the dispatched event, for a split with a serial id and for a split without one.ExposureCacheTests— deduplication when the serial id appears, changes, disappears, and when it is0.A serial id of
0is a usual value. Serial ids start at0for each organization, so the first allocation of every organization has one. The tests pin0as present, not as absent.378 tests pass.
Other details
The deduplication cache now compares the serial id. A subject therefore sends one more exposure when its serial id appears or changes while the allocation key and the variant key stay the same. This is intended: a configuration refresh can add or change a serial id without changing either key, and the intake must receive the new value.
Part of the work to send the serial id from every Datadog SDK.
DataDog/dd-trace-py#19753andDataDog/dd-trace-go#5214make the same change.This description was generated by Claude.