Make TracerSettings properly immutable and remove ImmutableTracerSettings#6415
Conversation
…utableDirectLogSubmissionSettings` (#6400) ## Summary of changes Merge `DirectLogSubmissionSettings` with `ImmutableDirectLogSubmissionSettings` ## Reason for change There was never really a good reason for having these as separate types. It was primarily to make testing a little easier and to mirror the `TracerSettings`/`ImmutableTracerSettings` dichotomy, but as that's going away, this is just unnecessary complexity ## Implementation details - Moved the additional logic that was previously inside `ImmutableDirectLogSubmissionSettings` into `DirectLogSubmissionSettings` - Renamed the `DirectLogSubmissionSettings` properties to match the "Immutable" version (and remove the unnecessary prefix) - Replace all usages of `Immutable*` with `DirectLogSubmissionSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization (using `IConfigurationSource`) ## Test coverage Essentially the same. I removed/tweaked some tests that are no longer relevant ## Other details Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 👈 This PR - #6405 - #6408 - #6415
c4cd7f8 to
248d0d8
Compare
f3258ab to
10ebd53
Compare
Datadog ReportBranch report: ✅ 0 Failed, 465592 Passed, 3681 Skipped, 33h 13m 49.74s Total Time New Flaky Tests (1)
|
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:
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 (6415) - mean (69ms) : 65, 73
. : milestone, 69,
master - mean (69ms) : 65, 72
. : milestone, 69,
section CallTarget+Inlining+NGEN
This PR (6415) - mean (976ms) : 954, 997
. : milestone, 976,
master - mean (978ms) : 952, 1004
. : milestone, 978,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6415) - mean (107ms) : 105, 109
. : milestone, 107,
master - mean (108ms) : 105, 110
. : milestone, 108,
section CallTarget+Inlining+NGEN
This PR (6415) - mean (674ms) : 660, 687
. : milestone, 674,
master - mean (680ms) : 663, 697
. : milestone, 680,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6415) - mean (91ms) : 89, 93
. : milestone, 91,
master - mean (91ms) : 89, 93
. : milestone, 91,
section CallTarget+Inlining+NGEN
This PR (6415) - mean (631ms) : 616, 646
. : milestone, 631,
master - mean (639ms) : 624, 653
. : milestone, 639,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6415) - mean (194ms) : 188, 199
. : milestone, 194,
master - mean (194ms) : 190, 199
. : milestone, 194,
section CallTarget+Inlining+NGEN
This PR (6415) - mean (1,099ms) : 1068, 1129
. : milestone, 1099,
master - mean (1,104ms) : 1076, 1132
. : milestone, 1104,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6415) - mean (278ms) : 273, 283
. : milestone, 278,
master - mean (277ms) : 272, 281
. : milestone, 277,
section CallTarget+Inlining+NGEN
This PR (6415) - mean (868ms) : 833, 903
. : milestone, 868,
master - mean (874ms) : 847, 901
. : milestone, 874,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6415) - mean (267ms) : 264, 271
. : milestone, 267,
master - mean (267ms) : 263, 271
. : milestone, 267,
section CallTarget+Inlining+NGEN
This PR (6415) - mean (852ms) : 822, 882
. : milestone, 852,
master - mean (856ms) : 823, 888
. : milestone, 856,
|
248d0d8 to
c039b45
Compare
10ebd53 to
f68edfc
Compare
c039b45 to
5b80296
Compare
f68edfc to
63b55e2
Compare
Benchmarks Report for appsec 🐌Benchmarks for #6415 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.Asm.AppSecBodyBenchmark - Faster 🎉 Fewer allocations 🎉
|
| Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net472 | 1.258 | 211.02 | 167.69 | |
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472 | 1.183 | 4,248.21 | 3,589.85 |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody‑net472 | 234.39 KB | 228.67 KB | -5.71 KB | -2.44% |
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody‑net472 | 230.87 KB | 225.16 KB | -5.71 KB | -2.47% |
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody‑netcoreapp3.1 | 207.17 KB | 199.07 KB | -8.1 KB | -3.91% |
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody‑netcoreapp3.1 | 203.75 KB | 195.66 KB | -8.1 KB | -3.97% |
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody‑net6.0 | 200.01 KB | 191.88 KB | -8.13 KB | -4.06% |
| Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody‑net6.0 | 196.5 KB | 188.38 KB | -8.13 KB | -4.14% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | AllCycleSimpleBody |
net6.0 | 212μs | 104ns | 388ns | 2.75 | 0 | 0 | 196.5 KB |
| master | AllCycleSimpleBody |
netcoreapp3.1 | 315μs | 165ns | 640ns | 2.8 | 0 | 0 | 203.75 KB |
| master | AllCycleSimpleBody |
net472 | 281μs | 379ns | 1.47μs | 36.7 | 2.39 | 0 | 230.87 KB |
| master | AllCycleMoreComplexBody |
net6.0 | 215μs | 210ns | 756ns | 2.79 | 0 | 0 | 200.01 KB |
| master | AllCycleMoreComplexBody |
netcoreapp3.1 | 319μs | 163ns | 565ns | 2.71 | 0 | 0 | 207.17 KB |
| master | AllCycleMoreComplexBody |
net472 | 288μs | 384ns | 1.44μs | 37.1 | 2.29 | 0 | 234.39 KB |
| master | ObjectExtractorSimpleBody |
net6.0 | 138ns | 0.152ns | 0.57ns | 0.00392 | 0 | 0 | 280 B |
| master | ObjectExtractorSimpleBody |
netcoreapp3.1 | 215ns | 0.0878ns | 0.304ns | 0.00368 | 0 | 0 | 272 B |
| master | ObjectExtractorSimpleBody |
net472 | 211ns | 0.324ns | 1.26ns | 0.0446 | 0 | 0 | 281 B |
| master | ObjectExtractorMoreComplexBody |
net6.0 | 2.94μs | 1.04ns | 3.87ns | 0.0531 | 0 | 0 | 3.78 KB |
| master | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 3.79μs | 2.95ns | 11.4ns | 0.0507 | 0 | 0 | 3.69 KB |
| master | ObjectExtractorMoreComplexBody |
net472 | 4.26μs | 9ns | 34.9ns | 0.602 | 0.00643 | 0 | 3.8 KB |
| #6415 | AllCycleSimpleBody |
net6.0 | 202μs | 109ns | 392ns | 2.63 | 0 | 0 | 188.38 KB |
| #6415 | AllCycleSimpleBody |
netcoreapp3.1 | 302μs | 88.2ns | 330ns | 2.7 | 0 | 0 | 195.66 KB |
| #6415 | AllCycleSimpleBody |
net472 | 270μs | 349ns | 1.26μs | 35.7 | 2.01 | 0 | 225.16 KB |
| #6415 | AllCycleMoreComplexBody |
net6.0 | 210μs | 69.2ns | 250ns | 2.64 | 0 | 0 | 191.88 KB |
| #6415 | AllCycleMoreComplexBody |
netcoreapp3.1 | 318μs | 112ns | 403ns | 2.71 | 0 | 0 | 199.07 KB |
| #6415 | AllCycleMoreComplexBody |
net472 | 276μs | 441ns | 1.71μs | 36.3 | 1.95 | 0 | 228.67 KB |
| #6415 | ObjectExtractorSimpleBody |
net6.0 | 137ns | 0.122ns | 0.473ns | 0.00395 | 0 | 0 | 280 B |
| #6415 | ObjectExtractorSimpleBody |
netcoreapp3.1 | 226ns | 0.226ns | 0.874ns | 0.00373 | 0 | 0 | 272 B |
| #6415 | ObjectExtractorSimpleBody |
net472 | 168ns | 0.146ns | 0.567ns | 0.0446 | 0 | 0 | 281 B |
| #6415 | ObjectExtractorMoreComplexBody |
net6.0 | 2.98μs | 1.38ns | 4.97ns | 0.0538 | 0 | 0 | 3.78 KB |
| #6415 | ObjectExtractorMoreComplexBody |
netcoreapp3.1 | 3.78μs | 3.31ns | 12.4ns | 0.0511 | 0 | 0 | 3.69 KB |
| #6415 | ObjectExtractorMoreComplexBody |
net472 | 3.6μs | 5.17ns | 20ns | 0.603 | 0.00535 | 0 | 3.8 KB |
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EncodeArgs |
net6.0 | 38.2μs | 21.5ns | 80.4ns | 0.454 | 0 | 0 | 32.4 KB |
| master | EncodeArgs |
netcoreapp3.1 | 55μs | 31.3ns | 121ns | 0.436 | 0 | 0 | 32.4 KB |
| master | EncodeArgs |
net472 | 67.4μs | 104ns | 401ns | 5.16 | 0.067 | 0 | 32.5 KB |
| master | EncodeLegacyArgs |
net6.0 | 78.2μs | 421ns | 2.35μs | 0 | 0 | 0 | 2.14 KB |
| master | EncodeLegacyArgs |
netcoreapp3.1 | 105μs | 338ns | 1.31μs | 0 | 0 | 0 | 2.14 KB |
| master | EncodeLegacyArgs |
net472 | 157μs | 86.2ns | 334ns | 0.313 | 0 | 0 | 2.15 KB |
| #6415 | EncodeArgs |
net6.0 | 36.2μs | 14.2ns | 49.3ns | 0.451 | 0 | 0 | 32.4 KB |
| #6415 | EncodeArgs |
netcoreapp3.1 | 53.9μs | 33.3ns | 129ns | 0.429 | 0 | 0 | 32.4 KB |
| #6415 | EncodeArgs |
net472 | 65.7μs | 93.4ns | 350ns | 5.15 | 0.0656 | 0 | 32.5 KB |
| #6415 | EncodeLegacyArgs |
net6.0 | 75.3μs | 432ns | 3.17μs | 0 | 0 | 0 | 2.14 KB |
| #6415 | EncodeLegacyArgs |
netcoreapp3.1 | 107μs | 64.6ns | 233ns | 0 | 0 | 0 | 2.15 KB |
| #6415 | EncodeLegacyArgs |
net472 | 151μs | 90.7ns | 339ns | 0.302 | 0 | 0 | 2.15 KB |
Benchmarks.Trace.Asm.AppSecWafBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunWafRealisticBenchmark |
net6.0 | 182μs | 202ns | 756ns | 0 | 0 | 0 | 2.44 KB |
| master | RunWafRealisticBenchmark |
netcoreapp3.1 | 195μs | 305ns | 1.18μs | 0 | 0 | 0 | 2.39 KB |
| master | RunWafRealisticBenchmark |
net472 | 208μs | 77.7ns | 301ns | 0.311 | 0 | 0 | 2.46 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net6.0 | 123μs | 58.1ns | 217ns | 0 | 0 | 0 | 1.47 KB |
| master | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 131μs | 97.7ns | 352ns | 0 | 0 | 0 | 1.46 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net472 | 140μs | 46.5ns | 174ns | 0.209 | 0 | 0 | 1.49 KB |
| #6415 | RunWafRealisticBenchmark |
net6.0 | 183μs | 200ns | 774ns | 0 | 0 | 0 | 2.44 KB |
| #6415 | RunWafRealisticBenchmark |
netcoreapp3.1 | 194μs | 281ns | 1.09μs | 0 | 0 | 0 | 2.39 KB |
| #6415 | RunWafRealisticBenchmark |
net472 | 207μs | 68.6ns | 247ns | 0.31 | 0 | 0 | 2.46 KB |
| #6415 | RunWafRealisticBenchmarkWithAttack |
net6.0 | 123μs | 73.3ns | 284ns | 0 | 0 | 0 | 1.47 KB |
| #6415 | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 130μs | 227ns | 880ns | 0 | 0 | 0 | 1.46 KB |
| #6415 | RunWafRealisticBenchmarkWithAttack |
net472 | 139μs | 71.6ns | 268ns | 0.21 | 0 | 0 | 1.49 KB |
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #6415
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1
253.05 KB
254.82 KB
1.78 KB
0.70%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472
59.07 KB
59.42 KB
344 B
0.58%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 253.05 KB | 254.82 KB | 1.78 KB | 0.70% |
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net472 | 59.07 KB | 59.42 KB | 344 B | 0.58% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StringConcatBenchmark |
net6.0 | 58.3μs | 733ns | 7.22μs | 0 | 0 | 0 | 43.44 KB |
| master | StringConcatBenchmark |
netcoreapp3.1 | 58.8μs | 680ns | 6.73μs | 0 | 0 | 0 | 42.64 KB |
| master | StringConcatBenchmark |
net472 | 38.6μs | 170ns | 638ns | 0 | 0 | 0 | 59.07 KB |
| master | StringConcatAspectBenchmark |
net6.0 | 291μs | 5.35μs | 53μs | 0 | 0 | 0 | 255.25 KB |
| master | StringConcatAspectBenchmark |
netcoreapp3.1 | 319μs | 5.88μs | 57.9μs | 0 | 0 | 0 | 253.05 KB |
| master | StringConcatAspectBenchmark |
net472 | 298μs | 7.72μs | 75.3μs | 0 | 0 | 0 | 278.53 KB |
| #6415 | StringConcatBenchmark |
net6.0 | 51.8μs | 211ns | 789ns | 0 | 0 | 0 | 43.44 KB |
| #6415 | StringConcatBenchmark |
netcoreapp3.1 | 53.3μs | 220ns | 761ns | 0 | 0 | 0 | 42.64 KB |
| #6415 | StringConcatBenchmark |
net472 | 37.4μs | 92.1ns | 345ns | 0 | 0 | 0 | 59.42 KB |
| #6415 | StringConcatAspectBenchmark |
net6.0 | 313μs | 1.77μs | 11.9μs | 0 | 0 | 0 | 254.33 KB |
| #6415 | StringConcatAspectBenchmark |
netcoreapp3.1 | 359μs | 2.01μs | 13.6μs | 0 | 0 | 0 | 254.82 KB |
| #6415 | StringConcatAspectBenchmark |
net472 | 278μs | 5.46μs | 52.4μs | 0 | 0 | 0 | 278.53 KB |
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 (6415) (11.036M) : 0, 11036456
master (11.199M) : 0, 11199251
benchmarks/2.9.0 (11.033M) : 0, 11032866
section Automatic
This PR (6415) (7.116M) : 0, 7115869
master (7.234M) : 0, 7234132
benchmarks/2.9.0 (7.786M) : 0, 7785853
section Trace stats
master (7.559M) : 0, 7558761
section Manual
master (11.028M) : 0, 11028498
section Manual + Automatic
This PR (6415) (6.621M) : 0, 6620906
master (6.711M) : 0, 6710751
section DD_TRACE_ENABLED=0
master (10.230M) : 0, 10230285
gantt
title Throughput Linux arm64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (6415) (9.739M) : 0, 9738910
master (9.417M) : 0, 9416704
benchmarks/2.9.0 (9.495M) : 0, 9494821
section Automatic
This PR (6415) (6.518M) : 0, 6517702
master (6.437M) : 0, 6437027
section Trace stats
master (6.619M) : 0, 6618620
section Manual
master (9.205M) : 0, 9205053
section Manual + Automatic
This PR (6415) (5.863M) : 0, 5863149
master (5.852M) : 0, 5852490
section DD_TRACE_ENABLED=0
master (8.872M) : 0, 8871934
gantt
title Throughput Windows x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (6415) (9.661M) : 0, 9661373
master (9.905M) : 0, 9904680
benchmarks/2.9.0 (10.020M) : 0, 10019592
section Automatic
This PR (6415) (6.396M) : 0, 6396225
master (6.337M) : 0, 6337163
benchmarks/2.9.0 (7.255M) : 0, 7255257
section Trace stats
master (6.938M) : 0, 6938182
section Manual
master (9.708M) : 0, 9708036
section Manual + Automatic
This PR (6415) (5.788M) : 0, 5788117
master (5.665M) : 0, 5664554
section DD_TRACE_ENABLED=0
master (9.111M) : 0, 9111450
|
Benchmarks Report for tracer 🐌Benchmarks for #6415 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Faster 🎉 Same allocations ✔️
|
| Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.ILoggerBenchmark.EnrichedLog‑net6.0 | 1.233 | 1,851.84 | 1,501.66 |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EnrichedLog |
net6.0 | 1.88μs | 16.9ns | 164ns | 0.0228 | 0 | 0 | 1.64 KB |
| master | EnrichedLog |
netcoreapp3.1 | 2.53μs | 1.34ns | 5.01ns | 0.0214 | 0 | 0 | 1.64 KB |
| master | EnrichedLog |
net472 | 2.66μs | 0.796ns | 3.08ns | 0.25 | 0 | 0 | 1.57 KB |
| #6415 | EnrichedLog |
net6.0 | 1.5μs | 0.615ns | 2.3ns | 0.0232 | 0 | 0 | 1.64 KB |
| #6415 | EnrichedLog |
netcoreapp3.1 | 2.28μs | 0.673ns | 2.43ns | 0.0217 | 0 | 0 | 1.64 KB |
| #6415 | EnrichedLog |
net472 | 2.67μs | 2.01ns | 7.78ns | 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 | 120μs | 160ns | 619ns | 0.0602 | 0 | 0 | 4.28 KB |
| master | EnrichedLog |
netcoreapp3.1 | 123μs | 191ns | 739ns | 0.0618 | 0 | 0 | 4.28 KB |
| master | EnrichedLog |
net472 | 153μs | 193ns | 746ns | 0.679 | 0.226 | 0 | 4.46 KB |
| #6415 | EnrichedLog |
net6.0 | 121μs | 71.3ns | 257ns | 0.0615 | 0 | 0 | 4.28 KB |
| #6415 | EnrichedLog |
netcoreapp3.1 | 124μs | 149ns | 579ns | 0 | 0 | 0 | 4.28 KB |
| #6415 | EnrichedLog |
net472 | 152μs | 111ns | 431ns | 0.686 | 0.229 | 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.08μs | 4.61ns | 17.9ns | 0.031 | 0 | 0 | 2.2 KB |
| master | EnrichedLog |
netcoreapp3.1 | 4.16μs | 2.04ns | 7.92ns | 0.0292 | 0 | 0 | 2.2 KB |
| master | EnrichedLog |
net472 | 4.87μs | 1.48ns | 5.74ns | 0.32 | 0 | 0 | 2.02 KB |
| #6415 | EnrichedLog |
net6.0 | 3μs | 1.38ns | 5.16ns | 0.03 | 0 | 0 | 2.2 KB |
| #6415 | EnrichedLog |
netcoreapp3.1 | 4.14μs | 0.94ns | 3.64ns | 0.029 | 0 | 0 | 2.2 KB |
| #6415 | EnrichedLog |
net472 | 4.92μs | 1.55ns | 5.99ns | 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.4μs | 0.984ns | 3.81ns | 0.0161 | 0 | 0 | 1.14 KB |
| master | SendReceive |
netcoreapp3.1 | 1.8μs | 0.914ns | 3.42ns | 0.0153 | 0 | 0 | 1.14 KB |
| master | SendReceive |
net472 | 2.11μs | 3.07ns | 11.9ns | 0.183 | 0 | 0 | 1.16 KB |
| #6415 | SendReceive |
net6.0 | 1.35μs | 1.02ns | 3.95ns | 0.0162 | 0 | 0 | 1.14 KB |
| #6415 | SendReceive |
netcoreapp3.1 | 1.82μs | 0.973ns | 3.77ns | 0.0155 | 0 | 0 | 1.14 KB |
| #6415 | SendReceive |
net472 | 2.16μs | 10.4ns | 40.1ns | 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.92μs | 0.793ns | 3.07ns | 0.0219 | 0 | 0 | 1.6 KB |
| master | EnrichedLog |
netcoreapp3.1 | 3.84μs | 1.59ns | 6.15ns | 0.0211 | 0 | 0 | 1.65 KB |
| master | EnrichedLog |
net472 | 4.47μs | 2.23ns | 8.62ns | 0.322 | 0 | 0 | 2.04 KB |
| #6415 | EnrichedLog |
net6.0 | 2.72μs | 0.921ns | 3.57ns | 0.0217 | 0 | 0 | 1.6 KB |
| #6415 | EnrichedLog |
netcoreapp3.1 | 3.93μs | 2.3ns | 8.92ns | 0.0215 | 0 | 0 | 1.65 KB |
| #6415 | EnrichedLog |
net472 | 4.6μs | 3.93ns | 15.2ns | 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 | 403ns | 0.229ns | 0.886ns | 0.00816 | 0 | 0 | 576 B |
| master | StartFinishSpan |
netcoreapp3.1 | 613ns | 0.503ns | 1.95ns | 0.00768 | 0 | 0 | 576 B |
| master | StartFinishSpan |
net472 | 687ns | 0.639ns | 2.48ns | 0.0918 | 0 | 0 | 578 B |
| master | StartFinishScope |
net6.0 | 511ns | 0.202ns | 0.783ns | 0.00972 | 0 | 0 | 696 B |
| master | StartFinishScope |
netcoreapp3.1 | 732ns | 1.57ns | 6.1ns | 0.00957 | 0 | 0 | 696 B |
| master | StartFinishScope |
net472 | 923ns | 0.528ns | 2.05ns | 0.105 | 0 | 0 | 658 B |
| #6415 | StartFinishSpan |
net6.0 | 402ns | 0.511ns | 1.98ns | 0.00809 | 0 | 0 | 576 B |
| #6415 | StartFinishSpan |
netcoreapp3.1 | 559ns | 0.223ns | 0.834ns | 0.00772 | 0 | 0 | 576 B |
| #6415 | StartFinishSpan |
net472 | 666ns | 0.415ns | 1.61ns | 0.0918 | 0 | 0 | 578 B |
| #6415 | StartFinishScope |
net6.0 | 508ns | 0.336ns | 1.3ns | 0.00987 | 0 | 0 | 696 B |
| #6415 | StartFinishScope |
netcoreapp3.1 | 796ns | 0.969ns | 3.75ns | 0.00959 | 0 | 0 | 696 B |
| #6415 | StartFinishScope |
net472 | 844ns | 0.271ns | 1.05ns | 0.104 | 0 | 0 | 658 B |
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunOnMethodBegin |
net6.0 | 677ns | 0.495ns | 1.92ns | 0.00983 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
netcoreapp3.1 | 972ns | 0.564ns | 2.11ns | 0.00925 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
net472 | 1.05μs | 0.494ns | 1.85ns | 0.104 | 0 | 0 | 658 B |
| #6415 | RunOnMethodBegin |
net6.0 | 715ns | 0.569ns | 2.2ns | 0.00965 | 0 | 0 | 696 B |
| #6415 | RunOnMethodBegin |
netcoreapp3.1 | 919ns | 4.71ns | 22.6ns | 0.00943 | 0 | 0 | 696 B |
| #6415 | RunOnMethodBegin |
net472 | 1.14μs | 1.01ns | 3.92ns | 0.105 | 0 | 0 | 658 B |
…tegrationSettings` (#6405) ## Summary of changes Merge `IntegrationSettings` with `ImmutableIntegrationSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Moved additional logic (handling `DisabledNames` that was previously inside `ImmutableIntegrationSettings` into `IntegrationSettings` - Replace all usages of `Immutable*` with `IntegrationSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization - Reorder initialization of `DisabledIntegrationNames` in `TracerSettings` so that it can be used in the `IntegrationSettings` constructor ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 👈 This PR - #6408 - #6415
5b80296 to
ced5042
Compare
63b55e2 to
21937c4
Compare
…terSettings` (#6408) ## Summary of changes Merge `ExporterSettings` with `ImmutableExporterSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Made the properties in `ExporterSettings` get-only. This required quite a lot of work because we were doing a lot of mutating of the settings in the "helper" functions. - I only _lightly_ refactored those methods (as much as possible) to avoid setting the properties in the functions and instead returning the details to set later. - These are prime candidates for some _much_ heavier refactoring later, but I didn't want to get bogged down with that in this PR - Replace all usages of `Immutable*` with `ExporterSettings` - Replace usages of `AgentUriInternal` with `AgentUri` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 👈 This PR - #6415
Make dictionaries immutable where possible Lots more scope to improve this later
…cerSettings to pass in the values instead
…tableTracerSettings to TracerSettings
21937c4 to
142488c
Compare
robertpi
left a comment
There was a problem hiding this comment.
ASM parts seem good - they are trivial.
More generally LGTM.
…utableDirectLogSubmissionSettings` (#6400) ## Summary of changes Merge `DirectLogSubmissionSettings` with `ImmutableDirectLogSubmissionSettings` ## Reason for change There was never really a good reason for having these as separate types. It was primarily to make testing a little easier and to mirror the `TracerSettings`/`ImmutableTracerSettings` dichotomy, but as that's going away, this is just unnecessary complexity ## Implementation details - Moved the additional logic that was previously inside `ImmutableDirectLogSubmissionSettings` into `DirectLogSubmissionSettings` - Renamed the `DirectLogSubmissionSettings` properties to match the "Immutable" version (and remove the unnecessary prefix) - Replace all usages of `Immutable*` with `DirectLogSubmissionSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization (using `IConfigurationSource`) ## Test coverage Essentially the same. I removed/tweaked some tests that are no longer relevant ## Other details Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 👈 This PR - #6405 - #6408 - #6415
…tegrationSettings` (#6405) ## Summary of changes Merge `IntegrationSettings` with `ImmutableIntegrationSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Moved additional logic (handling `DisabledNames` that was previously inside `ImmutableIntegrationSettings` into `IntegrationSettings` - Replace all usages of `Immutable*` with `IntegrationSettings` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization - Reorder initialization of `DisabledIntegrationNames` in `TracerSettings` so that it can be used in the `IntegrationSettings` constructor ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 👈 This PR - #6408 - #6415
…terSettings` (#6408) ## Summary of changes Merge `ExporterSettings` with `ImmutableExporterSettings` ## Reason for change This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. ## Implementation details - Made the properties in `ExporterSettings` get-only. This required quite a lot of work because we were doing a lot of mutating of the settings in the "helper" functions. - I only _lightly_ refactored those methods (as much as possible) to avoid setting the properties in the functions and instead returning the details to set later. - These are prime candidates for some _much_ heavier refactoring later, but I didn't want to get bogged down with that in this PR - Replace all usages of `Immutable*` with `ExporterSettings` - Replace usages of `AgentUriInternal` with `AgentUri` - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization ## Test coverage All covered by existing details ## Other details Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 👈 This PR - #6415
…ettings` (#6415) Merge `TracerSettings` with `ImmutableTracerSettings` This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types. - Make the properties in `TracerSettings` get-only. - Make the collections in `TracerSettings` readonly. - Move logic that used to be in the constructor of `ImmutableTracerSettings` into `TracerSettings` - e.g. Service/Version/Env were being changed based on DD_TAGS values. Moved that to TracerSettings and (importantly) added missing telemetry recording of these values. - Added missing recording of _effective_ `DisabledInstegrations` - Moving this logic caused some _tests_ to be broken (checking default values). Updated the expected values of those tests in a single - Replace all usages of `ImmutableTracerSettings` with `TracerSettings` - Move `ITracer` to Datadog.Trace.Manual - It's only used there, and references the manual-version of `ImmutableTracerSettings` which we _want_ to keep. - Move `Immutable*Tests` into appropriate file and tweak - Replace mutations with initialization in tests All covered by existing tests (I hope) 🤞 There's still a _lot_ of scope to improve this Part of Stack - #6370 - #6376 - #6385 - #6386 - #6397 - #6399 - #6400 - #6405 - #6408 - #6415 👈 This PR
Summary of changes
Merge
TracerSettingswithImmutableTracerSettingsReason for change
This stack of PRs is about doing one-shot configuration instead of mutation. We never mutate these in the tracer after creation, so there's no need for the separate types.
Implementation details
TracerSettingsget-only.TracerSettingsreadonly.ImmutableTracerSettingsintoTracerSettingsDisabledInstegrationsImmutableTracerSettingswithTracerSettingsITracerto Datadog.Trace.ManualImmutableTracerSettingswhich we want to keep.Immutable*Testsinto appropriate file and tweakTest coverage
All covered by existing tests (I hope) 🤞
Other details
There's still a lot of scope to improve this
Part of Stack
PublicApiTests.PublicApiHasNotChangedfor Datadog.Trace.dll #6385IConfigurationSource#6386IConfigurationSource#6397TracerSettingsusing a configuration source instead of mutating #6399DirectLogSubmissionSettingsproperly immutable and removeImmutableDirectLogSubmissionSettings#6400IntegrationSettingsproperly immutable and removeImmutableIntegrationSettings#6405ExporterSettingsproperly immutable and removeImmutableExporterSettings#6408TracerSettingsproperly immutable and removeImmutableTracerSettings#6415 👈 This PR