fix(openfeature): bound flagevaluation EVP payloads#4934
fix(openfeature): bound flagevaluation EVP payloads#4934leoromanovsky wants to merge 15 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: f4aa84f | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-01 14:03:18 Comparing candidate commit f4aa84f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 297 metrics, 2 unstable metrics, 1 flaky benchmarks without significant changes.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files
🚀 New features to boost your workflow:
|
…4-signed-logging # Conflicts: # openfeature/flageval_logging.go # openfeature/flageval_logging_hook.go # openfeature/flageval_logging_provider_test.go # openfeature/provider.go # openfeature/provider_bench_test.go
…46-evp-flagevaluation-go-limit # Conflicts: # openfeature/flageval_logging.go # openfeature/flageval_logging_hook.go # openfeature/flageval_logging_provider_test.go # openfeature/provider.go # openfeature/provider_bench_test.go
vjfridge
left a comment
There was a problem hiding this comment.
LGTM! Left some food for thought but no hard blockers. Thanks for taking this on!
|
/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.
leo.romanovsky@datadoghq.com unqueued this merge request |
genesor
left a comment
There was a problem hiding this comment.
LGTM, One question around the size limit.
|
|
||
| const ( | ||
| // PayloadSizeLimit is the EVP uncompressed request-body limit. | ||
| PayloadSizeLimit = 5 * 1024 * 1024 |
There was a problem hiding this comment.
Out of curiosity, I'm trying to understand why PayloadSizeLimit = 5 * 1024 * 1024 (5,242,880 bytes) and EventSizeLimit = 5_000_000 (5,000,000 bytes) differ by ~243 KB ?
There was a problem hiding this comment.
Thank you for bringing this up! I tried to be as minimally disruptive as possible and not change the LLMObs product, but my suspicion is that EventSizeLimit = 5_000_000 is not exactly correct. My intuition is that the real value is a multiple of a megabyte. I double-checked the agent and the real limit seems to be 10MB; updating here but still leaving LLMObs as-is.
Motivation
The merged Go flagevaluation writer aggregates bounded rows but still needed to enforce the shared EVP 5 MiB uncompressed JSON request limit before posting. At rollout scale, one flush can exceed that limit even when aggregation cardinality is bounded, so the writer must split/degrade/drop by encoded bytes without changing customer evaluation behavior or the worker-facing schema.
Changes
{context, flagEvaluations}wrapper.targeting_keyandcontext; drops/logs only if the degraded row still cannot fit.flagEvalMetricsHookfor OTel metrics andflagEvalLoggingHook/flagEvalLoggingWriterfor the EVP flagevaluation logging path.Decisions
flowchart TD A[drained aggregated rows] --> B[serialize candidate batch as JSON] B --> C{batch <= 5 MiB?} C -- yes --> D[post asynchronously through EVP proxy] C -- no --> E{current row fits degraded?} E -- yes --> F[omit targeting_key and context] F --> B E -- no --> G[drop, log, count]Validation Evidence
System Tests
Staging End-To-End
flagevaluationrows through the real Agent/EVP path: feat(openfeature): server-side EVP flagevaluation emission #4886Related PRs