Revert "Add modified WCF CallTarget instrumentation via opt-in environment variable (#1992)" by zacharycmontoya · Pull Request #2081 · DataDog/dd-trace-dotnet

zacharycmontoya · 2021-11-23T18:10:48Z

This reverts commit d009bf0.

The previous commit introduced a change to the CallTarget definition structure that passes integration information to the profiler, but this can cause issues if the profiler and the NuGet package versions mismatch.

@DataDog/apm-dotnet

…nment variable (#1992)" This reverts commit d009bf0.

tonyredondo

I think we can use com versioning for 2.0 if this change is required.

andrewlock · 2021-11-23T19:20:53Z

Benchmarks Report 🐌

Benchmarks for #2081 compared to master:

1 benchmarks are faster, with geometric mean 1.254
4 benchmarks are slower, with geometric mean 2.849
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	Error	StdDev	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	963.3 μs	11.96 μs	11.19 μs	3.09 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	781.4 μs	15.42 μs	27.80 μs	2.51 KB
#2081	`WriteAndFlushEnrichedTraces`	net472	984.0 μs	14.01 μs	13.10 μs	3.09 KB
#2081	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	765.6 μs	9.43 μs	8.36 μs	2.51 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`SendRequest`	net472	0.0032 ns	0.0131 ns	0.0122 ns	0.0000	0 B
master	`SendRequest`	netcoreapp3.1	314,788.7424 ns	6,266.9489 ns	5,555.4870 ns	0.1559	19617 B
#2081	`SendRequest`	net472	0.0037 ns	0.0092 ns	0.0151 ns	0.0000	0 B
#2081	`SendRequest`	netcoreapp3.1	317,539.6648 ns	6,218.4818 ns	6,107.3806 ns	0.1590	19833 B

Benchmarks.Trace.DbCommandBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2081

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net472	6.549	341.05	2,233.38	several?
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑netcoreapp3.1	6.540	302.54	1,978.69

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net472	341.1 ns	6.13 ns	5.44 ns	0.0000	0.0000	0 B
master	`ExecuteNonQuery`	netcoreapp3.1	302.8 ns	4.93 ns	4.37 ns	0.0000	0.0000	0 B
#2081	`ExecuteNonQuery`	net472	2.220 μs	0.0443 μs	0.1008 μs	0.1075	0.0011	682 B
#2081	`ExecuteNonQuery`	netcoreapp3.1	1.972 μs	0.0303 μs	0.0284 μs	0.0097	0.0000	712 B

Benchmarks.Trace.ElasticsearchBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #2081

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch‑net472	1.254	4,132.68	3,296.75

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	4.121 μs	0.0804 μs	0.0790 μs	0.1264	827 B
master	`CallElasticsearch`	netcoreapp3.1	1.619 μs	0.0160 μs	0.0197 μs	0.0112	768 B
master	`CallElasticsearchAsync`	net472	3.530 μs	0.0670 μs	0.0717 μs	0.1496	963 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.806 μs	0.0254 μs	0.0225 μs	0.0119	888 B
#2081	`CallElasticsearch`	net472	3.306 μs	0.0627 μs	0.0556 μs	0.1281	827 B
#2081	`CallElasticsearch`	netcoreapp3.1	1.613 μs	0.0208 μs	0.0174 μs	0.0107	768 B
#2081	`CallElasticsearchAsync`	net472	3.442 μs	0.0677 μs	0.0695 μs	0.1502	963 B
#2081	`CallElasticsearchAsync`	netcoreapp3.1	2.011 μs	0.0339 μs	0.0486 μs	0.0122	888 B

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	3.634 μs	0.0622 μs	0.0552 μs	0.1695	1083 B
master	`ExecuteAsync`	netcoreapp3.1	1.893 μs	0.0305 μs	0.0285 μs	0.0143	1008 B
#2081	`ExecuteAsync`	net472	3.571 μs	0.0408 μs	0.0361 μs	0.1679	1083 B
#2081	`ExecuteAsync`	netcoreapp3.1	1.959 μs	0.0263 μs	0.0246 μs	0.0139	1008 B

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	6.133 μs	0.1130 μs	0.1002 μs	0.3495	2.19 KB
master	`SendAsync`	netcoreapp3.1	4.263 μs	0.0636 μs	0.0531 μs	0.0298	2.09 KB
#2081	`SendAsync`	net472	6.411 μs	0.1265 μs	0.1183 μs	0.3485	2.19 KB
#2081	`SendAsync`	netcoreapp3.1	4.515 μs	0.0833 μs	0.0780 μs	0.0294	2.09 KB

Benchmarks.Trace.ILoggerBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2081

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.ILoggerBenchmark.EnrichedLog‑net472	1.293	4,102.32	5,302.80

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	4.098 μs	0.0608 μs	0.0508 μs	0.2867	1.79 KB
master	`EnrichedLog`	netcoreapp3.1	3.804 μs	0.0655 μs	0.0547 μs	0.0287	1.94 KB
#2081	`EnrichedLog`	net472	5.383 μs	0.1071 μs	0.2329 μs	0.2856	1.79 KB
#2081	`EnrichedLog`	netcoreapp3.1	4.143 μs	0.0818 μs	0.1517 μs	0.0268	1.94 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	191.0 μs	3.27 μs	4.37 μs	0.6732	0.2244	5.23 KB
master	`EnrichedLog`	netcoreapp3.1	150.9 μs	2.71 μs	4.29 μs	0.0000	0.0000	5.05 KB
#2081	`EnrichedLog`	net472	181.1 μs	2.88 μs	2.55 μs	0.6368	0.1820	5.23 KB
#2081	`EnrichedLog`	netcoreapp3.1	151.7 μs	2.74 μs	2.29 μs	0.0771	0.0000	5.05 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	12.15 μs	0.241 μs	0.258 μs	0.8519	5.31 KB
master	`EnrichedLog`	netcoreapp3.1	10.40 μs	0.174 μs	0.163 μs	0.0874	6.28 KB
#2081	`EnrichedLog`	net472	12.77 μs	0.247 μs	0.378 μs	0.8501	5.31 KB
#2081	`EnrichedLog`	netcoreapp3.1	10.46 μs	0.129 μs	0.107 μs	0.0869	6.28 KB

Benchmarks.Trace.RedisBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2081

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.RedisBenchmark.SendReceive‑net472	1.190	2,235.94	2,659.68

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.227 μs	0.0439 μs	0.0411 μs	0.1544	987 B
master	`SendReceive`	netcoreapp3.1	1.966 μs	0.0287 μs	0.0269 μs	0.0138	984 B
#2081	`SendReceive`	net472	2.657 μs	0.0366 μs	0.0324 μs	0.1536	987 B
#2081	`SendReceive`	netcoreapp3.1	2.146 μs	0.0412 μs	0.0385 μs	0.0139	984 B

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	8.133 μs	0.1246 μs	0.1105 μs	0.4468	2.8 KB
master	`EnrichedLog`	netcoreapp3.1	7.700 μs	0.0879 μs	0.0779 μs	0.0352	2.61 KB
#2081	`EnrichedLog`	net472	8.050 μs	0.1602 μs	0.1499 μs	0.4458	2.8 KB
#2081	`EnrichedLog`	netcoreapp3.1	7.714 μs	0.1292 μs	0.1208 μs	0.0373	2.61 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	934.3 ns	17.63 ns	15.63 ns	0.0678	433 B
master	`StartFinishSpan`	netcoreapp3.1	905.6 ns	13.23 ns	11.05 ns	0.0062	432 B
master	`StartFinishScope`	net472	1,138.1 ns	22.52 ns	23.12 ns	0.0805	514 B
master	`StartFinishScope`	netcoreapp3.1	1,073.9 ns	16.97 ns	23.78 ns	0.0078	552 B
#2081	`StartFinishSpan`	net472	929.5 ns	10.16 ns	9.00 ns	0.0677	433 B
#2081	`StartFinishSpan`	netcoreapp3.1	901.7 ns	13.04 ns	12.20 ns	0.0060	432 B
#2081	`StartFinishScope`	net472	1,115.6 ns	16.69 ns	13.93 ns	0.0806	514 B
#2081	`StartFinishScope`	netcoreapp3.1	1,086.9 ns	13.10 ns	12.26 ns	0.0074	552 B

andrewlock · 2021-11-23T19:39:36Z

Code Coverage Report 📊

⛔ Merging #2081 into master will will decrease line coverage by 5%
⚠️ Merging #2081 into master will will decrease branch coverage by 1%
⛔ Merging #2081 into master will will increase complexity by 1053

	master	#2081	Change
Lines	`8920` / `12345`	`10455` / `15639`
Lines %	`72%`	`67%`	`-5%` ⛔
Branches	`3962` / `5909`	`4561` / `6894`
Branches %	`67%`	`66%`	`-1%` ⚠️
Complexity	`7048`	`8101`	`1053` ⛔

View the full report for further details:

Datadog.Trace Breakdown ⛔

	master	#2081	Change
Lines %	`72%`	`67%`	`-5%` ⛔
Branches %	`67%`	`66%`	`-1%` ⚠️
Complexity	`7048`	`8101`	`1053` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.AppSec.Transport.Http.HttpTransport	`-11%` ⛔	`-75%` ⛔	`6` ⛔
Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.ILogger.LoggerIntegrationCommon	`-10%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.DuckTyping.DuckTypeExtensions	`-9%` ⛔	`-15%` ⛔	`22` ⛔
Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCompositeWorkItemSkipChildrenIntegration	`-8%` ⛔	`-8%` ⛔	`0` ✔️
Datadog.Trace.Util.UriHelpers	`-8%` ⛔	`-5%` ⛔	`4` ⛔
Datadog.Trace.DiagnosticListeners.DiagnosticObserver	`-7%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.Configuration.IntegrationSettingsCollection	`-4%` ⚠️	`-43%` ⛔	`7` ⛔
[Datadog.Trace.ClrProfiler.CallTarget.Handlers.EndMethodHandler`3](https://github.com/DataDog/dd-trace-dotnet/tree/16c90b5a03cca5e848cd994e52efb118a542968e/tracer/src/Datadog.Trace/ClrProfiler/CallTarget/Handlers/EndMethodHandler`1.cs)	`-4%` ⚠️	`-6%` ⛔	`0` ✔️
Datadog.Trace.Tracer	`-3%` ⚠️	`-11%` ⛔	`80` ⛔
Datadog.Trace.Configuration.IntegrationRegistry	`0%` ✔️	`-8%` ⛔	`1` ⛔
...And 19 more

The following classes were added in #2081:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.AppSec.AspNetCore.BlockingMiddleware	`41%`	`21%`	`25`
Datadog.Trace.AppSec.BlockActionException	`0%`	`100%`	`4`
Datadog.Trace.AppSec.PageBlockedByAppSecException	`0%`	`100%`	`4`
Datadog.Trace.AsyncLocalCompat`1	`100%`	`100%`	`8`
Datadog.Trace.Ci.CITracer	`100%`	`100%`	`1`
...And 72 more

45 classes were removed from Datadog.Trace in #2081

View the full reports for further details:

Revert "Add modified WCF CallTarget instrumentation via opt-in enviro…

f830cb5

…nment variable (#1992)" This reverts commit d009bf0.

zacharycmontoya self-assigned this Nov 23, 2021

zacharycmontoya requested a review from a team as a code owner November 23, 2021 18:10

lucaspimentel approved these changes Nov 23, 2021

View reviewed changes

tonyredondo approved these changes Nov 23, 2021

View reviewed changes

zacharycmontoya merged commit 15c0409 into release/1.x Nov 23, 2021

zacharycmontoya deleted the zach/hotfix-1.30.0 branch November 23, 2021 19:58

zacharycmontoya added this to the 1.30.1 milestone Nov 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "Add modified WCF CallTarget instrumentation via opt-in environment variable (#1992)"#2081

Revert "Add modified WCF CallTarget instrumentation via opt-in environment variable (#1992)"#2081
zacharycmontoya merged 1 commit intorelease/1.xfrom
zach/hotfix-1.30.0

zacharycmontoya commented Nov 23, 2021

Uh oh!

tonyredondo left a comment

Uh oh!

andrewlock commented Nov 23, 2021

Raw results

Raw results

Slower ⚠️ in #2081

Raw results

Faster 🎉 in #2081

Raw results

Raw results

Raw results

Slower ⚠️ in #2081

Raw results

Raw results

Raw results

Slower ⚠️ in #2081

Raw results

Raw results

Raw results

Uh oh!

andrewlock commented Nov 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

zacharycmontoya commented Nov 23, 2021

Uh oh!

tonyredondo left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Nov 23, 2021

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Slower ⚠️ in #2081

Raw results

Faster 🎉 in #2081

Raw results

Raw results

Raw results

Slower ⚠️ in #2081

Raw results

Raw results

Raw results

Slower ⚠️ in #2081

Raw results

Raw results

Raw results

Uh oh!

andrewlock commented Nov 23, 2021

Code Coverage Report 📊

Datadog.Trace Breakdown ⛔

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants