Skip to content

Disable warnings in vendored code#2672

Merged
OmerRaviv merged 8 commits into
masterfrom
omerr/disableWarningsInVendoredCode
Apr 12, 2022
Merged

Disable warnings in vendored code#2672
OmerRaviv merged 8 commits into
masterfrom
omerr/disableWarningsInVendoredCode

Conversation

@OmerRaviv

@OmerRaviv OmerRaviv commented Apr 12, 2022

Copy link
Copy Markdown
Collaborator

Summary of changes

Automatically disable warnings in vendored code.

Reason for change

#2592 introduced dnlib as a new vendored dependency. Unfortunately, I did not notice at the same that this introduced a bunch of nasty compiler warnings.

This change adds the ability to automatically ignore warnings in vendored code, without being forced to add the exclusions to the GlobalSupperssions.cs file (and thus lose those warnings when they arise in our own code).

Note

As a bonus, I disabled the CS0618 warning in our own code, which gets us to zero-warnings nirvana in Datadog.Trace.csproj

@OmerRaviv
OmerRaviv requested a review from a team as a code owner April 12, 2022 01:51
@OmerRaviv OmerRaviv mentioned this pull request Apr 12, 2022
Merged
@OmerRaviv
OmerRaviv requested a review from andrewlock April 12, 2022 01:53
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

andrewlock commented Apr 12, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #2672 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 1.01ms 5.92μs 55.8μs 0 0 0 3.17 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 771μs 4.52μs 42μs 0 0 0 2.57 KB
#2672 WriteAndFlushEnrichedTraces net472 976μs 4.56μs 19.9μs 0 0 0 3.16 KB
#2672 WriteAndFlushEnrichedTraces netcoreapp3.1 765μs 3.77μs 16μs 0 0 0 2.57 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 306ns 0.998ns 3.73ns 0.0519 0 0 329 B
master AllCycleSimpleBody netcoreapp3.1 383ns 1.64ns 6.55ns 0.00458 0 0 328 B
master AllCycleMoreComplexBody net472 298ns 0.797ns 2.87ns 0.0482 0 0 305 B
master AllCycleMoreComplexBody netcoreapp3.1 387ns 2.09ns 10.9ns 0.00418 0 0 304 B
master BodyExtractorSimpleBody net472 440ns 1.88ns 7.04ns 0.0569 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 445ns 2.05ns 7.94ns 0.00375 0 0 272 B
master BodyExtractorMoreComplexBody net472 25.3μs 137ns 737ns 1.19 0.0127 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 23.3μs 126ns 665ns 0.0958 0 0 6.75 KB
#2672 AllCycleSimpleBody net472 304ns 1.27ns 4.74ns 0.052 0 0 329 B
#2672 AllCycleSimpleBody netcoreapp3.1 408ns 2.39ns 22.7ns 0.00444 0 0 328 B
#2672 AllCycleMoreComplexBody net472 303ns 1.47ns 6.43ns 0.0482 0 0 305 B
#2672 AllCycleMoreComplexBody netcoreapp3.1 388ns 1.9ns 8.72ns 0.00429 0 0 304 B
#2672 BodyExtractorSimpleBody net472 444ns 2.35ns 17ns 0.057 0 0 361 B
#2672 BodyExtractorSimpleBody netcoreapp3.1 456ns 1.4ns 5.06ns 0.00362 0 0 272 B
#2672 BodyExtractorMoreComplexBody net472 27.2μs 155ns 1.2μs 1.18 0.0127 0 7.62 KB
#2672 BodyExtractorMoreComplexBody netcoreapp3.1 23μs 116ns 531ns 0.0927 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 340μs 1.51μs 6.21μs 0.168 0 0 19.74 KB
#2672 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#2672 SendRequest netcoreapp3.1 333μs 1.55μs 6.02μs 0.167 0 0 19.74 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.91μs 10.3ns 56.3ns 0.094 0.00099 0 594 B
master ExecuteNonQuery netcoreapp3.1 1.54μs 6.62ns 24.8ns 0.00905 0 0 632 B
#2672 ExecuteNonQuery net472 1.83μs 8.5ns 34ns 0.0938 0.000929 0 594 B
#2672 ExecuteNonQuery netcoreapp3.1 1.6μs 7.33ns 29.3ns 0.00847 0 0 632 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.79μs 12.6ns 47ns 0.126 0 0 803 B
master CallElasticsearch netcoreapp3.1 1.74μs 5.12ns 19.2ns 0.0108 0 0 792 B
master CallElasticsearchAsync net472 2.97μs 15.2ns 71.3ns 0.148 0.00144 0 939 B
master CallElasticsearchAsync netcoreapp3.1 1.87μs 8.08ns 31.3ns 0.013 0 0 912 B
#2672 CallElasticsearch net472 2.75μs 10.8ns 41.7ns 0.126 0 0 803 B
#2672 CallElasticsearch netcoreapp3.1 1.88μs 10.3ns 66ns 0.011 0 0 792 B
#2672 CallElasticsearchAsync net472 3.09μs 17.8ns 151ns 0.146 0 0 939 B
#2672 CallElasticsearchAsync netcoreapp3.1 1.85μs 8.63ns 34.5ns 0.0121 0 0 912 B
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 3.24μs 12.6ns 47.3ns 0.167 0.00165 0 1.06 KB
master ExecuteAsync netcoreapp3.1 2.09μs 8.87ns 40.6ns 0.0142 0 0 1.03 KB
#2672 ExecuteAsync net472 3.27μs 17.7ns 97.1ns 0.167 0.00159 0 1.06 KB
#2672 ExecuteAsync netcoreapp3.1 2.08μs 8.91ns 34.5ns 0.0144 0 0 1.03 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 7.08μs 31.5ns 109ns 0.36 0 0 2.28 KB
master SendAsync netcoreapp3.1 5.14μs 29.7ns 252ns 0.0314 0 0 2.21 KB
#2672 SendAsync net472 7.22μs 35.6ns 159ns 0.361 0 0 2.28 KB
#2672 SendAsync netcoreapp3.1 4.89μs 24.3ns 111ns 0.0305 0 0 2.21 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.68μs 20.3ns 163ns 0.227 0 0 1.45 KB
master EnrichedLog netcoreapp3.1 3.12μs 10.4ns 39.1ns 0.02 0 0 1.53 KB
#2672 EnrichedLog net472 3.56μs 19.6ns 113ns 0.228 0 0 1.45 KB
#2672 EnrichedLog netcoreapp3.1 3.22μs 12.4ns 44.9ns 0.0206 0 0 1.53 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 303μs 1.02μs 3.68μs 0.444 0.148 0 4.33 KB
master EnrichedLog netcoreapp3.1 240μs 869ns 3.25μs 0 0 0 4.21 KB
#2672 EnrichedLog net472 300μs 961ns 3.6μs 0.45 0.15 0 4.33 KB
#2672 EnrichedLog netcoreapp3.1 249μs 1.39μs 9.51μs 0 0 0 4.21 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 7.94μs 36.5ns 141ns 0.505 0 0 3.23 KB
master EnrichedLog netcoreapp3.1 6.63μs 32.8ns 147ns 0.0494 0 0 3.6 KB
#2672 EnrichedLog net472 8.46μs 49.3ns 435ns 0.505 0 0 3.23 KB
#2672 EnrichedLog netcoreapp3.1 6.53μs 32.5ns 134ns 0.0509 0 0 3.6 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.63μs 12.9ns 51.5ns 0.16 0 0 1.01 KB
master SendReceive netcoreapp3.1 2.12μs 12ns 88ns 0.0142 0 0 1.01 KB
#2672 SendReceive net472 2.43μs 12.9ns 68.2ns 0.16 0.00122 0 1.01 KB
#2672 SendReceive netcoreapp3.1 2.12μs 6.63ns 23.9ns 0.0139 0 0 1.01 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 6.06μs 19.7ns 73.6ns 0.29 0 0 1.87 KB
master EnrichedLog netcoreapp3.1 5.5μs 32ns 283ns 0.0206 0 0 1.49 KB
#2672 EnrichedLog net472 6.18μs 24.8ns 92.9ns 0.292 0 0 1.87 KB
#2672 EnrichedLog netcoreapp3.1 5.38μs 26.2ns 108ns 0.0191 0 0 1.49 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 975ns 5.59ns 43.6ns 0.0718 0 0 457 B
master StartFinishSpan netcoreapp3.1 888ns 3.88ns 14.5ns 0.00609 0 0 456 B
master StartFinishScope net472 1.15μs 4.25ns 14.7ns 0.0842 0 0 538 B
master StartFinishScope netcoreapp3.1 1.06μs 4.72ns 17.7ns 0.00791 0 0 576 B
#2672 StartFinishSpan net472 928ns 4.63ns 19.1ns 0.0718 0 0 457 B
#2672 StartFinishSpan netcoreapp3.1 914ns 4.32ns 18.3ns 0.00632 0 0 456 B
#2672 StartFinishScope net472 1.16μs 5.41ns 25.4ns 0.0843 0 0 538 B
#2672 StartFinishScope netcoreapp3.1 1.1μs 6.43ns 63ns 0.00754 0 0 576 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.34μs 5.55ns 22.9ns 0.0844 0 0 538 B
master RunOnMethodBegin netcoreapp3.1 1.19μs 5.51ns 25.8ns 0.00765 0 0 576 B
#2672 RunOnMethodBegin net472 1.34μs 7.67ns 54.2ns 0.0843 0 0 538 B
#2672 RunOnMethodBegin netcoreapp3.1 1.19μs 5.16ns 19.3ns 0.0076 0 0 576 B

andrewlock
andrewlock approved these changes Apr 12, 2022
View reviewed changes

@andrewlock andrewlock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Comment thread tracer/build/_build/UpdateVendors/VendoredDependency.cs Outdated
Comment thread tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs Outdated
Comment thread tracer/src/Datadog.Trace/ClrProfiler/ServerlessInstrumentation/AWS/LambdaRequestBuilder.cs Outdated
Comment thread tracer/src/Datadog.Trace/Telemetry/Transports/JsonWebRequestTelemetryTransport.cs Outdated
@andrewlock

This comment has been minimized.

Sign in to view
1 similar comment
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

This comment has been minimized.

Sign in to view
OmerRaviv and others added 8 commits April 12, 2022 18:27
@OmerRaviv
OmerRaviv force-pushed the omerr/disableWarningsInVendoredCode branch from b475804 to 9b819cd Compare April 12, 2022 15:27
@andrewlock

andrewlock commented Apr 12, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

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

master #2672 Change
Lines 13518 / 18627 13607 / 18669
Lines % 73% 73% 0% ✔️
Branches 7820 / 11125 7863 / 11165
Branches % 70% 70% 0% ✔️
Complexity 12431 12478 47 ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #2672 Change
Lines % 73% 73% 0% ✔️
Branches % 70% 70% 0% ✔️
Complexity 12431 12478 47 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Telemetry.JsonWebRequestTelemetryTransport -48% ⛔ -68% ⛔ 30 ⛔
Datadog.Trace.Agent.Transports.ApiWebRequestFactory -5% ⛔ -6% ⛔ 11 ⛔
Datadog.Trace.Ci.CIVisibility 7% ✔️ 7% ✔️ 0 ✔️
Datadog.Trace.Ci.Processors.OriginTagTraceProcessor 9% ✔️ 17% ✔️ 0 ✔️
Datadog.Trace.Telemetry.JsonHttpClientTelemetryTransport 26% ✔️ 38% ✔️ 0 ✔️

View the full reports for further details:

@OmerRaviv
OmerRaviv merged commit a210dde into master Apr 12, 2022
@OmerRaviv
OmerRaviv deleted the omerr/disableWarningsInVendoredCode branch April 12, 2022 17:01
@github-actions github-actions Bot added this to the vNext milestone Apr 12, 2022
@andrewlock andrewlock added type:cleanup Minor code clean up area:vendors Code from other vendors labels Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrewlock andrewlock andrewlock approved these changes

Assignees

No one assigned

Labels

area:vendors Code from other vendors type:cleanup Minor code clean up

Projects

None yet

Milestone

2.7.0

Development

Successfully merging this pull request may close these issues.

2 participants

@OmerRaviv @andrewlock