Skip to content

Add srv_src field to client stats payload#8339

Open
NachoEchevarria wants to merge 35 commits intomasterfrom
nacho/ServiceSourceStats
Open

Add srv_src field to client stats payload#8339
NachoEchevarria wants to merge 35 commits intomasterfrom
nacho/ServiceSourceStats

Conversation

@NachoEchevarria
Copy link
Copy Markdown
Collaborator

@NachoEchevarria NachoEchevarria commented Mar 19, 2026

Summary of changes

Add srv_src field to the client stats payload, propagating the service name source alongside trace stats.

Reason for change

Per the service name source RFC, the source must be propagated in both trace and stats payloads. The trace payload (_dd.svc_src span meta) was added in PR #8302. This completes the implementation by adding srv_src at the stats bucket level.

Implementation details

  • StatsAggregationKey.cs: Added ServiceSource field. It participates in equality and hash code, so stats with different sources aggregate into distinct buckets.
  • StatsAggregator.cs: BuildKey() now reads span.Context.ServiceNameSource and passes it to the aggregation key.
  • StatsBuffer.cs: SerializeBucket() conditionally serializes srv_src when ServiceSource is non-null (dynamic map size: 13 with source, 12 without).

Test coverage

  • KeyEquality_WithServiceSource: Verifies that keys with different sources are not equal.
  • Serialization_WithServiceSource: Verifies srv_src is serialized when present and absent when null.
  • Existing StatsBufferTests and StatsAggregatorTests updated and passing.

Other details

APMLP-1015

Related agent PR: DataDog/datadog-agent#45982
Reference Java implementation: DataDog/dd-trace-java#10653

@pr-commenter
Copy link
Copy Markdown

pr-commenter bot commented Mar 19, 2026

Benchmarks

Benchmark execution time: 2026-03-27 17:20:18

Comparing candidate commit ab85a20 in PR branch nacho/ServiceSourceStats with baseline commit 017e910 in branch master.

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces net6.0

  • 🟥 execution_time [+77.269ms; +77.423ms] or [+60.816%; +60.937%]

scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0

  • 🟥 execution_time [+11.335ms; +17.883ms] or [+5.657%; +8.925%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0

  • 🟥 execution_time [+24.996ms; +28.971ms] or [+14.064%; +16.301%]

scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1

  • 🟥 execution_time [+14.087ms; +21.474ms] or [+9.154%; +13.955%]
  • 🟥 throughput [-186.445op/s; -132.633op/s] or [-11.954%; -8.504%]

scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark net6.0

  • 🟥 allocated_mem [+19.932KB; +19.976KB] or [+7.721%; +7.738%]

scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore netcoreapp3.1

  • 🟩 throughput [+14850769.141op/s; +16106974.803op/s] or [+6.585%; +7.142%]

scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope net6.0

  • 🟥 execution_time [+12.276ms; +15.614ms] or [+6.162%; +7.838%]

scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope netcoreapp3.1

  • 🟥 execution_time [+16.156ms; +21.618ms] or [+8.177%; +10.942%]

@NachoEchevarria NachoEchevarria added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Mar 19, 2026
@dd-trace-dotnet-ci-bot
Copy link
Copy Markdown

dd-trace-dotnet-ci-bot bot commented Mar 19, 2026

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (8339) and master.

⚠️ Potential regressions detected

HttpMessageHandler

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration193.46 ± (193.25 - 194.07) ms220.36 ± (219.82 - 221.33) ms+13.9%❌⬆️
.NET Framework 4.8 - Bailout
duration196.16 ± (196.05 - 196.84) ms232.81 ± (232.17 - 234.83) ms+18.7%❌⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1150.65 ± (1151.26 - 1157.73) ms1263.45 ± (1263.90 - 1273.02) ms+9.8%❌⬆️
Full Metrics Comparison

FakeDbCommand

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration72.15 ± (72.15 - 72.46) ms71.82 ± (71.86 - 72.21) ms-0.5%
.NET Framework 4.8 - Bailout
duration76.39 ± (76.17 - 76.50) ms76.51 ± (76.26 - 76.60) ms+0.2%✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1073.18 ± (1072.51 - 1078.17) ms1077.76 ± (1077.77 - 1085.43) ms+0.4%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms22.40 ± (22.35 - 22.45) ms22.29 ± (22.26 - 22.33) ms-0.5%
process.time_to_main_ms83.68 ± (83.44 - 83.92) ms83.17 ± (82.97 - 83.36) ms-0.6%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.91 ± (10.91 - 10.91) MB10.92 ± (10.91 - 10.92) MB+0.1%✅⬆️
runtime.dotnet.threads.count12 ± (12 - 12)12 ± (12 - 12)+0.0%
.NET Core 3.1 - Bailout
process.internal_duration_ms22.25 ± (22.21 - 22.29) ms22.32 ± (22.29 - 22.36) ms+0.3%✅⬆️
process.time_to_main_ms84.98 ± (84.78 - 85.18) ms85.18 ± (84.96 - 85.40) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.94 ± (10.94 - 10.95) MB10.95 ± (10.95 - 10.96) MB+0.1%✅⬆️
runtime.dotnet.threads.count13 ± (13 - 13)13 ± (13 - 13)+0.0%
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms223.85 ± (222.81 - 224.88) ms222.19 ± (221.08 - 223.30) ms-0.7%
process.time_to_main_ms533.57 ± (532.31 - 534.83) ms533.84 ± (532.68 - 535.00) ms+0.1%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed48.32 ± (48.29 - 48.35) MB48.25 ± (48.22 - 48.28) MB-0.1%
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)-1.1%
.NET 6 - Baseline
process.internal_duration_ms21.00 ± (20.97 - 21.02) ms20.99 ± (20.96 - 21.02) ms-0.0%
process.time_to_main_ms72.14 ± (71.99 - 72.29) ms72.06 ± (71.91 - 72.21) ms-0.1%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.63 ± (10.63 - 10.63) MB10.64 ± (10.63 - 10.64) MB+0.1%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 6 - Bailout
process.internal_duration_ms21.02 ± (20.99 - 21.06) ms20.85 ± (20.81 - 20.89) ms-0.8%
process.time_to_main_ms73.19 ± (73.01 - 73.37) ms72.80 ± (72.64 - 72.97) ms-0.5%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.73 ± (10.73 - 10.73) MB10.75 ± (10.75 - 10.75) MB+0.2%✅⬆️
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms216.01 ± (214.61 - 217.40) ms387.14 ± (384.80 - 389.48) ms+79.2%✅⬆️
process.time_to_main_ms530.99 ± (529.64 - 532.35) ms531.85 ± (530.91 - 532.79) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed50.12 ± (50.10 - 50.15) MB50.36 ± (50.33 - 50.38) MB+0.5%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 29)28 ± (28 - 28)-1.3%
.NET 8 - Baseline
process.internal_duration_ms19.14 ± (19.09 - 19.18) ms19.35 ± (19.31 - 19.38) ms+1.1%✅⬆️
process.time_to_main_ms71.36 ± (71.18 - 71.54) ms71.97 ± (71.81 - 72.13) ms+0.9%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.69 ± (7.68 - 7.70) MB7.68 ± (7.68 - 7.69) MB-0.1%
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 8 - Bailout
process.internal_duration_ms19.35 ± (19.31 - 19.38) ms19.13 ± (19.10 - 19.17) ms-1.1%
process.time_to_main_ms73.21 ± (73.05 - 73.37) ms72.31 ± (72.17 - 72.45) ms-1.2%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.75 ± (7.74 - 7.76) MB7.73 ± (7.72 - 7.73) MB-0.3%
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms160.07 ± (159.25 - 160.89) ms308.18 ± (305.85 - 310.50) ms+92.5%✅⬆️
process.time_to_main_ms490.08 ± (489.03 - 491.13) ms490.98 ± (490.08 - 491.89) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed36.93 ± (36.90 - 36.95) MB37.25 ± (37.23 - 37.27) MB+0.9%✅⬆️
runtime.dotnet.threads.count28 ± (28 - 28)27 ± (27 - 27)-1.6%

HttpMessageHandler

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration193.46 ± (193.25 - 194.07) ms220.36 ± (219.82 - 221.33) ms+13.9%❌⬆️
.NET Framework 4.8 - Bailout
duration196.16 ± (196.05 - 196.84) ms232.81 ± (232.17 - 234.83) ms+18.7%❌⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1150.65 ± (1151.26 - 1157.73) ms1263.45 ± (1263.90 - 1273.02) ms+9.8%❌⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms187.25 ± (186.89 - 187.62) ms213.03 ± (212.41 - 213.65) ms+13.8%✅⬆️
process.time_to_main_ms80.70 ± (80.47 - 80.93) ms91.28 ± (91.06 - 91.50) ms+13.1%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.14 ± (16.11 - 16.17) MB15.83 ± (15.82 - 15.85) MB-1.9%
runtime.dotnet.threads.count20 ± (19 - 20)20 ± (20 - 20)+1.9%✅⬆️
.NET Core 3.1 - Bailout
process.internal_duration_ms186.52 ± (186.21 - 186.82) ms211.82 ± (211.16 - 212.47) ms+13.6%✅⬆️
process.time_to_main_ms82.09 ± (81.94 - 82.24) ms92.87 ± (92.60 - 93.13) ms+13.1%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.15 ± (16.13 - 16.18) MB15.91 ± (15.90 - 15.93) MB-1.5%
runtime.dotnet.threads.count21 ± (20 - 21)21 ± (21 - 21)+1.9%✅⬆️
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms395.92 ± (394.50 - 397.34) ms425.33 ± (423.76 - 426.91) ms+7.4%✅⬆️
process.time_to_main_ms522.87 ± (521.84 - 523.90) ms572.37 ± (571.09 - 573.66) ms+9.5%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed58.82 ± (58.67 - 58.98) MB59.16 ± (59.11 - 59.21) MB+0.6%✅⬆️
runtime.dotnet.threads.count30 ± (30 - 30)30 ± (30 - 30)+0.5%✅⬆️
.NET 6 - Baseline
process.internal_duration_ms191.61 ± (191.24 - 191.97) ms219.07 ± (218.44 - 219.71) ms+14.3%✅⬆️
process.time_to_main_ms69.75 ± (69.61 - 69.89) ms79.87 ± (79.66 - 80.09) ms+14.5%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.12 ± (15.98 - 16.25) MB16.15 ± (16.13 - 16.17) MB+0.2%✅⬆️
runtime.dotnet.threads.count19 ± (18 - 19)20 ± (20 - 20)+4.9%✅⬆️
.NET 6 - Bailout
process.internal_duration_ms191.37 ± (190.95 - 191.78) ms218.06 ± (217.44 - 218.67) ms+13.9%✅⬆️
process.time_to_main_ms71.00 ± (70.85 - 71.16) ms80.99 ± (80.80 - 81.18) ms+14.1%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.07 ± (15.92 - 16.22) MB16.20 ± (16.19 - 16.22) MB+0.8%✅⬆️
runtime.dotnet.threads.count20 ± (20 - 20)21 ± (20 - 21)+4.1%✅⬆️
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms413.66 ± (412.25 - 415.07) ms600.34 ± (597.36 - 603.32) ms+45.1%✅⬆️
process.time_to_main_ms523.82 ± (522.76 - 524.87) ms574.67 ± (573.35 - 575.99) ms+9.7%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed60.94 ± (60.90 - 60.99) MB61.72 ± (61.63 - 61.81) MB+1.3%✅⬆️
runtime.dotnet.threads.count31 ± (30 - 31)31 ± (31 - 31)+1.1%✅⬆️
.NET 8 - Baseline
process.internal_duration_ms188.84 ± (188.46 - 189.21) ms214.99 ± (214.38 - 215.59) ms+13.8%✅⬆️
process.time_to_main_ms69.45 ± (69.24 - 69.65) ms78.23 ± (78.03 - 78.42) ms+12.6%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.79 ± (11.77 - 11.82) MB11.51 ± (11.49 - 11.52) MB-2.4%
runtime.dotnet.threads.count18 ± (18 - 18)19 ± (19 - 19)+4.3%✅⬆️
.NET 8 - Bailout
process.internal_duration_ms188.15 ± (187.87 - 188.42) ms213.99 ± (213.42 - 214.56) ms+13.7%✅⬆️
process.time_to_main_ms70.31 ± (70.19 - 70.44) ms79.42 ± (79.24 - 79.60) ms+13.0%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.80 ± (11.73 - 11.86) MB11.56 ± (11.54 - 11.57) MB-2.0%
runtime.dotnet.threads.count19 ± (19 - 19)20 ± (20 - 20)+5.0%✅⬆️
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms340.99 ± (339.88 - 342.11) ms609.73 ± (603.75 - 615.72) ms+78.8%✅⬆️
process.time_to_main_ms480.85 ± (479.91 - 481.79) ms531.86 ± (530.90 - 532.82) ms+10.6%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed48.81 ± (48.77 - 48.85) MB51.83 ± (51.75 - 51.91) MB+6.2%✅⬆️
runtime.dotnet.threads.count30 ± (30 - 30)30 ± (30 - 30)+0.1%✅⬆️
Comparison explanation

Execution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

Duration charts
FakeDbCommand (.NET Framework 4.8)
gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (72ms)  : 69, 75
    master - mean (72ms)  : 70, 75

    section Bailout
    This PR (8339) - mean (76ms)  : 75, 78
    master - mean (76ms)  : 75, 78

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (1,082ms)  : 1027, 1136
    master - mean (1,075ms)  : 1035, 1115

Loading
FakeDbCommand (.NET Core 3.1)
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (112ms)  : 109, 116
    master - mean (113ms)  : 107, 119

    section Bailout
    This PR (8339) - mean (114ms)  : 112, 117
    master - mean (114ms)  : 111, 117

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (795ms)  : 768, 822
    master - mean (796ms)  : 773, 818

Loading
FakeDbCommand (.NET 6)
gantt
    title Execution time (ms) FakeDbCommand (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (99ms)  : 96, 103
    master - mean (99ms)  : 96, 103

    section Bailout
    This PR (8339) - mean (100ms)  : 97, 103
    master - mean (100ms)  : 97, 103

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (947ms)  : crit, 914, 980
    master - mean (782ms)  : 758, 807

Loading
FakeDbCommand (.NET 8)
gantt
    title Execution time (ms) FakeDbCommand (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (99ms)  : 96, 102
    master - mean (98ms)  : 95, 101

    section Bailout
    This PR (8339) - mean (99ms)  : 96, 102
    master - mean (100ms)  : 98, 103

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (830ms)  : crit, 795, 865
    master - mean (689ms)  : 668, 710

Loading
HttpMessageHandler (.NET Framework 4.8)
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (221ms)  : 209, 232
    master - mean (194ms)  : 189, 198

    section Bailout
    This PR (8339) - mean (234ms)  : crit, 214, 253
    master - mean (196ms)  : 193, 200

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (1,268ms)  : crit, 1198, 1339
    master - mean (1,154ms)  : 1108, 1201

Loading
HttpMessageHandler (.NET Core 3.1)
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (315ms)  : 304, 325
    master - mean (276ms)  : 270, 283

    section Bailout
    This PR (8339) - mean (314ms)  : crit, 302, 326
    master - mean (277ms)  : 273, 281

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (1,040ms)  : crit, 1006, 1073
    master - mean (950ms)  : 921, 979

Loading
HttpMessageHandler (.NET 6)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (308ms)  : 294, 322
    master - mean (269ms)  : 264, 274

    section Bailout
    This PR (8339) - mean (308ms)  : crit, 297, 319
    master - mean (270ms)  : 265, 276

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (1,207ms)  : crit, 1156, 1258
    master - mean (968ms)  : 945, 991

Loading
HttpMessageHandler (.NET 8)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (8339) - mean (304ms)  : 292, 316
    master - mean (268ms)  : 262, 274

    section Bailout
    This PR (8339) - mean (304ms)  : crit, 294, 314
    master - mean (268ms)  : 264, 272

    section CallTarget+Inlining+NGEN
    This PR (8339) - mean (1,172ms)  : crit, 1082, 1263
    master - mean (852ms)  : 833, 871

Loading

@NachoEchevarria NachoEchevarria changed the title Nacho/service source stats Add srv_src field to client stats payload Mar 19, 2026
@NachoEchevarria NachoEchevarria marked this pull request as ready for review March 20, 2026 11:34
@NachoEchevarria NachoEchevarria requested a review from a team as a code owner March 20, 2026 11:34
Copy link
Copy Markdown
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

As discussed, we're missing a lot from the spec, but hey 🤷‍♂️


if (hasServiceSource)
{
MessagePackBinary.WriteString(stream, "srv_src");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Based on the stats computation spec, I think this should be service_source, but given that none of our keys match here, I don't know what's correct 😅

Suggested change
MessagePackBinary.WriteString(stream, "srv_src");
MessagePackBinary.WriteString(stream, "service_source");

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We should wait for clarification. Thanks for finding that!

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.

Does this need to be service_source?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, its srv_src. We had a discussion. Also, the system tests PR confirms the field name: DataDog/system-tests#6565

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The spec Andrew posted above needs to be updated?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it does, but they are already notified.

[Key("TopLevelHits")]
public long TopLevelHits { get; set; }

[Key("srv_src")]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
[Key("srv_src")]
[Key("service_source")]

@NachoEchevarria NachoEchevarria requested a review from a team as a code owner March 26, 2026 10:52
@NachoEchevarria
Copy link
Copy Markdown
Collaborator Author

Note: System test was tested manually by chaging temprarily the pipeline: https://github.com/DataDog/system-tests/pull/6598/changes

Copy link
Copy Markdown
Collaborator

@bouwkast bouwkast left a comment

Choose a reason for hiding this comment

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

LGTM

The service_source vs svc_src question doesn't look answered yet which seems to be the only concern


if (hasServiceSource)
{
MessagePackBinary.WriteString(stream, "srv_src");
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.

Does this need to be service_source?

@lucaspimentel
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2afede67f4

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

span.ServiceName,
span.OperationName,
span.Type,
span.Context.ServiceNameSource,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Normalize service source before adding it to stats key

Use of span.Context.ServiceNameSource here bypasses the safety normalization used for trace serialization: when a span’s service equals the default service, non-opt.* sources are intentionally cleared before _dd.svc_src is emitted. In this path, those sources are still preserved, so spans can produce stats buckets with srv_src (and separate aggregation keys) even when the corresponding trace payload drops the source. A concrete case is setting ISpan.ServiceName to the default service, which sets source m; this now creates inconsistent trace-vs-stats attribution and extra timeseries cardinality.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

setting ISpan.ServiceName to the default service, which sets source m

That's a good question. Should this be considered "manual" or "default"? (in both the _dd.svc_src tag and the trace stats)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we should apply the same logic for that corner case. If user sets the service name to Default, no tags or stats should be emitted. Fixed.

@NachoEchevarria
Copy link
Copy Markdown
Collaborator Author

LGTM

The service_source vs svc_src question doesn't look answered yet which seems to be the only concern

We had a discussion regarding the topic and the right name is srv_src. Also, the system tests PR confirms the field name: DataDog/system-tests#6565

public readonly string Service;
public readonly string OperationName;
public readonly string Type;
public readonly string ServiceSource;
Copy link
Copy Markdown
Member

@lucaspimentel lucaspimentel Mar 27, 2026

Choose a reason for hiding this comment

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

Not a blocker, but maybe add #nullable enabled while you're here and declare some of these as string?? 😅

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point. I can do that in a subsequent PR. We would need to check what fields can be null and check the callers. It's worth to do that, but probably in a new PR. Also, using scoped namespaces. I promise I will create a subsequent PR. :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a good point. I can do that in a subsequent PR

Please don't, it'll conflict with mine 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants