Skip to content

Conversation

@andrewlock
Copy link
Member

@andrewlock andrewlock commented Nov 29, 2022

Summary of changes

  • Update tests (only) to target net462 instead of net461
  • Update testing SDK, xunit, coverlet, and DatadogTestLogger to fix .NET FX code coverage

Reason for change

In #3482, after updating to the .NET 7 SDK, we had to disable code coverage when running on .NET FX, as incompatibilities meant the tests weren't running at all. To fix these issues, we need to update to the latest Microsoft.NET.Test.Sdk package version. Unfortunately, this doesn't support net461, netcoreapp2.1, or netcoreapp3.0.

Implementation details

To avoid dropping support for netcoreapp2.1 and netcoreapp3.0 did several things

Test coverage

Should have the same level of coverage, but as we can now enable code coverage for .NET Framework again, the reported numbers will likely change a bit

Other details

I had to install the .NET Framework 4.6.2 targeting pack on the Windows VMs. Note that you may need to install this with Visual Studio too (I updated the .vsconfig file appropriately)

@andrewlock andrewlock added area:builds project files, build scripts, pipelines, versioning, releases, packages area:tests unit tests, integration tests area:test-apps apps used to test integrations labels Nov 29, 2022
@andrewlock andrewlock requested review from a team as code owners November 29, 2022 13:56
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Nov 29, 2022

Datadog Report

Branch report: andrew/conditional-test-fx
Commit report: 9f283af

❄️ dd-trace-dotnet 0 Failed, 2 New Flaky, 280531 Passed, 1124 Skipped, 22m 53.54s Wall Time

New Flaky Tests (2)

  • NoExceptions - Datadog.Trace.ClrProfiler.IntegrationTests.SmokeTests.DuplicateTypeProxySmokeTest - Last Failure

    Expand for error
     The smoke test is running for too long or was lost.
    
  • NoExceptions - Datadog.Trace.ClrProfiler.IntegrationTests.SmokeTests.ServiceBusMinimalNServiceBusTest - Last Failure

    Expand for error
     The smoke test is running for too long or was lost.
    

@andrewlock

This comment has been minimized.

@andrewlock
Copy link
Member Author

Code Coverage Report 📊

✔️ Merging #3520 into master will will increase line coverage by 2%
✔️ Merging #3520 into master will not change branch coverage
⛔ Merging #3520 into master will will increase complexity by 1397

master #3520 Change
Lines 19723 / 27079 20928 / 28123
Lines % 73% 74% 2% ✔️
Branches 11336 / 16449 12248 / 17821
Branches % 69% 69% 0% ✔️
Complexity 17868 19265 1397

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #3520 Change
Lines % 73% 74% 2% ✔️
Branches % 69% 69% 0% ✔️
Complexity 17868 19265 1397

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.DuckTyping.TypesTuple 0% ✔️ 17% ✔️ 0 ✔️
Datadog.Trace.ExtensionMethods.SpanExtensions 4% ✔️ 7% ✔️ 0 ✔️
Datadog.Trace.AppSec.Waf.Encoder 5% ✔️ 7% ✔️ 0 ✔️
Datadog.Trace.Propagators.DistributedContextExtractor 6% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Tracer 7% ✔️ 7% ✔️ 0 ✔️
Datadog.Trace.AppSec.Waf.Result 8% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Propagators.SpanContextPropagator 8% ✔️ 6% ✔️ 0 ✔️
Datadog.Trace.AppSec.Waf.NativeBindings.WafNative 9% ✔️ 11% ✔️ 0 ✔️
Datadog.Trace.Configuration.GlobalSettings 10% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Headers.Ip.RequestIpExtractor 10% ✔️ 0% ✔️ 0 ✔️
...And 26 more

The following classes were added in #3520:

File Line coverage Branch coverage Complexity
Datadog.Trace.AppSec.ControllerContextExtensions 95% 100% 12
Datadog.Trace.AppSec.Transports.Http.HttpTransport 73% 75% 19
Datadog.Trace.AspNet.SharedItems 96% 88% 19
Datadog.Trace.AspNet.TracingHttpModule 76% 74% 95
Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNet.ApiController_ExecuteAsync_Integration 97% 88% 9
...And 27 more

View the full reports for further details:

@andrewlock andrewlock force-pushed the andrew/conditional-test-fx branch from 48d23cb to 9f283af Compare December 5, 2022 11:25
@andrewlock
Copy link
Member Author

Benchmarks Report 🐌

