[Tests] Add Azure Functions Trigger tests for messaging SDKs#8698
Conversation
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2026-06-22 08:57:30 Comparing candidate commit f28e192 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 63 known flaky benchmarks, 63 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8698) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-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 highlighted 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). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (73ms) : 69, 76
master - mean (72ms) : 68, 76
section Bailout
This PR (8698) - mean (77ms) : 73, 80
master - mean (77ms) : 73, 80
section CallTarget+Inlining+NGEN
This PR (8698) - mean (1,087ms) : 1024, 1149
master - mean (1,078ms) : 1036, 1120
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (113ms) : 108, 119
master - mean (112ms) : 107, 118
section Bailout
This PR (8698) - mean (111ms) : 108, 114
master - mean (112ms) : 107, 117
section CallTarget+Inlining+NGEN
This PR (8698) - mean (771ms) : 754, 789
master - mean (774ms) : 752, 796
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (98ms) : 93, 103
master - mean (99ms) : 95, 104
section Bailout
This PR (8698) - mean (101ms) : 96, 105
master - mean (99ms) : 96, 102
section CallTarget+Inlining+NGEN
This PR (8698) - mean (936ms) : 889, 983
master - mean (935ms) : 897, 973
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (95ms) : 92, 97
master - mean (95ms) : 92, 98
section Bailout
This PR (8698) - mean (96ms) : 94, 98
master - mean (98ms) : 92, 104
section CallTarget+Inlining+NGEN
This PR (8698) - mean (810ms) : 770, 850
master - mean (812ms) : 779, 846
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (201ms) : 195, 207
master - mean (202ms) : 195, 209
section Bailout
This PR (8698) - mean (204ms) : 199, 208
master - mean (206ms) : 199, 213
section CallTarget+Inlining+NGEN
This PR (8698) - mean (1,211ms) : 1161, 1261
master - mean (1,203ms) : 1162, 1245
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (289ms) : 280, 299
master - mean (286ms) : 278, 294
section Bailout
This PR (8698) - mean (290ms) : 284, 295
master - mean (290ms) : 283, 298
section CallTarget+Inlining+NGEN
This PR (8698) - mean (970ms) : 945, 995
master - mean (966ms) : 947, 985
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (282ms) : 275, 290
master - mean (283ms) : 276, 290
section Bailout
This PR (8698) - mean (283ms) : 277, 289
master - mean (282ms) : 278, 287
section CallTarget+Inlining+NGEN
This PR (8698) - mean (1,168ms) : 1133, 1203
master - mean (1,169ms) : 1121, 1218
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8698) - mean (281ms) : 274, 288
master - mean (282ms) : 273, 290
section Bailout
This PR (8698) - mean (281ms) : 274, 288
master - mean (281ms) : 272, 289
section CallTarget+Inlining+NGEN
This PR (8698) - mean (1,041ms) : 1002, 1080
master - mean (1,043ms) : 996, 1089
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
97b398c to
1f3465e
Compare
andrewlock
left a comment
There was a problem hiding this comment.
Just checked it build-wise - we should probably really think this to make the tool install unconditional, otherwise we risk a lot of flake in CI.
I feel like we should also do some consolidation of the azure function sample builds - if we're going to build them all in the BuildSamples stage, we should stop building them in the Windows stage.
Additionally, we may want/need to split out the azure functions tests on linux if we're going to start running them there.
Side note - this should probably be 2 separate PRs
- One to add the new sample and tests
- One changing the build
| TracerDirectory.GlobFiles("test/test-applications/integrations/**/*.csproj") | ||
| TracerDirectory.GlobFiles( | ||
| "test/test-applications/integrations/**/*.csproj", | ||
| "test/test-applications/azure-functions/**/*.csproj") |
There was a problem hiding this comment.
We have an explicit Azure Functions step, which we split out from normal integrations, so I'm not sure if we should have this here? 🤔 Or, if you do want to have this, we probably need to stop having the dedicated Azure Functions step
There was a problem hiding this comment.
This is here so we can do locally
build.sh BuildAndRunIntegrationTests --SampleName Samples.AzureFunctions.V4Isolated.Messaging
The dedicated Azure Functions step only works for windows. I think it makes sense to eventually have the windows step removed, once we no longer support in-process. But it does different things for now.
|
@andrewlock I'm gonna push back on two points
Created a draft PR showing what needs to be done: https://github.com/DataDog/dd-trace-dotnet/pull/8765/changes The in-process samples need to be treated specially. Unless we have a reason to believe sample building is considerably heavy, I'm not sure it's worth the special treatment
Why would we need to? Because of increasing resource consumption on group 2? The decision to include them as part of group 2 was intentional, because the emulators are already being booted up on group 2 for SDK integration testing |
e304cf1 to
cb5b982
Compare
| // Disable Continuous Profiler to avoid error log "Stable Configuration has not been set: the profiler was never started" | ||
| SetEnvironmentVariable("DD_PROFILING_ENABLED", "0"); | ||
| SetEnvironmentVariable("DD_PROFILING_MANAGED_ACTIVATION_ENABLED", "0"); |
There was a problem hiding this comment.
🤔 This seems like a problem, no? Either we're not working with everything published (in which case the profiler shouldn't be there) or we are working with everything published, and then this shouldn't be necessary unless there's a bug...
There was a problem hiding this comment.
It's the second case, we run the full monitoring home, the profiler is loaded. It waits for activation but never gets it (IsContinuousProfilerAvailable is false). When it shuts down without having been activated, it prints this error log, making the tests fail.
So this is an artifact of running the tests with everything, Datadog.AzureFunctions just doesn't include the profiler
andrewlock
left a comment
There was a problem hiding this comment.
This LGTM AFACIT, the only "issue" is that we need to update the VMs so that the layer is cahed, otherwise it will impact all our build times... Given we have a release next week, we should probably hold off merging it, so that we can avoid any potential issues impacting the release when we rebuild the VMs
Summary of changes
Add Azure Functions Trigger tests for messaging SDKs, enabling integration tests for AzureFunctionsCommon.
Also paves the path for future trigger integration tests.
Reason for change
The logic in AzureFunctionsCommon is complex and mainly untested (especially context extraction). Up until now, functions trigger tests were only done on windows, and without the ability to boot up the docker dependencies. This has limitations
Implementation details
Install azure functions core tools on the test image through GH releases.
Send (via http trigger) & receive (via messaging trigger) messages from the function itself so both ends are traced.
Only done for the isolated worker model.
Test coverage
Snapshot testing for Service Bus and Event Hubs, covering span links (context propagation) and parent-child relationships.