Remove ImmutableDebuggerSettings by shurivich · Pull Request #3166 · DataDog/dd-trace-dotnet

shurivich · 2022-09-04T15:07:29Z

Summary of changes

Remove ImmutableDebugger Settings

Reason for change

As part of moving LiveDebugger initialization to TracerManager we want to remove ImmutableDebuggerSettings

OmerRaviv

LGTM, thanks!

GreenMatan · 2022-09-05T09:03:58Z

+        var settings = DebuggerSettings.FromSource(new NameValueConfigurationSource(new()
        {
-            { ConfigurationKeys.Debugger.Enabled, "0" },
+            { ConfigurationKeys.Debugger.Enabled, "1" },


The test name says Debugger Disabled but now you are enabling the Live Debugger - Is it on purpose?

Yare right, copy..paste issues.
Fixed here bf5ae3e

andrewlock

LGTM! (though I agree with the existing comments)

andrewlock · 2022-09-05T10:44:36Z

Benchmarks Report 🐌

Benchmarks for #3166 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	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	711μs	737ns	2.86μs	0.357	3.18 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	460μs	294ns	1.14μs	0	2.59 KB
#3166	`WriteAndFlushEnrichedTraces`	net472	732μs	575ns	2.23μs	0.365	3.18 KB
#3166	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	454μs	1.64μs	6.35μs	0	2.58 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	183ns	0.258ns	1ns	0.0676	9.14E-05	425 B
master	`AllCycleSimpleBody`	netcoreapp3.1	236ns	0.322ns	1.25ns	0.00584	0	424 B
master	`AllCycleMoreComplexBody`	net472	183ns	0.11ns	0.424ns	0.0637	0	401 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	234ns	0.188ns	0.704ns	0.00549	0	400 B
master	`BodyExtractorSimpleBody`	net472	251ns	0.327ns	1.22ns	0.0573	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	222ns	0.399ns	1.44ns	0.00368	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	14.6μs	8.71ns	32.6ns	1.21	0.0217	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.4μs	24.2ns	90.5ns	0.0922	0	6.75 KB
#3166	`AllCycleSimpleBody`	net472	182ns	0.149ns	0.559ns	0.0675	0	425 B
#3166	`AllCycleSimpleBody`	netcoreapp3.1	237ns	0.29ns	1.05ns	0.00582	0	424 B
#3166	`AllCycleMoreComplexBody`	net472	185ns	0.199ns	0.769ns	0.0637	0	401 B
#3166	`AllCycleMoreComplexBody`	netcoreapp3.1	233ns	0.264ns	0.989ns	0.0055	0	400 B
#3166	`BodyExtractorSimpleBody`	net472	267ns	0.37ns	1.38ns	0.0573	0	361 B
#3166	`BodyExtractorSimpleBody`	netcoreapp3.1	230ns	0.526ns	2.04ns	0.00369	0	272 B
#3166	`BodyExtractorMoreComplexBody`	net472	14.2μs	8.83ns	30.6ns	1.21	0.0211	7.62 KB
#3166	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.2μs	13.7ns	51.3ns	0.0904	0	6.75 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
master	`SendRequest`	netcoreapp3.1	182μs	245ns	950ns	0.184	20.57 KB
#3166	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3166	`SendRequest`	netcoreapp3.1	184μs	199ns	770ns	0.182	20.57 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net472	1.91μs	0.849ns	3.18ns	0.15	0.000949	947 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.52μs	0.739ns	2.86ns	0.0129	0	936 B
#3166	`ExecuteNonQuery`	net472	1.85μs	0.691ns	2.59ns	0.15	0.000931	947 B
#3166	`ExecuteNonQuery`	netcoreapp3.1	1.4μs	0.581ns	2.25ns	0.0126	0	936 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.47μs	0.662ns	2.48ns	0.183	1.16 KB
master	`CallElasticsearch`	netcoreapp3.1	1.55μs	0.282ns	1.02ns	0.0147	1.1 KB
master	`CallElasticsearchAsync`	net472	2.66μs	0.809ns	3.13ns	0.205	1.29 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.58μs	0.351ns	1.27ns	0.0166	1.22 KB
#3166	`CallElasticsearch`	net472	2.51μs	0.7ns	2.71ns	0.184	1.16 KB
#3166	`CallElasticsearch`	netcoreapp3.1	1.52μs	0.379ns	1.42ns	0.0149	1.1 KB
#3166	`CallElasticsearchAsync`	net472	2.71μs	1.12ns	4.34ns	0.204	1.29 KB
#3166	`CallElasticsearchAsync`	netcoreapp3.1	1.66μs	0.594ns	2.14ns	0.0159	1.22 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	2.64μs	6.43ns	24.1ns	0.225	1.42 KB
master	`ExecuteAsync`	netcoreapp3.1	1.76μs	3.9ns	15.1ns	0.0177	1.34 KB
#3166	`ExecuteAsync`	net472	2.6μs	8.5ns	32.9ns	0.225	1.42 KB
#3166	`ExecuteAsync`	netcoreapp3.1	1.74μs	5.19ns	20.1ns	0.018	1.34 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	5.78μs	12.5ns	48.5ns	0.439	2.77 KB
master	`SendAsync`	netcoreapp3.1	3.56μs	7.26ns	27.1ns	0.0349	2.6 KB
#3166	`SendAsync`	net472	5.7μs	14.8ns	57.2ns	0.439	2.77 KB
#3166	`SendAsync`	netcoreapp3.1	3.67μs	3.77ns	13.1ns	0.036	2.6 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.16μs	2.13ns	7.95ns	0.288	1.81 KB
master	`EnrichedLog`	netcoreapp3.1	2.63μs	0.779ns	3.02ns	0.0251	1.85 KB
#3166	`EnrichedLog`	net472	3.11μs	1.75ns	6.56ns	0.287	1.81 KB
#3166	`EnrichedLog`	netcoreapp3.1	2.5μs	1.73ns	6.68ns	0.0252	1.85 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	153μs	94.1ns	365ns	0.691	0.23	4.65 KB
master	`EnrichedLog`	netcoreapp3.1	117μs	198ns	767ns	0.058	0	4.49 KB
#3166	`EnrichedLog`	net472	149μs	89.3ns	346ns	0.674	0.225	4.65 KB
#3166	`EnrichedLog`	netcoreapp3.1	117μs	92.5ns	358ns	0	0	4.49 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	5.6μs	21.2ns	82ns	0.568	0.00276	3.59 KB
master	`EnrichedLog`	netcoreapp3.1	4.37μs	9.07ns	32.7ns	0.0542	0	3.91 KB
#3166	`EnrichedLog`	net472	5.53μs	17ns	65.8ns	0.57	0.00275	3.59 KB
#3166	`EnrichedLog`	netcoreapp3.1	4.53μs	10.3ns	40.1ns	0.0536	0	3.91 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.35μs	1.26ns	4.36ns	0.217	1.37 KB
master	`SendReceive`	netcoreapp3.1	1.79μs	0.643ns	2.49ns	0.0179	1.32 KB
#3166	`SendReceive`	net472	2.3μs	1.07ns	4.15ns	0.217	1.37 KB
#3166	`SendReceive`	netcoreapp3.1	1.75μs	0.657ns	2.37ns	0.0176	1.32 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	4.98μs	2.01ns	7.77ns	0.353	2.23 KB
master	`EnrichedLog`	netcoreapp3.1	4.31μs	2.74ns	10.6ns	0.0237	1.8 KB
#3166	`EnrichedLog`	net472	4.99μs	0.746ns	2.79ns	0.352	2.23 KB
#3166	`EnrichedLog`	netcoreapp3.1	4.29μs	2.02ns	7.83ns	0.0235	1.8 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	1.13μs	0.34ns	1.27ns	0.128	810 B
master	`StartFinishSpan`	netcoreapp3.1	966ns	1.19ns	4.29ns	0.0101	760 B
master	`StartFinishScope`	net472	1.43μs	1.42ns	5.5ns	0.141	891 B
master	`StartFinishScope`	netcoreapp3.1	1.1μs	0.468ns	1.75ns	0.0121	880 B
#3166	`StartFinishSpan`	net472	1.17μs	0.441ns	1.65ns	0.129	810 B
#3166	`StartFinishSpan`	netcoreapp3.1	961ns	0.424ns	1.59ns	0.0101	760 B
#3166	`StartFinishScope`	net472	1.47μs	0.909ns	3.52ns	0.141	891 B
#3166	`StartFinishScope`	netcoreapp3.1	1.04μs	0.513ns	1.85ns	0.012	880 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.6μs	0.604ns	2.34ns	0.141	891 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.18μs	0.703ns	2.63ns	0.0118	880 B
#3166	`RunOnMethodBegin`	net472	1.45μs	0.647ns	2.5ns	0.141	891 B
#3166	`RunOnMethodBegin`	netcoreapp3.1	1.17μs	0.322ns	1.16ns	0.0123	880 B

andrewlock · 2022-09-05T13:03:17Z

Code Coverage Report 📊

✔️ Merging #3166 into master will not change line coverage
✔️ Merging #3166 into master will not change branch coverage
✔️ Merging #3166 into master will will decrease complexity by 8

	master	#3166	Change
Lines	`17529` / `24154`	`17514` / `24135`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`10378` / `14883`	`10381` / `14878`
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`16068`	`16060`	`-8` ✔️

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3166	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`16068`	`16060`	`-8` ✔️

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.Debugger.DebuggerSettings	`-1%` ⚠️	`-19%` ⛔	`-5` ✔️
Datadog.Trace.Ci.CIVisibility	`5%` ✔️	`6%` ✔️	`0` ✔️
Datadog.Trace.Agent.DiscoveryService.DiscoveryService	`11%` ✔️	`0%` ✔️	`0` ✔️

1 classes were removed from Datadog.Trace in #3166

View the full reports for further details:

remove immutable debugger settings

81e811f

shurivich requested review from a team as code owners September 4, 2022 15:07

This comment has been minimized.

Sign in to view

fix tests

48b9ecd

This comment has been minimized.

Sign in to view

shurivich self-assigned this Sep 5, 2022

shurivich added the area:debugger label Sep 5, 2022

OmerRaviv reviewed Sep 5, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Debugger/Snapshots/DebuggerSnapshotSerializer.cs

OmerRaviv approved these changes Sep 5, 2022

View reviewed changes

GreenMatan reviewed Sep 5, 2022

View reviewed changes

andrewlock approved these changes Sep 5, 2022

View reviewed changes

dsiable key

bf5ae3e

shurivich merged commit 8c07d02 into master Sep 6, 2022

shurivich deleted the shurivich/remove_immutable_debugger_settings branch September 6, 2022 08:30

github-actions Bot added this to the vNext milestone Sep 6, 2022

Uh oh!

Conversation

shurivich commented Sep 4, 2022

Summary of changes

Reason for change

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

OmerRaviv left a comment

Choose a reason for hiding this comment

Uh oh!

GreenMatan Sep 5, 2022

Choose a reason for hiding this comment

Uh oh!

shurivich Sep 5, 2022

Choose a reason for hiding this comment

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Sep 5, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

andrewlock commented Sep 5, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants