Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSM Full propagation mode for SQL Server #5859

Merged
merged 25 commits into from
Sep 5, 2024
Merged

DSM Full propagation mode for SQL Server #5859

merged 25 commits into from
Sep 5, 2024

Conversation

vandonr
Copy link
Contributor

@vandonr vandonr commented Aug 7, 2024

Summary of changes

Adding support for Full propagation mode for SQL Server when using DSM.
DSM has 3 modes: disabled, service and full.
Service means that we inject info about the service who sent the query
Full means Service + trace ID and span ID.

Reason for change

We used to forbid Full mode for sql server because it uses a cache of query text -> execution plan, and injecting with comments like we did before would break the cache. (injecting service is OK because it doesn't change much, so the cache is still OK)
The solution that was found was to use the context_info to set the trace/span ID. It's independant from the query itself, and sets a "context" for the duration of the session that is visible to DBM when they sample a "slice" of the DB.
The drawback ofc is that it's an extra query (albeit a fast one) that we have to run before every "regular" query. It can be slow mostly if the network is slow. For this reason, we want to cover it with a span to clearly represent the time being spent there, so that clients can identify where the time is spent.

Implementation details

Test coverage

I added this scenario to the integration test, but since the query is parameterized, I don't think I can assert easily on the value that is set.

Other details

This is the dotnet equivalent of DataDog/dd-trace-java#7186

@andrewlock
Copy link
Member

andrewlock commented Aug 7, 2024

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

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5859) - mean (75ms)  : 70, 81
     .   : milestone, 75,
    master - mean (69ms)  : 67, 71
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (5859) - mean (1,096ms)  : 1064, 1128
     .   : milestone, 1096,
    master - mean (1,077ms)  : 1053, 1101
     .   : milestone, 1077,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5859) - mean (110ms)  : 104, 116
     .   : milestone, 110,
    master - mean (109ms)  : 105, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (5859) - mean (786ms)  : 745, 826
     .   : milestone, 786,
    master - mean (758ms)  : 739, 778
     .   : milestone, 758,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5859) - mean (93ms)  : 88, 97
     .   : milestone, 93,
    master - mean (92ms)  : 90, 95
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (5859) - mean (709ms)  : 691, 727
     .   : milestone, 709,
    master - mean (710ms)  : 697, 724
     .   : milestone, 710,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5859) - mean (191ms)  : 188, 194
     .   : milestone, 191,
    master - mean (190ms)  : 187, 192
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (5859) - mean (1,157ms)  : 1139, 1175
     .   : milestone, 1157,
    master - mean (1,160ms)  : 1137, 1184
     .   : milestone, 1160,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5859) - mean (275ms)  : 270, 280
     .   : milestone, 275,
    master - mean (274ms)  : 270, 279
     .   : milestone, 274,

    section CallTarget+Inlining+NGEN
    This PR (5859) - mean (921ms)  : 901, 942
     .   : milestone, 921,
    master - mean (922ms)  : 902, 942
     .   : milestone, 922,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5859) - mean (264ms)  : 259, 268
     .   : milestone, 264,
    master - mean (264ms)  : 259, 268
     .   : milestone, 264,

    section CallTarget+Inlining+NGEN
    This PR (5859) - mean (905ms)  : 884, 925
     .   : milestone, 905,
    master - mean (902ms)  : 883, 922
     .   : milestone, 902,

Loading

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Aug 7, 2024

Datadog Report

Branch report: vandonr/setcontext
Commit report: d7429c6
Test service: dd-trace-dotnet

✅ 0 Failed, 365540 Passed, 2089 Skipped, 15h 9m 39.67s Total Time

@andrewlock
Copy link
Member

andrewlock commented Aug 7, 2024

Benchmarks Report for tracer 🐌

Benchmarks for #5859 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.231
  • 1 benchmarks have fewer 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.84μs 44.3ns 319ns 0.0149 0.00747 0 5.43 KB
