Feature: Sampling delegation by dmehala · Pull Request #59 · DataDog/dd-trace-cpp

dmehala · 2023-09-28T13:47:57Z

How to test

docker compose build --build-arg BRANCH=dmehala/sampling-delegation --no-cache
[DD_TRACE_DELEGATE_SAMPLING=<0 or 1>] [DD_API_KEY=<API_KEY>] docker compose up
Enjoy! 🥳

pr-commenter · 2023-09-28T13:49:58Z

Benchmarks

Benchmark execution time: 2024-01-12 13:07:37

Comparing candidate commit 9fc8f73 in PR branch dmehala/sampling-delegation with baseline commit 89a8e36 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

examples/http-server/server/server.cpp

examples/http-server/proxy/proxy.cpp

src/datadog/trace_segment.cpp

examples/http-server/proxy/proxy.cpp

examples/http-server/common/httplib-helper.hpp

src/datadog/trace_segment.cpp

examples/http-server/server/server.cpp

src/datadog/tags.h

src/datadog/tracer.cpp

test/test_tracer_config.cpp

src/datadog/trace_segment.h

dgoffredo · 2024-01-10T17:19:13Z

the combination of all these boolean is used to delegate or not. May I suggest a documentation in dd-trace-cpp repository to explain/details this?

Good idea, I will add that.

I can't approve because it's a draft PR. I don't know how you want to proceed, I foresee these possibilities: [...]

I'll do (1) once I've tidied up my PR.

Thanks for the review!

- Protect `struct SamplingDelegation` with a mutex. - Check the result of `finalize_config(config3)` in `test_tracer.cpp`.

Since there were quite a few conflicts, there's a fair bit of restructuring here as well.

dgoffredo

These changes look good to me. I've merged in the recommendations from #83, and merged in main.

The merge of main was substantial, so please check whether you're ok with the current state of this PR.

examples/http-server/Dockerfile

src/datadog/trace_segment.cpp

* feat: Set integration informations * run `make format` --------- Co-authored-by: David Goffredo <[email protected]>

This fixes a regression introduced by Sampling Delegation (#59). When the sampling delegation header is present during the trace context extraction, the sampling priority is discarded even if the feature is disabled. This change of behaviour can potentially result in an increase of ingested spans. Additionnally, the tracer should behave as before the introduction of this feature when it is not enabled. Changes: - Sampling priority is not longer discard when `x-datadog-delegate-trace-sampling` is present. - Update sampling delegation tests.

This fixes a regression introduced by Sampling Delegation (#59). When the sampling delegation header is present during the trace context extraction, the sampling priority is discarded even if the feature is disabled. This change of behaviour can potentially result in an increase of ingested spans. Additionally, the tracer should behave as before the introduction of this feature when it is not enabled. Changes: - Sampling priority is not longer discard when`x-datadog-delegate-trace-sampling` is present. - Update sampling delegation tests.

dmehala added 3 commits September 26, 2023 13:34

core implementation + simple test

1ab97a3

add tests

5a7206c

wip: delegate example

16bc538

dmehala commented Sep 28, 2023

View reviewed changes

examples/http-server/server/server.cpp Outdated Show resolved Hide resolved

dmehala commented Sep 28, 2023

View reviewed changes

examples/http-server/server/server.cpp Outdated Show resolved Hide resolved

dmehala commented Sep 28, 2023

View reviewed changes

examples/http-server/proxy/proxy.cpp Outdated Show resolved Hide resolved

dmehala commented Sep 28, 2023

View reviewed changes

examples/http-server/proxy/proxy.cpp Outdated Show resolved Hide resolved

dmehala commented Sep 28, 2023

View reviewed changes

examples/http-server/proxy/proxy.cpp Show resolved Hide resolved

dmehala commented Sep 28, 2023

View reviewed changes

src/datadog/trace_segment.cpp Outdated Show resolved Hide resolved

dmehala requested review from cgilmour and dgoffredo September 28, 2023 14:34

dmehala force-pushed the dmehala/sampling-delegation branch from d9b880a to 0907717 Compare September 28, 2023 15:10

self code review

e0a5048

dmehala force-pushed the dmehala/sampling-delegation branch from 0907717 to e0a5048 Compare September 28, 2023 15:15

Rework docker-compose

92ca1f8