Fix flakiness in Azure Functions #6524
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Datadog ReportBranch report: ✅ 0 Failed, 236471 Passed, 2085 Skipped, 19h 53m 28.9s Total Time New Flaky Tests (1)
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing the following branches/commits: Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6524) - mean (69ms) : 64, 73
. : milestone, 69,
master - mean (69ms) : 66, 72
. : milestone, 69,
section CallTarget+Inlining+NGEN
This PR (6524) - mean (977ms) : 955, 1000
. : milestone, 977,
master - mean (980ms) : 955, 1005
. : milestone, 980,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6524) - mean (107ms) : 104, 110
. : milestone, 107,
master - mean (107ms) : 106, 109
. : milestone, 107,
section CallTarget+Inlining+NGEN
This PR (6524) - mean (676ms) : 657, 694
. : milestone, 676,
master - mean (678ms) : 662, 693
. : milestone, 678,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6524) - mean (91ms) : 89, 93
. : milestone, 91,
master - mean (91ms) : 89, 93
. : milestone, 91,
section CallTarget+Inlining+NGEN
This PR (6524) - mean (632ms) : 618, 646
. : milestone, 632,
master - mean (639ms) : 624, 655
. : milestone, 639,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6524) - mean (194ms) : 190, 198
. : milestone, 194,
master - mean (195ms) : 190, 200
. : milestone, 195,
section CallTarget+Inlining+NGEN
This PR (6524) - mean (1,101ms) : 1065, 1136
. : milestone, 1101,
master - mean (1,107ms) : 1079, 1136
. : milestone, 1107,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6524) - mean (278ms) : 273, 283
. : milestone, 278,
master - mean (281ms) : 276, 286
. : milestone, 281,
section CallTarget+Inlining+NGEN
This PR (6524) - mean (869ms) : 842, 896
. : milestone, 869,
master - mean (878ms) : 850, 906
. : milestone, 878,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6524) - mean (268ms) : 263, 272
. : milestone, 268,
master - mean (269ms) : 265, 273
. : milestone, 269,
section CallTarget+Inlining+NGEN
This PR (6524) - mean (852ms) : 820, 885
. : milestone, 852,
master - mean (855ms) : 820, 891
. : milestone, 855,
|
andrewlock
force-pushed
the
andrew/more-azure-function-futzing
branch
from
da6c7ec to
53843db
Compare
Benchmarks Report for tracer 🐌Benchmarks for #6524 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️
|
| Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑netcoreapp3.1 | 1.129 | 995.89 | 881.87 |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunOnMethodBegin |
net6.0 | 653ns | 0.406ns | 1.57ns | 0.00984 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
netcoreapp3.1 | 996ns | 0.69ns | 2.67ns | 0.00903 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
net472 | 1.05μs | 0.725ns | 2.71ns | 0.105 | 0 | 0 | 658 B |
| #6524 | RunOnMethodBegin |
net6.0 | 601ns | 0.266ns | 1.03ns | 0.00995 | 0 | 0 | 696 B |
| #6524 | RunOnMethodBegin |
netcoreapp3.1 | 882ns | 0.965ns | 3.74ns | 0.00925 | 0 | 0 | 696 B |
| #6524 | RunOnMethodBegin |
net472 | 1.12μs | 1.06ns | 4.12ns | 0.104 | 0 | 0 | 658 B |
Otherwise we run into issues with the multi api build
andrewlock
force-pushed
the
andrew/more-azure-function-futzing
branch
from
53843db to
b730aeb
Compare
andrewlock
changed the title
andrewlock
added
area:builds
andrewlock
changed the title
kevingosse
approved these changes
Jan 10, 2025
bouwkast
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Attempt to fix flakiness in Azure Functions multi-sample build
Reason for change
#6472 added multi-version building for Azure Functions samples. Unfortunately, this caused the build stage to be flaky, because Azure Functions apparently generates and builds a completely separate .csproj. Unfortunately, it also ignores all the artifact output stuff, which means when we restore/build multiple versions of the app (with different api versions, it stomps over itself.
e.g. we can see the sample app build itself doing the right thing:
But there's also the "WorkerExtensions" thing which is going completely off the rails, ignores all of the Directory.Build.props etc and just does its own thing
Unfortunately, this shows it's trying to build two separate versions to the same location, and things break.
Implementation details
My initial attempt in #6521 failed.
In this attempt I split the v1 SDK and v2 SDK into two separate projects. The SDK versions are tracked separately. I was thinking that as long as we only build a single version per project we should be ok.
It was not OK.
So in the end I tore out the Azure Functions version tracking. We could/should consider adding it back in some way, but right now this is causing too many issues
An alternative is to just remove the Azure Functions samples from the "auto-updating multi version".
I kept the split between v1 and v2 of the Azure Functions SDK to give us a little more coverage; I think it's about the best we can do.
Test coverage
If this all finally passes, we should be good.
Other details
An important thing to note is that currently, even though we will now get dependabot notifactions about updates to the azure functions libs, these aren't tested automatically.
That's because currently our testing on Windows does not run the "multi version" tests like we do on Linux. And we only test Azure functions on Windows.
This is something I'd like to address longer term with PRs like #6498, but for the meantime, we'll need to manually keep the samples up to date with the latest SDK version when there's a dependabot PR