master StartStopWithChild netcoreapp3.1 10μs 55.7ns 348ns 0.0145 0.00482 0 5.62 KB
master StartStopWithChild net472 16μs 51.7ns 200ns 1.01 0.288 0.0881 6.06 KB
#5859 StartStopWithChild net6.0 7.95μs 44.5ns 292ns 0.0158 0.00789 0 5.43 KB
#5859 StartStopWithChild netcoreapp3.1 9.87μs 56.1ns 392ns 0.0248 0.00994 0 5.61 KB
#5859 StartStopWithChild net472 15.9μs 47.7ns 185ns 1.02 0.305 0.0937 6.07 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 499μs 261ns 1.01μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 638μs 270ns 1.05μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 850μs 595ns 2.31μs 0.422 0 0 3.3 KB
#5859 WriteAndFlushEnrichedTraces net6.0 478μs 279ns 1.04μs 0 0 0 2.7 KB
#5859 WriteAndFlushEnrichedTraces netcoreapp3.1 657μs 434ns 1.62μs 0 0 0 2.7 KB
#5859 WriteAndFlushEnrichedTraces net472 831μs 460ns 1.78μs 0.417 0 0 3.29 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 224μs 2.24μs 22.2μs 0.205 0 0 18.45 KB
master SendRequest netcoreapp3.1 235μs 1.43μs 14.1μs 0.254 0 0 20.61 KB
master SendRequest net472 0.00254ns 0.00105ns 0.00408ns 0 0 0 0 b
#5859 SendRequest net6.0 205μs 1.18μs 9.77μs 0.214 0 0 18.45 KB
#5859 SendRequest netcoreapp3.1 229μs 1.35μs 13μs 0.237 0 0 20.61 KB
#5859 SendRequest net472 0.000238ns 0.000165ns 0.000637ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #5859

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.69 KB 41.47 KB -221 B -0.53%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 584μs 2.42μs 9.05μs 0.571 0 0 41.69 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 686μs 3.82μs 24.2μs 0.342 0 0 41.64 KB
master WriteAndFlushEnrichedTraces net472 877μs 4.1μs 15.9μs 8.19 2.59 0.431 53.32 KB
#5859 WriteAndFlushEnrichedTraces net6.0 561μs 2.15μs 7.76μs 0.592 0 0 41.47 KB
#5859 WriteAndFlushEnrichedTraces netcoreapp3.1 696μs 3.82μs 22.6μs 0.34 0 0 41.78 KB
#5859 WriteAndFlushEnrichedTraces net472 877μs 2.97μs 10.7μs 8.3 2.62 0.437 53.32 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.25μs 1.4ns 5.41ns 0.0143 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.69μs 1.94ns 7.27ns 0.0135 0 0 1.02 KB
master ExecuteNonQuery net472 2.01μs 1.21ns 4.53ns 0.156 0 0 987 B
#5859 ExecuteNonQuery net6.0 1.22μs 1.24ns 4.64ns 0.0141 0 0 1.02 KB
#5859 ExecuteNonQuery netcoreapp3.1 1.77μs 2.21ns 7.98ns 0.0134 0 0 1.02 KB
#5859 ExecuteNonQuery net472 2.04μs 1.76ns 6.33ns 0.156 0 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.19μs 0.623ns 2.16ns 0.0137 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.59μs 0.569ns 2.13ns 0.0127 0 0 976 B
master CallElasticsearch net472 2.41μs 1.37ns 5.31ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.16μs 0.644ns 2.49ns 0.0134 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.6μs 0.79ns 2.95ns 0.0136 0 0 1.02 KB
master CallElasticsearchAsync net472 2.61μs 1.44ns 5.57ns 0.166 0 0 1.05 KB
#5859 CallElasticsearch net6.0 1.16μs 0.801ns 3ns 0.014 0 0 976 B
#5859 CallElasticsearch netcoreapp3.1 1.48μs 0.763ns 2.86ns 0.0134 0 0 976 B
#5859 CallElasticsearch net472 2.45μs 1.81ns 6.99ns 0.157 0 0 995 B
#5859 CallElasticsearchAsync net6.0 1.21μs 0.323ns 1.21ns 0.0133 0 0 952 B
#5859 CallElasticsearchAsync netcoreapp3.1 1.6μs 0.671ns 2.6ns 0.0136 0 0 1.02 KB
#5859 CallElasticsearchAsync net472 2.56μs 1.2ns 4.47ns 0.167 0 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.2μs 0.951ns 3.68ns 0.0132 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.59μs 0.906ns 3.51ns 0.0128 0 0 952 B
master ExecuteAsync net472 1.76μs 0.73ns 2.83ns 0.145 0 0 915 B
#5859 ExecuteAsync net6.0 1.33μs 0.651ns 2.43ns 0.0134 0 0 952 B
#5859 ExecuteAsync netcoreapp3.1 1.65μs 0.682ns 2.64ns 0.0124 0 0 952 B
#5859 ExecuteAsync net472 1.78μs 1.11ns 4.3ns 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.18μs 3ns 10.8ns 0.0313 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.06μs 2.06ns 7.96ns 0.0355 0 0 2.76 KB
master SendAsync net472 7.88μs 2.89ns 10.8ns 0.497 0 0 3.15 KB
#5859 SendAsync net6.0 4.05μs 2.84ns 11ns 0.0305 0 0 2.22 KB
#5859 SendAsync netcoreapp3.1 5.02μs 3.1ns 12ns 0.0376 0 0 2.76 KB
#5859 SendAsync net472 7.78μs 1.59ns 5.96ns 0.497 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.457ns 1.65ns 0.0231 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.2μs 5.09ns 18.4ns 0.0217 0 0 1.64 KB
master EnrichedLog net472 2.62μs 1.34ns 5.02ns 0.249 0 0 1.57 KB
#5859 EnrichedLog net6.0 1.49μs 0.85ns 3.29ns 0.0232 0 0 1.64 KB
#5859 EnrichedLog netcoreapp3.1 2.14μs 0.8ns 2.88ns 0.0215 0 0 1.64 KB
#5859 EnrichedLog net472 2.55μs 2.41ns 9.35ns 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 188ns 730ns 0 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 120μs 168ns 649ns 0.06 0 0 4.28 KB
master EnrichedLog net472 149μs 153ns 594ns 0.668 0.223 0 4.46 KB
#5859 EnrichedLog net6.0 114μs 101ns 377ns 0.0573 0 0 4.28 KB
#5859 EnrichedLog netcoreapp3.1 119μs 179ns 693ns 0 0 0 4.28 KB
#5859 EnrichedLog net472 146μs 218ns 843ns 0.661 0.22 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 3.01μs 2.55ns 9.88ns 0.0315 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.09μs 1.11ns 4.28ns 0.0287 0 0 2.2 KB
master EnrichedLog net472 4.85μs 1.88ns 7.27ns 0.319 0 0 2.02 KB
#5859 EnrichedLog net6.0 2.92μs 0.583ns 2.1ns 0.0307 0 0 2.2 KB
#5859 EnrichedLog netcoreapp3.1 4.13μs 1.59ns 6.15ns 0.029 0 0 2.2 KB
#5859 EnrichedLog net472 4.88μs 1.25ns 4.84ns 0.32 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.39μs 0.591ns 2.29ns 0.0159 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.7μs 1.25ns 4.82ns 0.0153 0 0 1.14 KB
master SendReceive net472 2.16μs 1.48ns 5.74ns 0.183 0.00107 0 1.16 KB
#5859 SendReceive net6.0 1.35μs 0.95ns 3.68ns 0.0157 0 0 1.14 KB
#5859 SendReceive netcoreapp3.1 1.85μs 1.68ns 6.51ns 0.0157 0 0 1.14 KB
#5859 SendReceive net472 2.13μs 1.06ns 3.95ns 0.183 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.91μs 0.892ns 3.46ns 0.0219 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.94μs 2.89ns 11.2ns 0.0216 0 0 1.65 KB
master EnrichedLog net472 4.43μs 0.962ns 3.33ns 0.323 0 0 2.04 KB
#5859 EnrichedLog net6.0 2.64μs 0.666ns 2.49ns 0.0224 0 0 1.6 KB
#5859 EnrichedLog netcoreapp3.1 3.85μs 1.62ns 6.26ns 0.0228 0 0 1.65 KB
#5859 EnrichedLog net472 4.34μs 1.88ns 7.29ns 0.322 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 400ns 0.142ns 0.532ns 0.00805 0 0 576 B
master StartFinishSpan netcoreapp3.1 588ns 0.562ns 2.18ns 0.00787 0 0 576 B
master StartFinishSpan net472 623ns 1.11ns 4.31ns 0.0916 0 0 578 B
master StartFinishScope net6.0 497ns 0.419ns 1.62ns 0.00967 0 0 696 B
master StartFinishScope netcoreapp3.1 731ns 1.96ns 7.61ns 0.00952 0 0 696 B
master StartFinishScope net472 884ns 0.993ns 3.84ns 0.104 0 0 658 B
#5859 StartFinishSpan net6.0 397ns 0.372ns 1.44ns 0.00819 0 0 576 B
#5859 StartFinishSpan netcoreapp3.1 576ns 0.443ns 1.72ns 0.00783 0 0 576 B
#5859 StartFinishSpan net472 643ns 0.362ns 1.4ns 0.0918 0 0 578 B
#5859 StartFinishScope net6.0 479ns 0.459ns 1.78ns 0.00968 0 0 696 B
#5859 StartFinishScope netcoreapp3.1 718ns 0.284ns 1.06ns 0.00949 0 0 696 B
#5859 StartFinishScope net472 902ns 0.854ns 3.31ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #5859

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.231 748.23 607.92

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 748ns 0.702ns 2.72ns 0.00971 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 1E+03ns 1.28ns 4.97ns 0.00912 0 0 696 B
master RunOnMethodBegin net472 1.11μs 1.33ns 5.14ns 0.104 0 0 658 B
#5859 RunOnMethodBegin net6.0 608ns 0.332ns 1.28ns 0.00976 0 0 696 B
#5859 RunOnMethodBegin netcoreapp3.1 903ns 0.798ns 3.09ns 0.00947 0 0 696 B
#5859 RunOnMethodBegin net472 1.1μs 1.1ns 4.26ns 0.104 0 0 658 B

