Skip to content

Use LifeTimeManager for LiveDebugger shutdown events#3200

Merged
shurivich merged 2 commits into
masterfrom
shurivich/use_lifetimemanager
Sep 14, 2022
Merged

Use LifeTimeManager for LiveDebugger shutdown events#3200
shurivich merged 2 commits into
masterfrom
shurivich/use_lifetimemanager

Conversation

@shurivich

@shurivich shurivich commented Sep 12, 2022 •
edited by OmerRaviv
Loading

Copy link
Copy Markdown
Contributor

Summary of changes

Use LifeTimeManager.AddShutdownTask() method, instead of custom subscritpion to AppDomain.CurrentDomain.DomainUnload event.
Run each dispose method as separate task.

Reason for change

Use standard way for register shutdown events.

@shurivich shurivich self-assigned this Sep 12, 2022
@shurivich
shurivich requested a review from a team as a code owner September 12, 2022 07:25
@github-actions github-actions Bot added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Sep 12, 2022
@shurivich shurivich removed the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Sep 12, 2022
@andrewlock

This comment has been minimized.

Sign in to view
andrewlock
andrewlock reviewed Sep 12, 2022
View reviewed changes
Comment thread tracer/src/Datadog.Trace/Debugger/LiveDebugger.cs
andrewlock
andrewlock reviewed Sep 12, 2022
View reviewed changes
Comment thread tracer/src/Datadog.Trace/Debugger/LiveDebugger.cs
@OmerRaviv OmerRaviv changed the title Use LiveTimeManager for LiveDebugger shutdown events Use LifeTimeManager for LiveDebugger shutdown events Sep 12, 2022
@andrewlock

andrewlock commented Sep 13, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #3200 compared to master:

  • All benchmarks have the same speed
  • 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.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net472 717μs 607ns 2.35μs 0.357 0 0 3.18 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 471μs 645ns 2.5μs 0 0 0 2.59 KB
