Skip to content

Make TracerSettings properly immutable and remove ImmutableTracerSettings#6415

Merged
andrewlock merged 7 commits into
masterfrom
andrew/make-tracer-settings-immutable
Dec 12, 2024
Merged

Make TracerSettings properly immutable and remove ImmutableTracerSettings#6415
andrewlock merged 7 commits into
masterfrom
andrew/make-tracer-settings-immutable

Conversation

@andrewlock

@andrewlock andrewlock commented Dec 10, 2024

Copy link
Copy Markdown
Member

Summary of changes

Merge TracerSettings with ImmutableTracerSettings

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

  • 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

Test coverage

All covered by existing tests (I hope) 🤞

Other details

There's still a lot of scope to improve this

Part of Stack

andrewlock added a commit that referenced this pull request Dec 10, 2024
…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
@andrewlock
andrewlock force-pushed the andrew/make-exporter-settings-immutable branch from c4cd7f8 to 248d0d8 Compare December 10, 2024 14:20
@andrewlock
andrewlock force-pushed the andrew/make-tracer-settings-immutable branch from f3258ab to 10ebd53 Compare December 10, 2024 14:20
@datadog-ddstaging

datadog-ddstaging Bot commented Dec 10, 2024

Copy link
Copy Markdown

Datadog Report

Branch report: andrew/make-tracer-settings-immutable
Commit report: 142488c
Test service: dd-trace-dotnet

✅ 0 Failed, 465592 Passed, 3681 Skipped, 33h 13m 49.74s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

  • DetectContinousProfilerState - Datadog.Trace.Tools.dd_dotnet.ArtifactTests.Checks.ProcessBasicChecksTests - Last Failure

    Expand for error
     Timeout when waiting for the target process to start
    

@andrewlock

andrewlock commented Dec 10, 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).

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,

Loading
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,

Loading
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,

Loading
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,

Loading
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,

Loading
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,

Loading

@andrewlock
andrewlock force-pushed the andrew/make-exporter-settings-immutable branch from 248d0d8 to c039b45 Compare December 10, 2024 15:46
@andrewlock
andrewlock force-pushed the andrew/make-tracer-settings-immutable branch from 10ebd53 to f68edfc Compare December 10, 2024 15:46
@andrewlock
andrewlock force-pushed the andrew/make-exporter-settings-immutable branch from c039b45 to 5b80296 Compare December 10, 2024 15:55
@andrewlock
andrewlock force-pushed the andrew/make-tracer-settings-immutable branch from f68edfc to 63b55e2 Compare December 10, 2024 15:55
@andrewlock

andrewlock commented Dec 10, 2024

Copy link
Copy Markdown
Member Author

Benchmarks Report for appsec 🐌

Benchmarks for #6415 compared to master:

  • 2 benchmarks are faster, with geometric mean 1.220
  • 6 benchmarks have fewer allocations
  • 2 benchmarks have more 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.Asm.AppSecBodyBenchmark - Faster 🎉 Fewer allocations 🎉

Faster 🎉 in #6415

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

Fewer allocations 🎉 in #6415

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%

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

@andrewlock

andrewlock commented Dec 10, 2024

Copy link
Copy Markdown
Member Author

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

Loading
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

Loading
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

Loading

@andrewlock

andrewlock commented Dec 11, 2024

Copy link
Copy Markdown
Member Author

Benchmarks Report for tracer 🐌

Benchmarks for #6415 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.233
  • 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 8μs 41.9ns 258ns 0.0148 0.0037 0 5.62 KB
master StartStopWithChild netcoreapp3.1 10.4μs 52.8ns 259ns 0.021 0.0105 0 5.8 KB
master StartStopWithChild net472 16.3μs 41.8ns 162ns 1.04 0.308 0.0998 6.2 KB
#6415 StartStopWithChild net6.0 8.08μs 40.5ns 236ns 0.0116 0.00385 0 5.61 KB
#6415 StartStopWithChild netcoreapp3.1 10.4μs 41.7ns 162ns 0.0204 0.0102 0 5.8 KB
#6415 StartStopWithChild net472 16.5μs 52.4ns 203ns 1.06 0.329 0.107 6.22 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 501μs 258ns 964ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 647μs 356ns 1.38μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 842μs 460ns 1.78μs 0.419 0 0 3.3 KB
#6415 WriteAndFlushEnrichedTraces net6.0 468μs 389ns 1.51μs 0 0 0 2.7 KB
#6415 WriteAndFlushEnrichedTraces netcoreapp3.1 651μs 554ns 2.15μs 0 0 0 2.7 KB
#6415 WriteAndFlushEnrichedTraces net472 862μs 478ns 1.79μs 0.428 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 152μs 879ns 7.71μs 0.144 0 0 14.47 KB
master SendRequest netcoreapp3.1 173μs 1.19μs 11.8μs 0.16 0 0 17.27 KB
master SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#6415 SendRequest net6.0 154μs 1.03μs 10.2μs 0.141 0 0 14.47 KB
#6415 SendRequest netcoreapp3.1 174μs 1.23μs 12μs 0.199 0 0 17.27 KB
#6415 SendRequest net472 0.000255ns 0.000178ns 0.000665ns 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 568μs 3.15μs 18.6μs 0.566 0 0 41.66 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 676μs 3.77μs 23.2μs 0.326 0 0 41.81 KB
master WriteAndFlushEnrichedTraces net472 857μs 3.67μs 13.7μs 8.22 2.47 0.411 53.35 KB
#6415 WriteAndFlushEnrichedTraces net6.0 559μs 1.62μs 6.08μs 0.543 0 0 41.85 KB
#6415 WriteAndFlushEnrichedTraces netcoreapp3.1 723μs 4.07μs 27.3μs 0.368 0 0 41.65 KB
#6415 WriteAndFlushEnrichedTraces net472 834μs 2.96μs 11.5μs 8.33 2.5 0.417 53.3 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.32μs 2.24ns 8.68ns 0.0145 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.84μs 1.79ns 6.94ns 0.0137 0 0 1.02 KB
master ExecuteNonQuery net472 2.12μs 2.24ns 8.67ns 0.156 0.00106 0 987 B
#6415 ExecuteNonQuery net6.0 1.32μs 1.83ns 7.09ns 0.0145 0 0 1.02 KB
#6415 ExecuteNonQuery netcoreapp3.1 1.79μs 1.27ns 4.77ns 0.0134 0 0 1.02 KB
#6415 ExecuteNonQuery net472 2.06μs 1.37ns 5.29ns 0.156 0.00103 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.3μs 0.487ns 1.82ns 0.0137 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.53μs 0.731ns 2.63ns 0.0132 0 0 976 B
master CallElasticsearch net472 2.64μs 2.24ns 8.68ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.24μs 0.694ns 2.6ns 0.0131 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.62μs 0.754ns 2.72ns 0.0139 0 0 1.02 KB
master CallElasticsearchAsync net472 2.64μs 1.87ns 7.01ns 0.167 0 0 1.05 KB
#6415 CallElasticsearch net6.0 1.2μs 0.776ns 3.01ns 0.0138 0 0 976 B
#6415 CallElasticsearch netcoreapp3.1 1.58μs 0.967ns 3.74ns 0.0134 0 0 976 B
#6415 CallElasticsearch net472 2.61μs 2.77ns 10.7ns 0.157 0 0 995 B
#6415 CallElasticsearchAsync net6.0 1.25μs 0.522ns 1.81ns 0.0131 0 0 952 B
#6415 CallElasticsearchAsync netcoreapp3.1 1.67μs 1.18ns 4.41ns 0.0134 0 0 1.02 KB
#6415 CallElasticsearchAsync net472 2.69μs 1.37ns 5.14ns 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.36μs 5.84ns 50.5ns 0.0136 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.73μs 0.581ns 2.09ns 0.0121 0 0 952 B
master ExecuteAsync net472 1.83μs 0.317ns 1.19ns 0.145 0 0 915 B
#6415 ExecuteAsync net6.0 1.26μs 1ns 3.75ns 0.0134 0 0 952 B
#6415 ExecuteAsync netcoreapp3.1 1.7μs 1.28ns 4.95ns 0.0127 0 0 952 B
#6415 ExecuteAsync net472 1.8μs 0.426ns 1.59ns 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.37μs 2.18ns 8.46ns 0.0328 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.17μs 1.63ns 5.87ns 0.0388 0 0 2.85 KB
master SendAsync net472 7.43μs 3.51ns 13.1ns 0.494 0 0 3.12 KB
#6415 SendAsync net6.0 4.4μs 1.5ns 5.63ns 0.0308 0 0 2.31 KB
#6415 SendAsync netcoreapp3.1 5.27μs 3.34ns 12.9ns 0.037 0 0 2.85 KB
#6415 SendAsync net472 7.37μs 1.8ns 6.97ns 0.494 0 0 3.12 KB
Benchmarks.Trace.ILoggerBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6415

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

andrewlock added a commit that referenced this pull request Dec 11, 2024
…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
@andrewlock
andrewlock force-pushed the andrew/make-exporter-settings-immutable branch from 5b80296 to ced5042 Compare December 11, 2024 12:43
@andrewlock
andrewlock force-pushed the andrew/make-tracer-settings-immutable branch from 63b55e2 to 21937c4 Compare December 11, 2024 12:43
andrewlock added a commit that referenced this pull request Dec 11, 2024
…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
Base automatically changed from andrew/make-exporter-settings-immutable to master December 11, 2024 17:03
@andrewlock
andrewlock force-pushed the andrew/make-tracer-settings-immutable branch from 21937c4 to 142488c Compare December 11, 2024 17:04

@robertpi robertpi left a comment

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.

ASM parts seem good - they are trivial.
More generally LGTM.

@andrewlock
andrewlock merged commit b13e7b8 into master Dec 12, 2024
@andrewlock
andrewlock deleted the andrew/make-tracer-settings-immutable branch December 12, 2024 12:19
@github-actions github-actions Bot added this to the vNext-v3 milestone Dec 12, 2024
veerbia pushed a commit that referenced this pull request Dec 16, 2024
…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
veerbia pushed a commit that referenced this pull request Dec 16, 2024
…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
veerbia pushed a commit that referenced this pull request Dec 16, 2024
…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
veerbia pushed a commit that referenced this pull request Dec 16, 2024
…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
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) type:refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants