Skip to content

[tests] clean up test project references#3198

Merged
lucaspimentel merged 6 commits into
masterfrom
lpimentel/clean-up-test-project-references
Sep 15, 2022
Merged

[tests] clean up test project references#3198
lucaspimentel merged 6 commits into
masterfrom
lpimentel/clean-up-test-project-references

Conversation

@lucaspimentel

@lucaspimentel lucaspimentel commented Sep 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Summary of changes

Move a few files and project references from Datadog.Trace.TestHelpers to Datadog.Trace.TestHelpers.AutoInstrumentation to reduce their blast radius.

Reason for change

When running unit tests locally, everyone likes the dev loop (edit, build, run, repeat) to be as tight as possible.

Datadog.Trace.TestHelpers contains helpers and utilities shared by most test projects. There was code and dependencies in that project used exclusively for automatic instrumentation test that start a child process and use IL rewriting. I moved this code from Datadog.Trace.TestHelpers to a new project so it doesn't pollute the dependencies of all the other test projects.

In particular, this removes projects Datadog.InstrumentedAssemblyGenerator and Datadog.InstrumentedAssemblyVerification as dependencies to most test projects.

Implementation details

  • Added new project Datadog.Trace.TestHelpers.AutoInstrumentation

  • Moved the following files from Datadog.Trace.TestHelpers to the new project:

TestHelper.cs (used to start child processes with auto-instrumentation enabled)
IisFixture.cs (used to start IIS Express with auto-instrumentation enabled)
InstrumentationVerification.cs (used for IL verification)
  • Moved the following project references from Datadog.Trace.TestHelpers to the new project:
tracer/src/Datadog.InstrumentedAssemblyGenerator
tracer/src/Datadog.InstrumentedAssemblyVerification
  • The following projects were the only ones using this code, so they now reference the new project:
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests
tracer/test/Datadog.Trace.Security.IntegrationTests
tracer/test/Datadog.Trace.Tools.Runner.IntegrationTests

Test coverage

If the test project builds and runs, and the test still pass, we're good.

Other details

Bonus: Fix path to solution item tracer/GlobalSuppressions.cs. This does not affect the build, only the ability to open the file from IDEs like VS or Rider.

@lucaspimentel lucaspimentel added area:builds project files, build scripts, pipelines, versioning, releases, packages area:tests unit tests, integration tests type:cleanup Minor code clean up labels Sep 9, 2022
@andrewlock

This comment has been minimized.

Sign in to view
@lucaspimentel
lucaspimentel force-pushed the lpimentel/clean-up-test-project-references branch from a75d97c to a6594cb Compare September 12, 2022 15:13
@andrewlock

This comment has been minimized.

Sign in to view
@lucaspimentel
lucaspimentel force-pushed the lpimentel/clean-up-test-project-references branch from a6594cb to b383c04 Compare September 12, 2022 22:07
@andrewlock

This comment has been minimized.

Sign in to view
@lucaspimentel
lucaspimentel force-pushed the lpimentel/clean-up-test-project-references branch from b383c04 to 77c6bd4 Compare September 13, 2022 16:12
@andrewlock

This comment has been minimized.

Sign in to view
@lucaspimentel
lucaspimentel marked this pull request as ready for review September 13, 2022 20:23
@lucaspimentel
lucaspimentel requested review from a team as code owners September 13, 2022 20:23
dudikeleti
dudikeleti approved these changes Sep 14, 2022
View reviewed changes

@dudikeleti dudikeleti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Apologies, I completely forgot to clean this up. Thanks a lot, this is great!

@lucaspimentel
lucaspimentel force-pushed the lpimentel/clean-up-test-project-references branch from 77c6bd4 to 7852433 Compare September 14, 2022 17:01
@andrewlock

andrewlock commented Sep 14, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

✔️ Merging #3198 into master will not change line coverage
✔️ Merging #3198 into master will not change branch coverage
⛔ Merging #3198 into master will will increase complexity by 19

master #3198 Change
Lines 17971 / 24517 18000 / 24566
Lines % 73% 73% 0% ✔️
Branches 10585 / 15050 10597 / 15056
Branches % 70% 70% 0% ✔️
Complexity 16261 16280 19 ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #3198 Change
Lines % 73% 73% 0% ✔️
Branches % 70% 70% 0% ✔️
Complexity 16261 16280 19 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.RemoteConfigurationManagement.RemoteConfigurationCache -86% ⛔ 0% ✔️ 2 ⛔
Datadog.Trace.RemoteConfigurationManagement.ProductConfigChangedEventArgs -68% ⛔ -67% ⛔ 6 ⛔
Datadog.Trace.RemoteConfigurationManagement.Product -31% ⛔ -29% ⛔ 9 ⛔
Datadog.Trace.Ci.CIVisibility 5% ✔️ 6% ✔️ 0 ✔️

