Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 23, 2021
Merged

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

merged 1 commit into from
Nov 23, 2021

Conversation

@zacharycmontoya
Copy link
Contributor

@zacharycmontoya zacharycmontoya commented Nov 23, 2021

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

@zacharycmontoya zacharycmontoya self-assigned this Nov 23, 2021
@zacharycmontoya zacharycmontoya requested a review from a team as a code owner November 23, 2021 18:10
tonyredondo
tonyredondo approved these changes Nov 23, 2021
View reviewed changes
Copy link
Member

@tonyredondo tonyredondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@andrewlock
Copy link
Member

andrewlock commented Nov 23, 2021

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 Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net472 963.3 μs 11.96 μs 11.19 μs 0.0000 0.0000 0.0000 3.09 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 781.4 μs 15.42 μs 27.80 μs 0.0000 0.0000 0.0000 2.51 KB
#2081 WriteAndFlushEnrichedTraces net472 984.0 μs 14.01 μs 13.10 μs 0.0000 0.0000 0.0000 3.09 KB
#2081 WriteAndFlushEnrichedTraces netcoreapp3.1 765.6 μs 9.43 μs 8.36 μs 0.0000 0.0000 0.0000 2.51 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net472 0.0032 ns 0.0131 ns 0.0122 ns 0.0000 0.0000 0.0000 0 B
master SendRequest netcoreapp3.1 314,788.7424 ns 6,266.9489 ns 5,555.4870 ns 0.1559 0.0000 0.0000 19617 B
#2081 SendRequest net472 0.0037 ns 0.0092 ns 0.0151 ns 0.0000 0.0000 0.0000 0 B
#2081 SendRequest netcoreapp3.1 317,539.6648 ns 6,218.4818 ns 6,107.3806 ns 0.1590 0.0000 0.0000 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 Gen 2 Allocated
master ExecuteNonQuery net472 341.1 ns 6.13 ns 5.44 ns 0.0000 0.0000 0.0000 0 B
master ExecuteNonQuery netcoreapp3.1 302.8 ns 4.93 ns 4.37 ns 0.0000 0.0000 0.0000 0 B
#2081 ExecuteNonQuery net472 2.220 μs 0.0443 μs 0.1008 μs 0.1075 0.0011 0.0000 682 B
#2081 ExecuteNonQuery netcoreapp3.1 1.972 μs 0.0303 μs 0.0284 μs 0.0097 0.0000 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 Gen 1 Gen 2 Allocated
master CallElasticsearch net472 4.121 μs 0.0804 μs 0.0790 μs 0.1264 0.0000 0.0000 827 B
master CallElasticsearch netcoreapp3.1 1.619 μs 0.0160 μs 0.0197 μs 0.0112 0.0000 0.0000 768 B
master CallElasticsearchAsync net472 3.530 μs 0.0670 μs 0.0717 μs 0.1496 0.0000 0.0000 963 B
master CallElasticsearchAsync netcoreapp3.1 1.806 μs 0.0254 μs 0.0225 μs 0.0119 0.0000 0.0000 888 B
#2081 CallElasticsearch net472 3.306 μs 0.0627 μs 0.0556 μs 0.1281 0.0000 0.0000 827 B
#2081 CallElasticsearch netcoreapp3.1 1.613 μs 0.0208 μs 0.0174 μs 0.0107 0.0000 0.0000 768 B
#2081 CallElasticsearchAsync net472 3.442 μs 0.0677 μs 0.0695 μs 0.1502 0.0000 0.0000 963 B
#2081 CallElasticsearchAsync netcoreapp3.1 2.011 μs 0.0339 μs 0.0486 μs 0.0122 0.0000 0.0000 888 B
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net472 3.634 μs 0.0622 μs 0.0552 μs 0.1695 0.0000 0.0000 1083 B
master ExecuteAsync netcoreapp3.1 1.893 μs 0.0305 μs 0.0285 μs 0.0143 0.0000 0.0000 1008 B
#2081 ExecuteAsync net472 3.571 μs 0.0408 μs 0.0361 μs 0.1679 0.0000 0.0000 1083 B
#2081 ExecuteAsync netcoreapp3.1 1.959 μs 0.0263 μs 0.0246 μs 0.0139 0.0000 0.0000 1008 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net472 6.133 μs 0.1130 μs 0.1002 μs 0.3495 0.0000 0.0000 2.19 KB
master SendAsync netcoreapp3.1 4.263 μs 0.0636 μs 0.0531 μs 0.0298 0.0000 0.0000 2.09 KB
#2081 SendAsync net472 6.411 μs 0.1265 μs 0.1183 μs 0.3485 0.0000 0.0000 2.19 KB
#2081 SendAsync netcoreapp3.1 4.515 μs 0.0833 μs 0.0780 μs 0.0294 0.0000 0.0000 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 Gen 1 Gen 2 Allocated
master EnrichedLog net472 4.098 μs 0.0608 μs 0.0508 μs 0.2867 0.0000 0.0000 1.79 KB
master EnrichedLog netcoreapp3.1 3.804 μs 0.0655 μs 0.0547 μs 0.0287 0.0000 0.0000 1.94 KB
#2081 EnrichedLog net472 5.383 μs 0.1071 μs 0.2329 μs 0.2856 0.0000 0.0000 1.79 KB
#2081 EnrichedLog netcoreapp3.1 4.143 μs 0.0818 μs 0.1517 μs 0.0268 0.0000 0.0000 1.94 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 191.0 μs 3.27 μs 4.37 μs 0.6732 0.2244 0.0000 5.23 KB
master EnrichedLog netcoreapp3.1 150.9 μs 2.71 μs 4.29 μs 0.0000 0.0000 0.0000 5.05 KB
#2081 EnrichedLog net472 181.1 μs 2.88 μs 2.55 μs 0.6368 0.1820 0.0000 5.23 KB
#2081 EnrichedLog netcoreapp3.1 151.7 μs 2.74 μs 2.29 μs 0.0771 0.0000 0.0000 5.05 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 12.15 μs 0.241 μs 0.258 μs 0.8519 0.0000 0.0000 5.31 KB
master EnrichedLog netcoreapp3.1 10.40 μs 0.174 μs 0.163 μs 0.0874 0.0000 0.0000 6.28 KB
#2081 EnrichedLog net472 12.77 μs 0.247 μs 0.378 μs 0.8501 0.0000 0.0000 5.31 KB
#2081 EnrichedLog netcoreapp3.1 10.46 μs 0.129 μs 0.107 μs 0.0869 0.0000 0.0000 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 Gen 1 Gen 2 Allocated
master SendReceive net472 2.227 μs 0.0439 μs 0.0411 μs 0.1544 0.0000 0.0000 987 B
master SendReceive netcoreapp3.1 1.966 μs 0.0287 μs 0.0269 μs 0.0138 0.0000 0.0000 984 B
#2081 SendReceive net472 2.657 μs 0.0366 μs 0.0324 μs 0.1536 0.0000 0.0000 987 B
#2081 SendReceive netcoreapp3.1 2.146 μs 0.0412 μs 0.0385 μs 0.0139 0.0000 0.0000 984 B
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net472 8.133 μs 0.1246 μs 0.1105 μs 0.4468 0.0000 0.0000 2.8 KB
master EnrichedLog netcoreapp3.1 7.700 μs 0.0879 μs 0.0779 μs 0.0352 0.0000 0.0000 2.61 KB
#2081 EnrichedLog net472 8.050 μs 0.1602 μs 0.1499 μs 0.4458 0.0000 0.0000 2.8 KB
#2081 EnrichedLog netcoreapp3.1 7.714 μs 0.1292 μs 0.1208 μs 0.0373 0.0000 0.0000 2.61 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

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

@andrewlock
Copy link
Member

andrewlock commented Nov 23, 2021

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.EndMethodHandler3](https://github.com/DataDog/dd-trace-dotnet/tree/16c90b5a03cca5e848cd994e52efb118a542968e/tracer/src/Datadog.Trace/ClrProfiler/CallTarget/Handlers/EndMethodHandler1.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:

@zacharycmontoya zacharycmontoya merged commit 15c0409 into release/1.x Nov 23, 2021
@zacharycmontoya zacharycmontoya deleted the zach/hotfix-1.30.0 branch November 23, 2021 19:58
@zacharycmontoya zacharycmontoya added this to the 1.30.1 milestone Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonyredondo tonyredondo tonyredondo approved these changes

@lucaspimentel lucaspimentel lucaspimentel approved these changes

Assignees

@zacharycmontoya zacharycmontoya

Labels

None yet

Projects

None yet

Milestone

1.30.1

Development

Successfully merging this pull request may close these issues.

5 participants

@zacharycmontoya @andrewlock @tonyredondo @lucaspimentel