#3200 WriteAndFlushEnrichedTraces net472 715μs 744ns 2.88μs 0.351 0 0 3.18 KB
#3200 WriteAndFlushEnrichedTraces netcoreapp3.1 449μs 745ns 2.79μs 0 0 0 2.58 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net472 201ns 0.302ns 1.17ns 0.0675 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 236ns 0.297ns 1.15ns 0.0058 0 0 424 B
master AllCycleMoreComplexBody net472 190ns 0.237ns 0.885ns 0.0637 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 232ns 0.195ns 0.756ns 0.00549 0 0 400 B
master BodyExtractorSimpleBody net472 276ns 0.472ns 1.76ns 0.0574 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 235ns 0.241ns 0.933ns 0.00367 0 0 272 B
master BodyExtractorMoreComplexBody net472 14.7μs 4.28ns 14.8ns 1.21 0.0148 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 12μs 12.2ns 47.3ns 0.0899 0 0 6.75 KB
#3200 AllCycleSimpleBody net472 207ns 0.17ns 0.614ns 0.0675 0 0 425 B
#3200 AllCycleSimpleBody netcoreapp3.1 237ns 0.352ns 1.36ns 0.00571 0 0 424 B
#3200 AllCycleMoreComplexBody net472 189ns 0.249ns 0.966ns 0.0637 0 0 401 B
#3200 AllCycleMoreComplexBody netcoreapp3.1 234ns 0.365ns 1.41ns 0.00544 0 0 400 B
#3200 BodyExtractorSimpleBody net472 280ns 0.27ns 1.01ns 0.0574 0 0 361 B
#3200 BodyExtractorSimpleBody netcoreapp3.1 231ns 0.244ns 0.944ns 0.00373 0 0 272 B
#3200 BodyExtractorMoreComplexBody net472 14.6μs 9.9ns 37ns 1.2 0.0146 0 7.62 KB
#3200 BodyExtractorMoreComplexBody netcoreapp3.1 12.3μs 14ns 52.5ns 0.086 0 0 6.75 KB
Benchmarks.Trace.AspNetCoreBenchmark - Unknown 🤷 Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
master SendRequest netcoreapp3.1 181μs 259ns 1μs 0.181 0 0 20.57 KB
#3200 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3200 SendRequest netcoreapp3.1 183μs 390ns 1.51μs 0.183 0 0 20.57 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 net472 1.89μs 2.64ns 9.87ns 0.151 0 0 947 B
master ExecuteNonQuery netcoreapp3.1 1.4μs 0.769ns 2.88ns 0.0121 0 0 936 B
#3200 ExecuteNonQuery net472 1.72μs 3.25ns 12.6ns 0.15 0.000869 0 947 B
#3200 ExecuteNonQuery netcoreapp3.1 1.39μs 1.53ns 5.93ns 0.0124 0 0 936 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 net472 2.54μs 1.36ns 5.28ns 0.183 0 0 1.16 KB
master CallElasticsearch netcoreapp3.1 1.5μs 0.587ns 2.12ns 0.015 0 0 1.1 KB
master CallElasticsearchAsync net472 2.77μs 2.51ns 9.71ns 0.205 0 0 1.29 KB
master CallElasticsearchAsync netcoreapp3.1 1.62μs 0.666ns 2.49ns 0.0162 0 0 1.22 KB
#3200 CallElasticsearch net472 2.66μs 1.97ns 7.35ns 0.184 0 0 1.16 KB
#3200 CallElasticsearch netcoreapp3.1 1.51μs 0.601ns 2.25ns 0.0151 0.000753 0 1.1 KB
#3200 CallElasticsearchAsync net472 2.79μs 1.82ns 7.04ns 0.205 0 0 1.29 KB
#3200 CallElasticsearchAsync netcoreapp3.1 1.62μs 0.488ns 1.89ns 0.0163 0 0 1.22 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 net472 2.71μs 5.27ns 20.4ns 0.225 0 0 1.42 KB
master ExecuteAsync netcoreapp3.1 1.67μs 3.89ns 15.1ns 0.0183 0 0 1.34 KB
#3200 ExecuteAsync net472 2.69μs 8.47ns 32.8ns 0.225 0 0 1.42 KB
#3200 ExecuteAsync netcoreapp3.1 1.74μs 2.74ns 10.2ns 0.0181 0 0 1.34 KB
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net472 5.68μs 13.4ns 52ns 0.438 0 0 2.77 KB
master SendAsync netcoreapp3.1 3.55μs 8.48ns 32.9ns 0.0359 0 0 2.6 KB
#3200 SendAsync net472 5.7μs 11.8ns 45.6ns 0.437 0 0 2.77 KB
#3200 SendAsync netcoreapp3.1 3.56μs 5.16ns 19.3ns 0.0353 0 0 2.6 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 net472 3.07μs 1.55ns 6ns 0.287 0 0 1.81 KB
master EnrichedLog netcoreapp3.1 2.56μs 1.37ns 5.32ns 0.0248 0 0 1.85 KB
#3200 EnrichedLog net472 3.09μs 1.4ns 5.25ns 0.287 0.00155 0 1.81 KB
#3200 EnrichedLog netcoreapp3.1 2.55μs 1.07ns 4.01ns 0.0255 0 0 1.85 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 net472 152μs 180ns 650ns 0.687 0.229 0 4.65 KB
master EnrichedLog netcoreapp3.1 118μs 223ns 864ns 0 0 0 4.49 KB
#3200 EnrichedLog net472 150μs 213ns 825ns 0.668 0.223 0 4.65 KB
#3200 EnrichedLog netcoreapp3.1 117μs 427ns 1.65μs 0.0589 0 0 4.49 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 net472 5.57μs 9.78ns 37.9ns 0.57 0.00275 0 3.59 KB
master EnrichedLog netcoreapp3.1 4.33μs 5.75ns 21.5ns 0.052 0 0 3.91 KB
#3200 EnrichedLog net472 5.68μs 13.5ns 52.3ns 0.569 0.0028 0 3.59 KB
#3200 EnrichedLog netcoreapp3.1 4.36μs 9.55ns 37ns 0.0528 0 0 3.91 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 net472 2.24μs 1.38ns 5.36ns 0.218 0 0 1.37 KB
master SendReceive netcoreapp3.1 1.76μs 1.03ns 3.7ns 0.0176 0 0 1.32 KB
#3200 SendReceive net472 2.23μs 0.96ns 3.32ns 0.217 0 0 1.37 KB
#3200 SendReceive netcoreapp3.1 1.83μs 1.48ns 5.71ns 0.0183 0 0 1.32 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 net472 5.16μs 3.56ns 13.8ns 0.353 0 0 2.23 KB
master EnrichedLog netcoreapp3.1 4.35μs 2.09ns 8.11ns 0.024 0 0 1.8 KB
#3200 EnrichedLog net472 5.16μs 2.34ns 9.08ns 0.353 0 0 2.23 KB
#3200 EnrichedLog netcoreapp3.1 4.4μs 1.29ns 4.64ns 0.0241 0 0 1.8 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 net472 1.19μs 0.775ns 3ns 0.128 0 0 810 B
master StartFinishSpan netcoreapp3.1 913ns 0.503ns 1.81ns 0.0099 0 0 760 B
master StartFinishScope net472 1.42μs 0.648ns 2.51ns 0.141 0 0 891 B
master StartFinishScope netcoreapp3.1 1.11μs 0.543ns 2.1ns 0.0116 0 0 880 B
#3200 StartFinishSpan net472 1.16μs 0.377ns 1.46ns 0.128 0 0 810 B
#3200 StartFinishSpan netcoreapp3.1 922ns 0.382ns 1.48ns 0.0102 0 0 760 B
#3200 StartFinishScope net472 1.43μs 0.59ns 2.21ns 0.142 0 0 891 B
#3200 StartFinishScope netcoreapp3.1 1.1μs 0.38ns 1.42ns 0.0116 0 0 880 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 net472 1.53μs 1.04ns 4.03ns 0.141 0 0 891 B
master RunOnMethodBegin netcoreapp3.1 1.21μs 0.373ns 1.44ns 0.012 0 0 880 B
#3200 RunOnMethodBegin net472 1.51μs 0.766ns 2.97ns 0.142 0 0 891 B
#3200 RunOnMethodBegin netcoreapp3.1 1.17μs 0.459ns 1.65ns 0.0118 0 0 880 B

@andrewlock

andrewlock commented Sep 13, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

✔️ Merging #3200 into master will will increase line coverage by 1%
✔️ Merging #3200 into master will will increase branch coverage by 1%
⛔ Merging #3200 into master will will increase complexity by 90

master #3200 Change
Lines 17696 / 24369 17935 / 24511
Lines % 73% 73% 1% ✔️
Branches 10398 / 14968 10578 / 15048
Branches % 69% 70% 1% ✔️
Complexity 16169 16259 90 ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #3200 Change
Lines % 73% 73% 1% ✔️
Branches % 69% 70% 1% ✔️
Complexity 16169 16259 90 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Ci.GitInfo -17% ⛔ -11% ⛔ 0 ✔️
Datadog.Trace.Agent.Transports.HttpStreamRequestFactory -12% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Sink.AgentBatchUploadApi -8% ⛔ 0% ✔️ 2 ⛔
Datadog.Trace.ClrProfiler.NativeMethods -6% ⛔ -2% ⚠️ 0 ✔️
Datadog.Trace.TracerManagerFactory 3% ✔️ 6% ✔️ 2 ⛔
Datadog.Trace.Logging.DatadogSerilogLogger 4% ✔️ 6% ✔️ 0 ✔️
Datadog.Trace.Propagators.SpanContextPropagator 8% ✔️ 6% ✔️ 0 ✔️
Datadog.Trace.Agent.StatsAggregator 11% ✔️ 15% ✔️ -2 ✔️
Datadog.Trace.Processors.ObfuscatorTraceProcessor 12% ✔️ 15% ✔️ 0 ✔️
Datadog.Trace.Telemetry.Transports.JsonTelemetryTransport 12% ✔️ 17% ✔️ 0 ✔️
...And 8 more

The following classes were added in #3200:

File Line coverage Branch coverage Complexity
Datadog.Trace.Agent.DiscoveryService.AgentConfiguration 100% 100% 1
Datadog.Trace.AgentProcessManagerLoader 0% 100% 1
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.EnvironmentVariablesScrubber 100% 100% 11
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.ProcessStartCommon 85% 80% 11
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.ProcessStartIntegration 80% 50% 3
...And 3 more

View the full reports for further details:

@shurivich
shurivich merged commit 0fb7aa1 into master Sep 14, 2022
@shurivich
shurivich deleted the shurivich/use_lifetimemanager branch September 14, 2022 06:47
@github-actions github-actions Bot added this to the vNext milestone Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@OmerRaviv OmerRaviv OmerRaviv approved these changes
@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes
@andrewlock andrewlock andrewlock approved these changes

Assignees

@shurivich shurivich

Labels

area:debugger

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

4 participants

@shurivich @andrewlock @OmerRaviv @zacharycmontoya