@vandonr vandonr changed the title [WIP] DSM Full propagation mode for SQL Server DSM Full propagation mode for SQL Server Aug 14, 2024
@andrewlock
Copy link
Member

andrewlock commented Aug 14, 2024

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5859) (11.275M)   : 0, 11274726
    master (11.164M)   : 0, 11164446
    benchmarks/2.9.0 (11.168M)   : 0, 11167899

    section Automatic
    This PR (5859) (7.431M)   : 0, 7431421
    master (7.489M)   : 0, 7489050
    benchmarks/2.9.0 (7.868M)   : 0, 7868380

    section Trace stats
    master (7.691M)   : 0, 7690564

    section Manual
    master (11.109M)   : 0, 11109337

    section Manual + Automatic
    This PR (5859) (6.815M)   : 0, 6814974
    master (6.834M)   : 0, 6834101

    section DD_TRACE_ENABLED=0
    master (10.373M)   : 0, 10372679

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5859) (9.602M)   : 0, 9601583
    master (9.552M)   : 0, 9552016

    section Automatic
    This PR (5859) (6.723M)   : 0, 6722793
    master (6.705M)   : 0, 6704955

    section Trace stats
    master (6.859M)   : 0, 6858624

    section Manual
    master (9.602M)   : 0, 9601727

    section Manual + Automatic
    This PR (5859) (6.086M)   : 0, 6085872
    master (6.103M)   : 0, 6102529

    section DD_TRACE_ENABLED=0
    master (9.021M)   : 0, 9020990

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5859) (10.160M)   : 0, 10159582
    benchmarks/2.9.0 (10.014M)   : 0, 10013706

    section Automatic
    This PR (5859) (6.897M)   : 0, 6897364
    benchmarks/2.9.0 (7.309M)   : 0, 7308591

    section Manual + Automatic
    This PR (5859) (6.428M)   : 0, 6427891

Loading

Copy link
Contributor

github-actions bot commented Aug 21, 2024

Snapshots difference summary

The following differences have been observed in committed snapshots. It is meant to help the reviewer.
The diff is simplistic, so please check some files anyway while we improve it.

168 occurrences of :

+      _dd.dbm_trace_injected: true
[...]
+      dd.instrumentation.time_ms: 123.456,

168 occurrences of :

+      _dd.dbm_trace_injected: true,
[...]
+    },
+    Metrics: {
+      dd.instrumentation.time_ms: 123.456

@vandonr vandonr marked this pull request as ready for review August 21, 2024 11:47
@vandonr vandonr requested review from a team as code owners August 21, 2024 11:47
Co-authored-by: Lucas Pimentel <[email protected]>
@vandonr
Copy link
Contributor Author

vandonr commented Sep 2, 2024

I tried locally by running a long command (WAITFOR DELAY '00:00:20') in the integration tests, and looking at the context set by querying the DB at the same time:
image
Looks fine I think.

Copy link
Member

@lucaspimentel lucaspimentel left a comment

Choose a reason for hiding this comment

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

Left a few small comments. LGTM otherwise.

Copy link
Contributor

@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.

Looks good to me thanks!


// Since sending the query to the DB can be a bit long, we register the time it took for transparency.
// Not using _dd because we want the customers to be able to see that tag.
span.SetMetric("dd.instrumentation.time_ms", stopwatch.Elapsed.TotalMilliseconds);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is dd.instrumentation.time_ms just a one-off name or is this something common? never seen it before and search doesn't show any other usages anywhere in the org

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm yeah it's a one off, I'm actually not aware of any existing instrumentation that would be long enough that we'd want to document how long it takes.

@vandonr vandonr merged commit bf9253b into master Sep 5, 2024
70 of 72 checks passed
@vandonr vandonr deleted the vandonr/setcontext branch September 5, 2024 11:52
@github-actions github-actions bot added this to the vNext-v3 milestone Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants