Skip to content

Fix flakey test Http_Headers_Contain_ContainerId #3206

Merged
OmerRaviv merged 4 commits into
masterfrom
omerr/figure-out-containerid-flakiness
Sep 13, 2022
Merged

Fix flakey test Http_Headers_Contain_ContainerId #3206
OmerRaviv merged 4 commits into
masterfrom
omerr/figure-out-containerid-flakiness

Conversation

@OmerRaviv

@OmerRaviv OmerRaviv commented Sep 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Reason for change

Following #3154 getting merged into master, we noticed this test is failing fairly consistently (on around 80%-90% of CI builds).
The likely culprit is an issue that may have already existed prior to that PR, but is now much more prevalent: certain requests that happen during startup (such as the request for /info for DiscoveryService) do not contain the ContainerId in the header. This created a race condition where depending on the order of requests, the test might flake out.

Summary of changes

Ensure that the ContainerTaggingTests.Http_Headers_Contain_ContainerId test will only inspect requests that report spans to the MockAgent, but not any other requests.

@OmerRaviv
Change Http_Headers_Contain_ContainerId so that it only looks at requ…
5e28871 
…ests that include spans and ignores other, unrelated requests
@OmerRaviv
OmerRaviv requested a review from a team as a code owner September 12, 2022 15:23
@github-actions github-actions Bot added the area:tests unit tests, integration tests label Sep 12, 2022
lucaspimentel
lucaspimentel reviewed Sep 12, 2022
View reviewed changes
Comment thread tracer/test/Datadog.Trace.IntegrationTests/ContainerTaggingTests.cs Outdated
andrewlock
andrewlock reviewed Sep 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.

Looks like the right approach, but I think we may need to tweak the asserts as Lucas pointed out

Comment thread tracer/test/Datadog.Trace.IntegrationTests/ContainerTaggingTests.cs Outdated
lucaspimentel
lucaspimentel reviewed Sep 12, 2022
View reviewed changes
Comment thread tracer/test/Datadog.Trace.IntegrationTests/ContainerTaggingTests.cs Outdated
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

This comment has been minimized.

Sign in to view
@andrewlock

andrewlock commented Sep 12, 2022

Copy link
Copy Markdown
Member

Benchmarks Report 🐌

Benchmarks for #3206 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 716μs 599ns 2.24μs 0.359 0 0 3.18 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 474μs 702ns 2.72μs 0 0 0 2.59 KB
#3206 WriteAndFlushEnrichedTraces net472 722μs 481ns 1.86μs 0.359 0 0 3.18 KB
#3206 WriteAndFlushEnrichedTraces netcoreapp3.1 469μs 122ns 474ns 0 0 0 2.58 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 200ns 0.0758ns 0.284ns 0.0676 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 237ns 0.352ns 1.36ns 0.00571 0 0 424 B
master AllCycleMoreComplexBody net472 185ns 0.151ns 0.567ns 0.0638 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 233ns 0.265ns 0.957ns 0.0054 0 0 400 B
master BodyExtractorSimpleBody net472 273ns 0.218ns 0.846ns 0.0573 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 225ns 0.339ns 1.27ns 0.00366 0 0 272 B
master BodyExtractorMoreComplexBody net472 14.9μs 7.36ns 28.5ns 1.21 0.0149 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 11.9μs 14.2ns 55ns 0.0888 0 0 6.75 KB
#3206 AllCycleSimpleBody net472 200ns 0.205ns 0.709ns 0.0676 0 0 425 B
#3206 AllCycleSimpleBody netcoreapp3.1 235ns 0.253ns 0.979ns 0.00593 0 0 424 B
#3206 AllCycleMoreComplexBody net472 186ns 0.212ns 0.822ns 0.0637 0 0 401 B
#3206 AllCycleMoreComplexBody netcoreapp3.1 232ns 0.397ns 1.54ns 0.00553 0 0 400 B
#3206 BodyExtractorSimpleBody net472 276ns 0.168ns 0.65ns 0.0574 0 0 361 B
#3206 BodyExtractorSimpleBody netcoreapp3.1 217ns 0.427ns 1.6ns 0.00361 0 0 272 B
#3206 BodyExtractorMoreComplexBody net472 14.9μs 6.83ns 25.5ns 1.2 0.015 0 7.62 KB
#3206 BodyExtractorMoreComplexBody netcoreapp3.1 11.8μs 15.3ns 59.3ns 0.089 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 181μs 305ns 1.18μs 0.181 0.0904 0 20.58 KB
#3206 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3206 SendRequest netcoreapp3.1 183μs 263ns 1.02μs 0.274 0 0 20.58 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.93μs 3.8ns 14.7ns 0.15 0 0 947 B
master ExecuteNonQuery netcoreapp3.1 1.44μs 2.02ns 7.57ns 0.0128 0 0 936 B
#3206 ExecuteNonQuery net472 1.85μs 2.41ns 9.33ns 0.15 0.000912 0 947 B
#3206 ExecuteNonQuery netcoreapp3.1 1.48μs 0.992ns 3.71ns 0.0126 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.67μs 1.72ns 6.65ns 0.183 0.00134 0 1.16 KB
master CallElasticsearch netcoreapp3.1 1.51μs 2.3ns 8.91ns 0.015 0 0 1.1 KB
master CallElasticsearchAsync net472 2.83μs 2.8ns 10.5ns 0.205 0 0 1.29 KB
master CallElasticsearchAsync netcoreapp3.1 1.67μs 0.548ns 2.05ns 0.0167 0 0 1.22 KB
#3206 CallElasticsearch net472 2.65μs 2.5ns 9.7ns 0.183 0 0 1.16 KB
#3206 CallElasticsearch netcoreapp3.1 1.53μs 0.556ns 2.08ns 0.0153 0 0 1.1 KB
#3206 CallElasticsearchAsync net472 2.73μs 2.12ns 8.19ns 0.205 0 0 1.29 KB
#3206 CallElasticsearchAsync netcoreapp3.1 1.63μs 0.604ns 2.34ns 0.0163 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.87μs 7.24ns 27.1ns 0.225 0 0 1.42 KB
master ExecuteAsync netcoreapp3.1 1.7μs 2.79ns 10.8ns 0.018 0 0 1.34 KB
#3206 ExecuteAsync net472 2.7μs 4.99ns 19.3ns 0.225 0 0 1.42 KB
#3206 ExecuteAsync netcoreapp3.1 1.73μs 3.54ns 13.7ns 0.0183 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.65μs 10.9ns 42.1ns 0.44 0 0 2.77 KB
master SendAsync netcoreapp3.1 3.62μs 5.27ns 20.4ns 0.0365 0 0 2.6 KB
#3206 SendAsync net472 5.65μs 8.92ns 34.5ns 0.438 0 0 2.77 KB
#3206 SendAsync netcoreapp3.1 3.6μs 7.98ns 30.9ns 0.0358 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.26μs 4.58ns 17.7ns 0.288 0 0 1.81 KB
master EnrichedLog netcoreapp3.1 2.47μs 1.65ns 5.95ns 0.0249 0 0 1.85 KB
#3206 EnrichedLog net472 3.26μs 2.21ns 8.57ns 0.287 0 0 1.81 KB
#3206 EnrichedLog netcoreapp3.1 2.58μs 2.17ns 7.82ns 0.0244 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 154μs 270ns 1.01μs 0.69 0.23 0 4.66 KB
master EnrichedLog netcoreapp3.1 118μs 314ns 1.22μs 0 0 0 4.5 KB
#3206 EnrichedLog net472 154μs 209ns 810ns 0.692 0.231 0 4.66 KB
#3206 EnrichedLog netcoreapp3.1 117μs 253ns 978ns 0.0581 0 0 4.5 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.64μs 9.11ns 35.3ns 0.569 0.00288 0 3.59 KB
master EnrichedLog netcoreapp3.1 4.49μs 7.69ns 28.8ns 0.0535 0 0 3.91 KB
#3206 EnrichedLog net472 5.75μs 9.91ns 38.4ns 0.568 0.0029 0 3.59 KB
#3206 EnrichedLog netcoreapp3.1 4.37μs 10.6ns 41ns 0.0542 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.36μs 4.55ns 17.6ns 0.218 0 0 1.37 KB
master SendReceive netcoreapp3.1 1.78μs 1.32ns 5.11ns 0.0179 0 0 1.32 KB
#3206 SendReceive net472 2.41μs 2.16ns 8.36ns 0.217 0 0 1.37 KB
#3206 SendReceive netcoreapp3.1 1.87μs 1.05ns 3.94ns 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μs 2.41ns 9.33ns 0.352 0 0 2.23 KB
master EnrichedLog netcoreapp3.1 4.24μs 3.61ns 13.5ns 0.023 0 0 1.8 KB
#3206 EnrichedLog net472 5μs 2.88ns 10.8ns 0.353 0 0 2.23 KB
#3206 EnrichedLog netcoreapp3.1 4.14μs 1.23ns 4.78ns 0.0248 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.627ns 2.43ns 0.128 0 0 810 B
master StartFinishSpan netcoreapp3.1 965ns 0.36ns 1.3ns 0.0102 0 0 760 B
master StartFinishScope net472 1.49μs 0.844ns 3.16ns 0.141 0 0 891 B
master StartFinishScope netcoreapp3.1 1.08μs 0.53ns 1.91ns 0.0119 0 0 880 B
#3206 StartFinishSpan net472 1.15μs 0.515ns 1.99ns 0.128 0 0 810 B
#3206 StartFinishSpan netcoreapp3.1 909ns 0.262ns 0.979ns 0.0105 0 0 760 B
#3206 StartFinishScope net472 1.37μs 0.852ns 3.3ns 0.141 0 0 891 B
#3206 StartFinishScope netcoreapp3.1 1.06μs 0.225ns 0.812ns 0.0117 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.44μs 0.778ns 3.01ns 0.141 0 0 891 B
master RunOnMethodBegin netcoreapp3.1 1.12μs 0.579ns 2.17ns 0.0117 0 0 880 B
#3206 RunOnMethodBegin net472 1.51μs 1.06ns 4.09ns 0.141 0 0 891 B
#3206 RunOnMethodBegin netcoreapp3.1 1.21μs 0.635ns 2.38ns 0.012 0 0 880 B

@andrewlock

andrewlock commented Sep 12, 2022

Copy link
Copy Markdown
Member

Code Coverage Report 📊

✔️ Merging #3206 into master will will increase line coverage by 1%
✔️ Merging #3206 into master will will increase branch coverage by 1%
⛔ Merging #3206 into master will will increase complexity by 92

master #3206 Change
Lines 17696 / 24369 17940 / 24517
Lines % 73% 73% 1% ✔️
Branches 10398 / 14968 10580 / 15050
Branches % 69% 70% 1% ✔️
Complexity 16169 16261 92 ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

master #3206 Change
Lines % 73% 73% 1% ✔️
Branches % 69% 70% 1% ✔️
Complexity 16169 16261 92 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Ci.GitInfo -17% ⛔ -11% ⛔ 0 ✔️
Datadog.Trace.Agent.Transports.HttpStreamRequestFactory -12% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Sink.AgentBatchUploadApi -8% ⛔ 0% ✔️ 2 ⛔
Datadog.Trace.ClrProfiler.NativeMethods -6% ⛔ -2% ⚠️ 0 ✔️
Datadog.Trace.TracerManagerFactory 3% ✔️ 6% ✔️ 2 ⛔
Datadog.Trace.Logging.DatadogSerilogLogger 4% ✔️ 6% ✔️ 0 ✔️
Datadog.Trace.Activity.DiagnosticSourceEventListener 6% ✔️ 0% ✔️ 0 ✔️
Datadog.Trace.Propagators.SpanContextPropagator 8% ✔️ 6% ✔️ 0 ✔️
Datadog.Trace.Agent.StatsAggregator 11% ✔️ 15% ✔️ -2 ✔️
Datadog.Trace.Processors.ObfuscatorTraceProcessor 12% ✔️ 15% ✔️ 0 ✔️
...And 9 more

The following classes were added in #3206:

File Line coverage Branch coverage Complexity
Datadog.Trace.Agent.DiscoveryService.AgentConfiguration 100% 100% 1
Datadog.Trace.AgentProcessManagerLoader 0% 100% 1
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.EnvironmentVariablesScrubber 100% 100% 11
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.ProcessStartCommon 85% 80% 11
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.ProcessStartIntegration 80% 50% 3
...And 3 more

View the full reports for further details:

lucaspimentel
lucaspimentel reviewed Sep 12, 2022
View reviewed changes
Comment thread tracer/test/Datadog.Trace.IntegrationTests/ContainerTaggingTests.cs
lucaspimentel
lucaspimentel approved these changes Sep 12, 2022
View reviewed changes

@lucaspimentel lucaspimentel 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!

andrewlock
andrewlock approved these changes Sep 13, 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.

Looks good, thanks for fixing it!

@OmerRaviv
OmerRaviv merged commit 65c6c4e into master Sep 13, 2022
@OmerRaviv
OmerRaviv deleted the omerr/figure-out-containerid-flakiness branch September 13, 2022 08:20
@github-actions github-actions Bot added this to the vNext milestone Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucaspimentel lucaspimentel lucaspimentel approved these changes
@andrewlock andrewlock andrewlock approved these changes

Assignees

No one assigned

Labels

area:tests unit tests, integration tests

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

3 participants

@OmerRaviv @andrewlock @lucaspimentel