The following classes were added in #3198:

File Line coverage Branch coverage Complexity
Datadog.Trace.RemoteConfigurationManagement.ApplyDetails 0% 100% 2
Datadog.Trace.RemoteConfigurationManagement.NamedRawFile 100% 100% 1
Datadog.Trace.RemoteConfigurationManagement.NamedTypedFile`1 100% 100% 1

View the full reports for further details:

@andrewlock

andrewlock commented Sep 15, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #3198 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 715μs 1.26μs 4.9μs 0.359 0 0 3.18 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 458μs 620ns 2.4μs 0 0 0 2.58 KB
#3198 WriteAndFlushEnrichedTraces net472 717μs 1.77μs 6.38μs 0.359 0 0 3.18 KB
#3198 WriteAndFlushEnrichedTraces netcoreapp3.1 461μs 423ns 1.58μs 0 0 0 2.59 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 185ns 0.152ns 0.588ns 0.0675 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 238ns 0.242ns 0.904ns 0.00583 0 0 424 B
master AllCycleMoreComplexBody net472 180ns 0.105ns 0.379ns 0.0637 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 235ns 0.446ns 1.61ns 0.00553 0 0 400 B
master BodyExtractorSimpleBody net472 261ns 0.301ns 1.13ns 0.0573 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 215ns 0.197ns 0.737ns 0.0038 0 0 272 B
master BodyExtractorMoreComplexBody net472 14.8μs 14.7ns 57.1ns 1.21 0.0147 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 12.1μs 9.75ns 37.7ns 0.0905 0 0 6.75 KB
#3198 AllCycleSimpleBody net472 181ns 0.134ns 0.503ns 0.0675 0 0 425 B
#3198 AllCycleSimpleBody netcoreapp3.1 240ns 0.275ns 1.07ns 0.00591 0 0 424 B
#3198 AllCycleMoreComplexBody net472 181ns 0.134ns 0.518ns 0.0637 0 0 401 B
#3198 AllCycleMoreComplexBody netcoreapp3.1 233ns 0.277ns 1.07ns 0.00549 0 0 400 B
#3198 BodyExtractorSimpleBody net472 279ns 0.476ns 1.84ns 0.0573 0 0 361 B
#3198 BodyExtractorSimpleBody netcoreapp3.1 214ns 0.245ns 0.951ns 0.00371 0 0 272 B
#3198 BodyExtractorMoreComplexBody net472 14.7μs 10.1ns 37.9ns 1.2 0.0148 0 7.62 KB
#3198 BodyExtractorMoreComplexBody netcoreapp3.1 11.9μs 13.1ns 50.7ns 0.0896 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 0.000716ns 0.000625ns 0.00242ns 0 0 0 0 b
master SendRequest netcoreapp3.1 179μs 396ns 1.53μs 0.178 0 0 20.59 KB
#3198 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3198 SendRequest netcoreapp3.1 184μs 318ns 1.23μs 0.275 0 0 20.6 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.84μs 3.9ns 15.1ns 0.15 0 0 947 B
master ExecuteNonQuery netcoreapp3.1 1.44μs 2.05ns 7.66ns 0.0122 0 0 936 B
#3198 ExecuteNonQuery net472 1.79μs 2.35ns 9.09ns 0.15 0 0 947 B
#3198 ExecuteNonQuery netcoreapp3.1 1.47μs 1.1ns 4.12ns 0.0123 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.57μs 1.41ns 5.47ns 0.183 0 0 1.16 KB
master CallElasticsearch netcoreapp3.1 1.57μs 0.685ns 2.65ns 0.0149 0 0 1.1 KB
master CallElasticsearchAsync net472 2.77μs 1.32ns 5.11ns 0.204 0 0 1.29 KB
master CallElasticsearchAsync netcoreapp3.1 1.61μs 0.577ns 2.16ns 0.0162 0 0 1.22 KB
#3198 CallElasticsearch net472 2.65μs 1.04ns 4.01ns 0.183 0 0 1.16 KB
#3198 CallElasticsearch netcoreapp3.1 1.62μs 1.36ns 4.71ns 0.0146 0 0 1.1 KB
#3198 CallElasticsearchAsync net472 2.77μs 1.63ns 5.88ns 0.204 0 0 1.29 KB
#3198 CallElasticsearchAsync netcoreapp3.1 1.64μs 0.95ns 3.68ns 0.0164 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.82μs 8.46ns 32.8ns 0.225 0 0 1.42 KB
master ExecuteAsync netcoreapp3.1 1.7μs 0.893ns 3.34ns 0.0186 0 0 1.34 KB
#3198 ExecuteAsync net472 2.88μs 9.01ns 33.7ns 0.226 0 0 1.42 KB
#3198 ExecuteAsync netcoreapp3.1 1.8μs 0.592ns 2.29ns 0.0178 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.88μs 2.65ns 10.3ns 0.438 0 0 2.77 KB
master SendAsync netcoreapp3.1 3.73μs 0.904ns 3.38ns 0.0355 0 0 2.6 KB
#3198 SendAsync net472 5.9μs 2.94ns 11.4ns 0.439 0 0 2.77 KB
#3198 SendAsync netcoreapp3.1 3.68μs 5.89ns 22.1ns 0.0348 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.16μs 3.56ns 13.3ns 0.287 0.00157 0 1.81 KB
master EnrichedLog netcoreapp3.1 2.5μs 1.3ns 4.68ns 0.025 0 0 1.85 KB
#3198 EnrichedLog net472 3.18μs 4.64ns 18ns 0.287 0 0 1.81 KB
#3198 EnrichedLog netcoreapp3.1 2.48μs 0.982ns 3.8ns 0.0249 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 136ns 528ns 0.679 0.226 0 4.66 KB
master EnrichedLog netcoreapp3.1 119μs 131ns 506ns 0.0596 0 0 4.49 KB
#3198 EnrichedLog net472 153μs 130ns 502ns 0.689 0.23 0 4.65 KB
#3198 EnrichedLog netcoreapp3.1 118μs 163ns 630ns 0 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.96μs 3.16ns 12.2ns 0.568 0.00297 0 3.59 KB
master EnrichedLog netcoreapp3.1 4.48μs 2.72ns 10.2ns 0.053 0 0 3.91 KB
#3198 EnrichedLog net472 5.79μs 2.08ns 7.78ns 0.568 0.00288 0 3.59 KB
#3198 EnrichedLog netcoreapp3.1 4.38μs 11ns 42.8ns 0.0537 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.35μs 3.64ns 13.6ns 0.217 0 0 1.37 KB
master SendReceive netcoreapp3.1 1.85μs 1.06ns 4.1ns 0.0184 0 0 1.32 KB
#3198 SendReceive net472 2.37μs 1.84ns 6.87ns 0.218 0 0 1.37 KB
#3198 SendReceive netcoreapp3.1 1.87μs 2.05ns 7.93ns 0.0178 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.14μs 2.41ns 9.35ns 0.354 0 0 2.23 KB
master EnrichedLog netcoreapp3.1 4.37μs 2.88ns 11.2ns 0.024 0 0 1.8 KB
#3198 EnrichedLog net472 5.07μs 3.33ns 12.9ns 0.352 0 0 2.23 KB
#3198 EnrichedLog netcoreapp3.1 4.22μs 5.81ns 22.5ns 0.0232 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.18μs 0.559ns 2.17ns 0.128 0 0 810 B
master StartFinishSpan netcoreapp3.1 949ns 0.417ns 1.56ns 0.0104 0 0 760 B
master StartFinishScope net472 1.49μs 0.667ns 2.58ns 0.141 0 0 891 B
master StartFinishScope netcoreapp3.1 1.06μs 0.331ns 1.24ns 0.0117 0 0 880 B
#3198 StartFinishSpan net472 1.17μs 1.1ns 4.25ns 0.129 0 0 810 B
#3198 StartFinishSpan netcoreapp3.1 970ns 0.392ns 1.52ns 0.0102 0 0 760 B
#3198 StartFinishScope net472 1.45μs 0.769ns 2.88ns 0.141 0 0 891 B
#3198 StartFinishScope netcoreapp3.1 1.1μs 0.289ns 1.04ns 0.0121 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.51μs 0.899ns 3.48ns 0.141 0 0 891 B
master RunOnMethodBegin netcoreapp3.1 1.17μs 0.528ns 1.98ns 0.0117 0 0 880 B
#3198 RunOnMethodBegin net472 1.44μs 0.686ns 2.66ns 0.141 0 0 891 B
#3198 RunOnMethodBegin netcoreapp3.1 1.15μs 0.51ns 1.91ns 0.0121 0 0 880 B

@lucaspimentel
lucaspimentel merged commit 0257121 into master Sep 15, 2022
@lucaspimentel
lucaspimentel deleted the lpimentel/clean-up-test-project-references branch September 15, 2022 15:13
@github-actions github-actions Bot added this to the vNext milestone Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dudikeleti dudikeleti dudikeleti approved these changes
@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes

Assignees

No one assigned

Labels

area:builds project files, build scripts, pipelines, versioning, releases, packages area:tests unit tests, integration tests type:cleanup Minor code clean up

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

4 participants

@lucaspimentel @andrewlock @dudikeleti @zacharycmontoya