Skip to content

chore(sampling): add manual keep parametric system test#5617

Merged
genesor merged 16 commits into
mainfrom
ben.db/parametric-manual-keep
Jan 7, 2026
Merged

chore(sampling): add manual keep parametric system test#5617
genesor merged 16 commits into
mainfrom
ben.db/parametric-manual-keep

Conversation

@genesor

@genesor genesor commented Oct 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Motivation

We've been having issues regarding sampling decisions in our services. Some services wants to manual keep a trace and the Go tracer currently cannot manual keep a trace if the upstream decision was to drop it.

This system test ensure that we have a consistent way of handling manual keep over distributed traces.

3 tracers need a fix (and rust does not implement manual keep), a JIRA card has been created for follow-up

Changes

New parametric system test for manual keep

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@github-actions

github-actions Bot commented Oct 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

tests/parametric/test_sampling_manual.py                                @DataDog/system-tests-core @DataDog/apm-sdk-capabilities
utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentracing/dto/SpanManualSamplingArgs.java  @DataDog/apm-java @DataDog/asm-java @DataDog/system-tests-core
manifests/cpp.yml                                                       @DataDog/dd-trace-cpp
manifests/java.yml                                                      @DataDog/asm-java @DataDog/apm-java
manifests/nodejs.yml                                                    @DataDog/dd-trace-js
manifests/rust.yml                                                      @DataDog/apm-rust
utils/build/docker/dotnet/parametric/Endpoints/ApmTestApi.cs            @DataDog/apm-dotnet @DataDog/asm-dotnet @DataDog/system-tests-core
utils/build/docker/golang/parametric/datadog.go                         @DataDog/dd-trace-go-guild @DataDog/system-tests-core
utils/build/docker/golang/parametric/helpers.go                         @DataDog/dd-trace-go-guild @DataDog/system-tests-core
utils/build/docker/golang/parametric/main.go                            @DataDog/dd-trace-go-guild @DataDog/system-tests-core
utils/build/docker/java/parametric/src/main/java/com/datadoghq/trace/opentracing/controller/OpenTracingController.java  @DataDog/apm-java @DataDog/asm-java @DataDog/system-tests-core
utils/build/docker/nodejs/parametric/server.js                          @DataDog/dd-trace-js @DataDog/system-tests-core
utils/build/docker/php/parametric/server.php                            @DataDog/apm-php @DataDog/system-tests-core
utils/build/docker/python/parametric/apm_test_client/server.py          @DataDog/apm-python @DataDog/asm-python @DataDog/system-tests-core
utils/build/docker/ruby/parametric/server.rb                            @DataDog/ruby-guild @DataDog/asm-ruby @DataDog/system-tests-core
utils/build/docker/rust/parametric/src/datadog/mod.rs                   @DataDog/apm-rust @DataDog/system-tests-core
utils/docker_fixtures/_test_clients/_test_client_parametric.py          @DataDog/system-tests-core

@genesor genesor force-pushed the ben.db/parametric-manual-keep branch 6 times, most recently from 5bd282f to 7fb4095 Compare October 31, 2025 20:00
@genesor genesor marked this pull request as ready for review October 31, 2025 20:24
@genesor genesor requested review from a team as code owners October 31, 2025 20:24
@genesor genesor requested review from dmehala, jandro996, mhlidd and robertpi and removed request for a team October 31, 2025 20:24
cbeauchesne
cbeauchesne requested changes Nov 3, 2025
View reviewed changes

@cbeauchesne cbeauchesne left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny detail

Comment thread tests/parametric/test_sampling_manual.py Outdated
@genesor genesor requested a review from cbeauchesne November 3, 2025 10:25
@genesor genesor force-pushed the ben.db/parametric-manual-keep branch from 90a48a4 to 7a70d82 Compare November 3, 2025 10:39
cbeauchesne
cbeauchesne approved these changes Nov 3, 2025
View reviewed changes

@cbeauchesne cbeauchesne left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From framework usage all good.

But I suggest you to ask a review from someone familiair with the feature

bm1549
bm1549 approved these changes Nov 4, 2025
View reviewed changes

@bm1549 bm1549 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly added test case looks solid to me

I think we should introduce more to cover all of the combinations of behaviors, but that should not block this PR from merging

zacharycmontoya
zacharycmontoya reviewed Nov 4, 2025
View reviewed changes
Comment thread tests/parametric/test_sampling_manual.py Outdated
@cbeauchesne

cbeauchesne commented Nov 26, 2025

Copy link
Copy Markdown
Collaborator

Hi @genesor , Do you need some help to fix lint issue ?

@genesor

genesor commented Nov 26, 2025

Copy link
Copy Markdown
Member Author

Hi @genesor , Do you need some help to fix lint issue ?

Hi @cbeauchesne I should be fine, thanks! I left this PR opened for too long, I will fix the lint, dig into the CPP and Ruby issues then merge it this week.

@genesor genesor force-pushed the ben.db/parametric-manual-keep branch 3 times, most recently from 2ac905f to 8f16857 Compare January 6, 2026 21:16
@genesor genesor force-pushed the ben.db/parametric-manual-keep branch from 8f16857 to cc25427 Compare January 6, 2026 21:40
@cbeauchesne

cbeauchesne commented Jan 7, 2026

Copy link
Copy Markdown
Collaborator

Updating your branch to fix CI issues

mcculls
mcculls reviewed Jan 7, 2026
View reviewed changes
Comment thread manifests/cpp.yml
zacharycmontoya
zacharycmontoya approved these changes Jan 7, 2026
View reviewed changes

@zacharycmontoya zacharycmontoya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for getting this testing in place

@genesor genesor merged commit ae80150 into main Jan 7, 2026
397 checks passed
@genesor genesor deleted the ben.db/parametric-manual-keep branch January 7, 2026 18:04
@mcculls mcculls mentioned this pull request Jan 7, 2026
Merged
@cbeauchesne cbeauchesne mentioned this pull request Jan 8, 2026
Merged
5 tasks
This was referenced Jan 8, 2026
Closed
Merged
dubloom pushed a commit that referenced this pull request Jan 8, 2026
@genesor @cbeauchesne @dubloom
chore(sampling): add manual keep parametric system test (#5617)
73078b2 
Co-authored-by: Charles de Beauchesne <charles.debeauchesne@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcculls mcculls mcculls approved these changes

@darccio darccio darccio approved these changes

@cbeauchesne cbeauchesne cbeauchesne approved these changes

@bm1549 bm1549 bm1549 approved these changes

@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes

@robertpi robertpi Awaiting requested review from robertpi robertpi is a code owner automatically assigned from DataDog/asm-java

@jandro996 jandro996 Awaiting requested review from jandro996 jandro996 is a code owner automatically assigned from DataDog/asm-java

@mhlidd mhlidd Awaiting requested review from mhlidd mhlidd is a code owner automatically assigned from DataDog/apm-java

@dmehala dmehala Awaiting requested review from dmehala dmehala is a code owner automatically assigned from DataDog/dd-trace-cpp

@emmettbutler emmettbutler Awaiting requested review from emmettbutler emmettbutler is a code owner automatically assigned from DataDog/apm-python

@florentinl florentinl Awaiting requested review from florentinl florentinl is a code owner automatically assigned from DataDog/apm-python

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@genesor @cbeauchesne @mcculls @darccio @bm1549 @zacharycmontoya