Benchmarks for #3520 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 753μs 675ns 2.61μs 0.374 0 0 3.22 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 563μs 249ns 964ns 0 0 0 2.63 KB
#3520 WriteAndFlushEnrichedTraces net472 763μs 538ns 2.01μs 0.377 0 0 3.22 KB
#3520 WriteAndFlushEnrichedTraces netcoreapp3.1 554μs 176ns 660ns 0 0 0 2.63 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 184ns 0.0161ns 0.0557ns 0.0676 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 241ns 0.196ns 0.759ns 0.00588 0 0 424 B
master AllCycleMoreComplexBody net472 181ns 0.0921ns 0.345ns 0.0637 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 238ns 0.2ns 0.776ns 0.00557 0 0 400 B
master BodyExtractorSimpleBody net472 280ns 0.626ns 2.43ns 0.0573 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 235ns 0.155ns 0.578ns 0.00366 0 0 272 B
master BodyExtractorMoreComplexBody net472 15.9μs 14.5ns 56.3ns 1.21 0.0159 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 12.9μs 4.67ns 16.9ns 0.0899 0 0 6.75 KB
#3520 AllCycleSimpleBody net472 183ns 0.143ns 0.535ns 0.0675 0 0 425 B
#3520 AllCycleSimpleBody netcoreapp3.1 239ns 0.261ns 1.01ns 0.00589 0 0 424 B
#3520 AllCycleMoreComplexBody net472 180ns 0.0554ns 0.2ns 0.0637 0 0 401 B
#3520 AllCycleMoreComplexBody netcoreapp3.1 237ns 0.199ns 0.773ns 0.00546 0 0 400 B
#3520 BodyExtractorSimpleBody net472 284ns 0.136ns 0.492ns 0.0573 0 0 361 B
#3520 BodyExtractorSimpleBody netcoreapp3.1 238ns 0.114ns 0.426ns 0.00369 0 0 272 B
#3520 BodyExtractorMoreComplexBody net472 16.2μs 16.3ns 63ns 1.21 0.0162 0 7.62 KB
#3520 BodyExtractorMoreComplexBody netcoreapp3.1 12.5μs 5.53ns 20.7ns 0.0871 0 0 6.75 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 net472 0ns 0ns 0ns 0 0 0 0 b
master SendRequest netcoreapp3.1 176μs 145ns 563ns 0.263 0 0 20.44 KB
#3520 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3520 SendRequest netcoreapp3.1 175μs 109ns 421ns 0.262 0 0 20.44 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.87μs 0.759ns 2.94ns 0.161 0.000939 0 1.01 KB
master ExecuteNonQuery netcoreapp3.1 1.38μs 0.447ns 1.67ns 0.0138 0 0 1 KB
#3520 ExecuteNonQuery net472 1.77μs 0.577ns 2.23ns 0.161 0.000908 0 1.01 KB
#3520 ExecuteNonQuery netcoreapp3.1 1.43μs 0.677ns 2.53ns 0.0129 0 0 1 KB
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.48μs 0.831ns 3.22ns 0.193 0 0 1.22 KB
master CallElasticsearch netcoreapp3.1 1.53μs 0.748ns 2.9ns 0.0153 0 0 1.16 KB
master CallElasticsearchAsync net472 2.68μs 1.2ns 4.66ns 0.215 0 0 1.36 KB
master CallElasticsearchAsync netcoreapp3.1 1.61μs 0.406ns 1.52ns 0.0169 0 0 1.28 KB
#3520 CallElasticsearch net472 2.43μs 1.09ns 4.2ns 0.193 0 0 1.22 KB
#3520 CallElasticsearch netcoreapp3.1 1.5μs 0.617ns 2.31ns 0.0159 0 0 1.16 KB
#3520 CallElasticsearchAsync net472 2.7μs 1.55ns 5.82ns 0.214 0 0 1.36 KB
#3520 CallElasticsearchAsync netcoreapp3.1 1.6μs 0.865ns 3.35ns 0.0174 0 0 1.28 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.64μs 0.934ns 3.5ns 0.236 0 0 1.49 KB
master ExecuteAsync netcoreapp3.1 1.75μs 0.459ns 1.78ns 0.0193 0 0 1.41 KB
#3520 ExecuteAsync net472 2.67μs 1.31ns 4.72ns 0.235 0 0 1.49 KB
#3520 ExecuteAsync netcoreapp3.1 1.76μs 0.611ns 2.37ns 0.0194 0 0 1.41 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.7μs 1.86ns 7.2ns 0.45 0 0 2.83 KB
master SendAsync netcoreapp3.1 3.64μs 2.42ns 8.72ns 0.0365 0 0 2.66 KB
#3520 SendAsync net472 5.77μs 2.71ns 10.5ns 0.448 0 0 2.83 KB
#3520 SendAsync netcoreapp3.1 3.65μs 1.37ns 4.93ns 0.0349 0 0 2.66 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 2.73μs 0.845ns 2.93ns 0.297 0 0 1.88 KB
master EnrichedLog netcoreapp3.1 2.24μs 0.904ns 3.5ns 0.0257 0 0 1.91 KB
#3520 EnrichedLog net472 2.78μs 2.66ns 10.3ns 0.298 0 0 1.88 KB
#3520 EnrichedLog netcoreapp3.1 2.33μs 0.976ns 3.65ns 0.0255 0 0 1.91 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 150μs 128ns 494ns 0.746 0.224 0 4.72 KB
master EnrichedLog netcoreapp3.1 119μs 182ns 706ns 0 0 0 4.55 KB
#3520 EnrichedLog net472 151μs 111ns 429ns 0.749 0.225 0 4.72 KB
#3520 EnrichedLog netcoreapp3.1 117μs 98.5ns 369ns 0.0582 0 0 4.55 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.49μs 1.6ns 5.98ns 0.578 0.00272 0 3.65 KB
master EnrichedLog netcoreapp3.1 4.27μs 3.42ns 13.2ns 0.0535 0 0 3.98 KB
#3520 EnrichedLog net472 5.46μs 2.34ns 9.06ns 0.579 0.00273 0 3.65 KB
#3520 EnrichedLog netcoreapp3.1 4.21μs 1.55ns 5.59ns 0.0545 0 0 3.98 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.22μs 1.27ns 4.56ns 0.228 0 0 1.44 KB
master SendReceive netcoreapp3.1 1.8μs 0.553ns 2.14ns 0.0189 0 0 1.38 KB
#3520 SendReceive net472 2.26μs 9.23ns 35.8ns 0.228 0 0 1.44 KB
#3520 SendReceive netcoreapp3.1 1.82μs 0.496ns 1.79ns 0.0183 0 0 1.38 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 4.68μs 1.46ns 5.25ns 0.363 0 0 2.3 KB
master EnrichedLog netcoreapp3.1 4.07μs 1.5ns 5.62ns 0.0244 0 0 1.86 KB
#3520 EnrichedLog net472 4.81μs 1.73ns 6.71ns 0.364 0 0 2.3 KB
#3520 EnrichedLog netcoreapp3.1 4.11μs 1.14ns 4.4ns 0.0247 0 0 1.86 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.13μs 0.462ns 1.79ns 0.139 0 0 875 B
master StartFinishSpan netcoreapp3.1 905ns 1ns 3.76ns 0.0113 0 0 824 B
master StartFinishScope net472 1.4μs 0.412ns 1.54ns 0.151 0 0 955 B
master StartFinishScope netcoreapp3.1 1.04μs 0.445ns 1.66ns 0.013 0 0 944 B
#3520 StartFinishSpan net472 1.15μs 0.323ns 1.25ns 0.139 0 0 875 B
#3520 StartFinishSpan netcoreapp3.1 882ns 0.235ns 0.847ns 0.0111 0 0 824 B
#3520 StartFinishScope net472 1.36μs 0.283ns 1.1ns 0.151 0 0 955 B
#3520 StartFinishScope netcoreapp3.1 1.07μs 0.307ns 1.15ns 0.0129 0 0 944 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.41μs 0.626ns 2.43ns 0.151 0 0 955 B
master RunOnMethodBegin netcoreapp3.1 1.12μs 0.379ns 1.42ns 0.0128 0 0 944 B
#3520 RunOnMethodBegin net472 1.45μs 0.337ns 1.21ns 0.151 0 0 955 B
#3520 RunOnMethodBegin netcoreapp3.1 1.2μs 0.474ns 1.71ns 0.0128 0 0 944 B

Copy link
Collaborator

@gleocadie gleocadie left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@shurivich shurivich left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@robertpi robertpi left a comment

Choose a reason for hiding this comment

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

LGTM!

@andrewlock andrewlock merged commit bab4dad into master Dec 6, 2022
@andrewlock andrewlock deleted the andrew/conditional-test-fx branch December 6, 2022 10:21
@github-actions github-actions bot added this to the vNext milestone Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:builds project files, build scripts, pipelines, versioning, releases, packages area:test-apps apps used to test integrations area:tests unit tests, integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants