Skip to content

Fix incorrect instrumentation for new TracerSettings(bool)#5949

Merged
andrewlock merged 1 commit into
masterfrom
andrew/fix-instrumentation
Aug 27, 2024
Merged

Fix incorrect instrumentation for new TracerSettings(bool)#5949
andrewlock merged 1 commit into
masterfrom
andrew/fix-instrumentation

Conversation

@andrewlock

@andrewlock andrewlock commented Aug 27, 2024

Copy link
Copy Markdown
Member

Summary of changes

Fix bug in v3 instrumentation of new TracerSettings(bool useDefaultSources)

Reason for change

We were missing a parameter in the instrumentation

Implementation details

Add the missing parameter

Test coverage

We don't have a good coverage for this and I'm not sure the best way to catch it in the future tbh 🤔 Typically we catch these when developing integrations.

I guess in this case the best way is to call every API and check for the side effects, but that also feels like a lot of work, so inclined to just YOLO it unless others think it's worth the effort

@andrewlock andrewlock added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) labels Aug 27, 2024
@andrewlock
andrewlock requested review from a team as code owners August 27, 2024 09:13
@datadog-ddstaging

datadog-ddstaging Bot commented Aug 27, 2024 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: andrew/fix-instrumentation
Commit report: 5f454ef
Test service: dd-trace-dotnet

✅ 0 Failed, 301984 Passed, 1618 Skipped, 12h 31m 1.95s Total Time

@andrewlock

andrewlock commented Aug 27, 2024

Copy link
Copy Markdown
Member Author

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

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 shown 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).

@andrewlock

andrewlock commented Aug 27, 2024

Copy link
Copy Markdown
Member Author

Benchmarks Report for tracer 🐌

Benchmarks for #5949 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.115
  • 1 benchmarks are slower, with geometric mean 1.296
  • All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 7.68μs 42.5ns 272ns 0.0148 0.00738 0 5.43 KB
master StartStopWithChild netcoreapp3.1 9.91μs 52.5ns 273ns 0.0192 0.00962 0 5.62 KB
master StartStopWithChild net472 16μs 55.9ns 216ns 1.02 0.302 0.103 6.06 KB
#5949 StartStopWithChild net6.0 7.59μs 43.2ns 302ns 0.0117 0.0039 0 5.43 KB
#5949 StartStopWithChild netcoreapp3.1 9.77μs 48.6ns 217ns 0.0194 0.0097 0 5.62 KB
#5949 StartStopWithChild net472 16μs 30.4ns 118ns 1.04 0.316 0.0949 6.06 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 465μs 200ns 722ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 634μs 278ns 1.08μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 836μs 677ns 2.62μs 0.417 0 0 3.3 KB
#5949 WriteAndFlushEnrichedTraces net6.0 468μs 225ns 843ns 0 0 0 2.7 KB
#5949 WriteAndFlushEnrichedTraces netcoreapp3.1 626μs 526ns 1.97μs 0 0 0 2.7 KB
#5949 WriteAndFlushEnrichedTraces net472 836μs 384ns 1.49μs 0.417 0 0 3.3 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 192μs 1.11μs 9.5μs 0.182 0 0 18.45 KB
master SendRequest netcoreapp3.1 208μs 1.15μs 7.46μs 0.198 0 0 20.61 KB
master SendRequest net472 0.001ns 0.000448ns 0.00168ns 0 0 0 0 b
#5949 SendRequest net6.0 185μs 977ns 5.07μs 0.183 0 0 18.45 KB
#5949 SendRequest netcoreapp3.1 208μs 1.15μs 6.69μs 0.214 0 0 20.61 KB
#5949 SendRequest net472 0.000326ns 0.000188ns 0.00073ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 555μs 2.33μs 8.42μs 0.566 0 0 41.57 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 669μs 2.33μs 8.41μs 0.326 0 0 41.73 KB
master WriteAndFlushEnrichedTraces net472 852μs 2.35μs 9.1μs 8.39 2.52 0.419 53.3 KB
#5949 WriteAndFlushEnrichedTraces net6.0 600μs 3.14μs 15.7μs 0.566 0 0 41.75 KB
#5949 WriteAndFlushEnrichedTraces netcoreapp3.1 679μs 3.03μs 11.7μs 0.338 0 0 41.66 KB
#5949 WriteAndFlushEnrichedTraces net472 895μs 3.12μs 12.1μs 8.36 2.64 0.44 53.33 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.23μs 1.01ns 3.8ns 0.0146 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.73μs 1.47ns 5.71ns 0.0137 0 0 1.02 KB
master ExecuteNonQuery net472 1.98μs 1.89ns 7.31ns 0.157 0 0 987 B
#5949 ExecuteNonQuery net6.0 1.25μs 1.64ns 6.34ns 0.0142 0 0 1.02 KB
#5949 ExecuteNonQuery netcoreapp3.1 1.65μs 1.57ns 6.1ns 0.0134 0 0 1.02 KB
#5949 ExecuteNonQuery net472 1.99μs 1.24ns 4.47ns 0.156 0 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5949

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net6.0 1.115 1,232.91 1,105.71

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.23μs 0.999ns 3.87ns 0.0135 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.63μs 0.555ns 2.15ns 0.013 0 0 976 B
master CallElasticsearch net472 2.43μs 1.72ns 6.66ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.38μs 1.31ns 4.91ns 0.0131 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.57μs 1.27ns 4.76ns 0.014 0 0 1.02 KB
master CallElasticsearchAsync net472 2.58μs 1.71ns 6.42ns 0.167 0 0 1.05 KB
#5949 CallElasticsearch net6.0 1.11μs 0.457ns 1.65ns 0.0136 0 0 976 B
#5949 CallElasticsearch netcoreapp3.1 1.58μs 0.784ns 2.83ns 0.0127 0 0 976 B
#5949 CallElasticsearch net472 2.47μs 2.1ns 8.13ns 0.158 0 0 995 B
#5949 CallElasticsearchAsync net6.0 1.39μs 1.24ns 4.81ns 0.0132 0 0 952 B
#5949 CallElasticsearchAsync netcoreapp3.1 1.6μs 0.79ns 2.96ns 0.0137 0 0 1.02 KB
#5949 CallElasticsearchAsync net472 2.57μs 2.17ns 8.42ns 0.167 0.00128 0 1.05 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.26μs 0.661ns 2.47ns 0.0133 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.55μs 0.555ns 2ns 0.0131 0 0 952 B
master ExecuteAsync net472 1.78μs 1.47ns 5.71ns 0.145 0 0 915 B
#5949 ExecuteAsync net6.0 1.29μs 0.932ns 3.61ns 0.0136 0 0 952 B
#5949 ExecuteAsync netcoreapp3.1 1.61μs 0.797ns 2.98ns 0.0128 0 0 952 B
#5949 ExecuteAsync net472 1.8μs 0.661ns 2.47ns 0.145 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.21μs 2.09ns 8.11ns 0.0315 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.09μs 2.27ns 8.81ns 0.0358 0 0 2.76 KB
master SendAsync net472 7.75μs 3.24ns 12.6ns 0.499 0 0 3.15 KB
#5949 SendAsync net6.0 4.25μs 1.2ns 4.33ns 0.0299 0 0 2.22 KB
#5949 SendAsync netcoreapp3.1 5.2μs 2.61ns 10.1ns 0.0361 0 0 2.76 KB
#5949 SendAsync net472 7.71μs 0.951ns 3.68ns 0.499 0 0 3.15 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.48μs 0.708ns 2.65ns 0.023 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.38μs 1.1ns 4.12ns 0.0217 0 0 1.64 KB
master EnrichedLog net472 2.68μs 1.93ns 7.24ns 0.249 0 0 1.57 KB
#5949 EnrichedLog net6.0 1.59μs 1.29ns 4.66ns 0.0233 0 0 1.64 KB
#5949 EnrichedLog netcoreapp3.1 2.36μs 1.22ns 4.73ns 0.0225 0 0 1.64 KB
#5949 EnrichedLog net472 2.62μs 1.17ns 4.37ns 0.249 0 0 1.57 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 116μs 166ns 621ns 0.0578 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 119μs 234ns 905ns 0 0 0 4.28 KB
master EnrichedLog net472 148μs 225ns 872ns 0.668 0.223 0 4.46 KB
#5949 EnrichedLog net6.0 118μs 139ns 538ns 0 0 0 4.28 KB
#5949 EnrichedLog netcoreapp3.1 119μs 180ns 699ns 0 0 0 4.28 KB
#5949 EnrichedLog net472 149μs 237ns 919ns 0.668 0.223 0 4.46 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.95μs 1.62ns 6.26ns 0.0313 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.09μs 1.07ns 4.01ns 0.0286 0 0 2.2 KB
master EnrichedLog net472 4.78μs 2.91ns 11.3ns 0.319 0 0 2.02 KB
#5949 EnrichedLog net6.0 3.14μs 0.988ns 3.83ns 0.0301 0 0 2.2 KB
#5949 EnrichedLog netcoreapp3.1 4.16μs 2.33ns 9.04ns 0.0291 0 0 2.2 KB
#5949 EnrichedLog net472 4.82μs 0.807ns 3.13ns 0.319 0 0 2.02 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.37μs 0.604ns 2.34ns 0.0164 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.76μs 1.38ns 5.35ns 0.0149 0 0 1.14 KB
master SendReceive net472 2.18μs 1.64ns 5.9ns 0.184 0.00109 0 1.16 KB
#5949 SendReceive net6.0 1.26μs 0.371ns 1.39ns 0.0158 0 0 1.14 KB
#5949 SendReceive netcoreapp3.1 1.84μs 0.748ns 2.9ns 0.0156 0 0 1.14 KB
#5949 SendReceive net472 2.17μs 1.55ns 6.02ns 0.184 0 0 1.16 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.81μs 0.997ns 3.86ns 0.0225 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.92μs 1.92ns 7.43ns 0.0217 0 0 1.65 KB
master EnrichedLog net472 4.36μs 2.83ns 11ns 0.324 0 0 2.04 KB
#5949 EnrichedLog net6.0 2.8μs 0.802ns 3.11ns 0.0224 0 0 1.6 KB
#5949 EnrichedLog netcoreapp3.1 3.95μs 1.61ns 6.23ns 0.0217 0 0 1.65 KB
#5949 EnrichedLog net472 4.25μs 1.72ns 6.18ns 0.323 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 396ns 0.124ns 0.463ns 0.00819 0 0 576 B
master StartFinishSpan netcoreapp3.1 606ns 0.414ns 1.6ns 0.00788 0 0 576 B
master StartFinishSpan net472 630ns 0.601ns 2.33ns 0.0918 0 0 578 B
master StartFinishScope net6.0 605ns 0.291ns 1.13ns 0.0097 0 0 696 B
master StartFinishScope netcoreapp3.1 717ns 0.508ns 1.97ns 0.00972 0 0 696 B
master StartFinishScope net472 841ns 0.845ns 3.27ns 0.104 0 0 658 B
#5949 StartFinishSpan net6.0 391ns 1.58ns 5.92ns 0.00816 0 0 576 B
#5949 StartFinishSpan netcoreapp3.1 619ns 1.24ns 4.8ns 0.00799 0 0 576 B
#5949 StartFinishSpan net472 616ns 0.205ns 0.739ns 0.0916 0 0 578 B
#5949 StartFinishScope net6.0 545ns 0.901ns 3.49ns 0.00964 0 0 696 B
#5949 StartFinishScope netcoreapp3.1 786ns 0.383ns 1.43ns 0.00954 0 0 696 B
#5949 StartFinishScope net472 834ns 0.446ns 1.67ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5949

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.296 578.90 750.53

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 579ns 0.354ns 1.37ns 0.00966 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 922ns 1.08ns 4.02ns 0.00931 0 0 696 B
master RunOnMethodBegin net472 1.05μs 0.801ns 3.1ns 0.104 0 0 658 B
#5949 RunOnMethodBegin net6.0 750ns 0.408ns 1.58ns 0.00979 0 0 696 B
#5949 RunOnMethodBegin netcoreapp3.1 876ns 0.703ns 2.63ns 0.00927 0 0 696 B
#5949 RunOnMethodBegin net472 1.15μs 1.62ns 6.27ns 0.104 0 0 658 B

@andrewlock
andrewlock merged commit 40ba1e2 into master Aug 27, 2024
@andrewlock
andrewlock deleted the andrew/fix-instrumentation branch August 27, 2024 16:10
@github-actions github-actions Bot added this to the vNext-v3 milestone Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@kevingosse kevingosse kevingosse approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) area:integrations area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@andrewlock @kevingosse