feat: add /trace/manual_keep_drop endpoint to all weblogs - #7407
Merged
Conversation
Contributor
|
|
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 2 jobs - 2 passed on retry 🔗 Commit SHA: 0137f2c | Docs | Datadog PR Page | Give us feedback! |
genesor
marked this pull request as ready for review
July 29, 2026 11:36
genesor
requested review from
Yun-Kim,
claponcet,
dromanol,
florentinl,
mcculls,
ncybul and
zacharycmontoya
and removed request for
a team
July 29, 2026 11:36
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 475f02ddb8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
bm1549
reviewed
Jul 29, 2026
bm1549
left a comment
bm1549
left a comment
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
bm1549
approved these changes
Jul 29, 2026
bm1549
left a comment
bm1549
left a comment
Contributor
There was a problem hiding this comment.
Overall Codex has some suggestions about manifest versions and a PR label, which I agree with
Outside of that, this is all good to go
anna-git
reviewed
Jul 30, 2026
genesor
force-pushed
the
ben.db/feat-manual-keep-drop-endpoint
branch
from
July 30, 2026 10:15
fcf4630 to
98a80cd
Compare
genesor
force-pushed
the
ben.db/feat-manual-keep-drop-endpoint
branch
from
July 30, 2026 10:16
98a80cd to
a984740
Compare
vpellan
approved these changes
Jul 30, 2026
anna-git
approved these changes
Jul 30, 2026
… weblogs without outbound headers
nccatoni
approved these changes
Jul 31, 2026
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
8 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.
Motivation
Nothing in the end-to-end suite can force a trace's sampling decision. The manual keep/drop API was only reachable from the parametric apps, so the "a manual decision overrides everything else" contract had no end-to-end coverage, and no weblog exposed an entry point for it.
Changes
New weblog endpoint
GET /trace/manual_keep_drop?decision=keep|dropapplies the tracer's manual keep/drop API to the current trace and returns200. A missing or invaliddecisionreturns400. Documented indocs/understand/weblogs/end-to-end_weblog.md.Implemented in every end-to-end weblog, one commit per variant, using each library's own manual keep/drop API:
GlobalTracer.get().activeSpan()+DDTags.MANUAL_KEEP/MANUAL_DROPtracer.current_span().set_tag(MANUAL_KEEP_KEY / MANUAL_DROP_KEY)tracer.scope().active().setTag(MANUAL_KEEP / MANUAL_DROP, true), constants fromext.tagsTracer.Instance.ActiveScope?.Span.SetTag(Tags.ManualKeep / ManualDrop, "true")Endpoints/ManualKeepDropEndpoint.csDatadog::Tracing.active_trace.keep!/.reject!common.ManualKeepDrophandler +ext.ManualKeep/ext.ManualDrop\DDTrace\set_priority_sampling(...)cpp and rust are skipped: they have no end-to-end weblog, only parametric.
Notes on three of those choices:
app/_shared/common/common.goas anhttp.HandlerFunc, matching howrasp.LFI,common.Requestdownstreamanddbm.StubDbmHandlerare already shared. gin and echo reuse the existingginHandleFunc/echoHandleFuncadapters.set_priority_samplingis what\DDTrace\Span::setTagitself calls for themanual.keep/manual.droptags. Writing the tag into span meta was not viable: in symfony and laravel\DDTrace\active_span()is thesymfony.controller/laravel.actionchild span rather than the root, and the meta path is skipped entirely when a keep decision was propagated from upstream.utils/build/docker/nodejs/express/app.js, so a single commit covers both. nextjs usesglobal._ddtraceand the literal tag names, matching its neighbouring routes, which avoid directdd-traceimports because of webpack bundling.Parametric fix
utils/build/docker/nodejs/parametric/server.jsdestructuredMANUAL_KEEP/MANUAL_DROPoff the top level ofdd-trace/ext, where they do not exist (they live underext.tags). It was therefore callingspan.setTag(undefined, true), so the existing nodejs parametric manual-keep coverage was asserting nothing.Tests
tests/test_sampling_manual.py::Test_Manual_Sampling, SAMPLING scenario,@features.ensure_that_sampling_is_consistent_across_languages. Both directions are driven by an upstream distributed trace context, so each case asserts that the manual decision wins over a propagated one:x-datadog-sampling-prioritydecision_sampling_priority_v1test_manual_keep_overrides_upstream_drop0(auto reject)keep2(user keep)test_manual_drop_overrides_upstream_keep2(user keep)drop-1(user reject)The SAMPLING scenario is used because it sets
DD_TRACE_STATS_COMPUTATION_ENABLED=false, so dropped P-1 traces still reach the agent and the drop case is observable. A manual decision takes precedence over that scenario's 0.5 sample rate, so the rate does not affect either assertion.Impact
The drop direction is new ground: the parametric suite only ever covered manual keep overriding an upstream drop. Asserting the reverse surfaced two library gaps, both declared in the manifests so they run as xfail and turn into an xpass when fixed.
bug (APMAPI-2194)ontest_manual_drop_overrides_upstream_keepmanual.keepgoes throughDDSpanContext.forceKeep(), an unconditional update that deliberately wins over an already propagated decision, whilemanual.dropgoes throughsetSamplingPriority(), a compare-and-set fromUNSETthat refuses when the priority is locked. Keep can override, drop cannot.bug (APMAPI-1720)on the whole classPrioritySampler.sample()returns early whenever a priority already exists, before it reads the manual tags, so neither direction is honoured.APMAPI-2194 is filed separately rather than folded into APMAPI-1720 because 1720's analysis only covers the keep direction and lists java as working.
dotnet, golang, php, python and ruby honour both directions.
Weblogs with no SDK to call also get declarations, since they run the SAMPLING scenario:
c.yml(perl-mojolicious)missing_feature, dd-trace-c does not implement manual keep/dropcpp_httpd.ymlmissing_feature, endpoint not implementedcpp_kong.yml,cpp_nginx.ymlirrelevant, proxy rather than an applicationjava_otel.yml,nodejs_otel.yml,python_otel.ymlirrelevant, the OpenTelemetry test library has no Datadog manual keep/dropWorkflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present