[DON'T MERGE][Azure Functions] Fix span parenting in isolated functions using ASP.NET Core integration#7628
Open
lucaspimentel wants to merge 29 commits intomasterfrom
Open
[DON'T MERGE][Azure Functions] Fix span parenting in isolated functions using ASP.NET Core integration#7628lucaspimentel wants to merge 29 commits intomasterfrom
lucaspimentel wants to merge 29 commits intomasterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (7628) 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 (7628) - mean (69ms) : 67, 71
master - mean (69ms) : 68, 71
section Bailout
This PR (7628) - mean (73ms) : 72, 74
master - mean (73ms) : 72, 75
section CallTarget+Inlining+NGEN
This PR (7628) - mean (1,040ms) : 1000, 1081
master - mean (1,044ms) : 990, 1098
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 (7628) - mean (108ms) : 105, 110
master - mean (108ms) : 105, 110
section Bailout
This PR (7628) - mean (108ms) : 106, 110
master - mean (108ms) : 106, 110
section CallTarget+Inlining+NGEN
This PR (7628) - mean (751ms) : 711, 791
master - mean (744ms) : 699, 789
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7628) - mean (95ms) : 93, 98
master - mean (95ms) : 93, 98
section Bailout
This PR (7628) - mean (96ms) : 95, 97
master - mean (97ms) : 95, 99
section CallTarget+Inlining+NGEN
This PR (7628) - mean (727ms) : 706, 747
master - mean (736ms) : 710, 762
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7628) - mean (94ms) : 92, 97
master - mean (94ms) : 91, 97
section Bailout
This PR (7628) - mean (95ms) : 93, 97
master - mean (95ms) : 93, 97
section CallTarget+Inlining+NGEN
This PR (7628) - mean (640ms) : 628, 652
master - mean (638ms) : 626, 651
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 (7628) - mean (195ms) : 191, 199
master - mean (192ms) : 189, 196
section Bailout
This PR (7628) - mean (199ms) : 195, 202
master - mean (197ms) : 193, 200
section CallTarget+Inlining+NGEN
This PR (7628) - mean (1,156ms) : 1096, 1215
master - mean (1,142ms) : 1096, 1188
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 (7628) - mean (278ms) : 272, 284
master - mean (276ms) : 270, 282
section Bailout
This PR (7628) - mean (280ms) : 275, 286
master - mean (276ms) : 272, 280
section CallTarget+Inlining+NGEN
This PR (7628) - mean (941ms) : 894, 988
master - mean (942ms) : 906, 978
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7628) - mean (273ms) : 264, 283
master - mean (270ms) : 264, 276
section Bailout
This PR (7628) - mean (278ms) : 268, 289
master - mean (269ms) : 265, 273
section CallTarget+Inlining+NGEN
This PR (7628) - mean (937ms) : 903, 970
master - mean (926ms) : 901, 951
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7628) - mean (271ms) : 266, 276
master - mean (267ms) : 262, 272
section Bailout
This PR (7628) - mean (273ms) : 266, 280
master - mean (267ms) : 262, 273
section CallTarget+Inlining+NGEN
This PR (7628) - mean (841ms) : 811, 870
master - mean (828ms) : 813, 844
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
12a23e2 to
0c87ab6
Compare
BenchmarksBenchmark execution time: 2026-02-26 19:46:03 Comparing candidate commit d12ffc5 in PR branch Found 9 performance improvements and 7 performance regressions! Performance is the same for 155 metrics, 21 unstable metrics. scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody net6.0
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody net472
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody netcoreapp3.1
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net472
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody netcoreapp3.1
scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs netcoreapp3.1
scenario:Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack netcoreapp3.1
scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net6.0
scenario:Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery netcoreapp3.1
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync netcoreapp3.1
scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1
scenario:Benchmarks.Trace.NLogBenchmark.EnrichedLog net6.0
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes net6.0
|
9dd669c to
27ba999
Compare
27ba999 to
584134a
Compare
24a8537 to
aae2cfb
Compare
974a81e to
613d6bb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7dc9745ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // just update the existing root span's tags to make it a "serverless" span. | ||
| // This matches the behavior for isolated worker scenario where we detect | ||
| // an existing scope and update it rather than creating a duplicate. | ||
| scope = activeScope; |
There was a problem hiding this comment.
Create a new function span when ASP.NET scope is active
Reusing activeScope here means OnIsolatedFunctionBegin can return the existing aspnet_core.request scope instead of creating azure_functions.invoke; when that happens, FunctionExecutionMiddlewareInvokeIntegration.OnAsyncMethodEnd disposes the ASP.NET scope via state.Scope, which can prematurely close the request span and drop the function-level span entirely. This is triggered whenever AsyncLocal context does flow (for example on worker/runtime variants where middleware context propagation differs), so the trace shape regresses back to missing/incorrect function spans.
Useful? React with 👍 / 👎.
| httpContext.Items[HttpContextTrackingKey] = new RequestTrackingFeature(originalPath, scope, proxyContext?.Scope); | ||
| #endif | ||
|
|
||
| if (EnvironmentHelpers.IsAzureFunctions()) |
There was a problem hiding this comment.
Cache Azure Functions detection outside request hot path
This check runs on every ASP.NET Core request, but EnvironmentHelpers.IsAzureFunctions() performs environment-variable probing each time; adding repeated env reads in request instrumentation increases per-request overhead for all apps even though the result is process-constant. Given this method is in a critical hot path, compute the flag once (startup/static cache) and reuse it.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
We are aware of this issue. I'm doing this for all the Azure Functions environment variable probes in a separate PR. Thanks.
AsyncLocal context doesn't flow through Azure Functions middleware, causing worker's azure_functions.invoke span to be incorrectly parented. Use HttpContext.Items as an explicit bridge to pass the AspNetCore scope to the Azure Functions middleware. Changes: - Store AspNetCore scope in HttpContext.Items after creation - Add Items property to IFunctionContext duck type interface - Retrieve scope from HttpContext.Items when creating azure_functions.invoke span - Only use HttpContext.Items fallback when tracer.InternalActiveScope is null 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Extracted context from gRPC propagation headers must take priority over InternalActiveScope. Enabling the AspNetCoreDiagnosticObserver in isolated workers caused InternalActiveScope to be a gRPC listener span with an unrelated trace ID, breaking host-to-worker context flow. 🤖 Co-Authored-By: Claude Code <noreply@anthropic.com>
When ASP.NET Core integration is active, check if the retrieved scope is already the active scope before creating a new span. If it's active, reuse it and update the root span tags instead of creating a duplicate azure_functions.invoke span. This prevents extra spans that break integration tests expecting specific span counts. 🤖 Co-Authored-By: Claude Code <noreply@anthropic.com>
🤖 Co-Authored-By: Claude Code <noreply@anthropic.com>
Replace direct Microsoft.AspNetCore.Http.HttpContext type reference with IHttpContextItems duck type to avoid FileNotFoundException in non-ASP.NET Core Azure Functions workers where the Microsoft.AspNetCore.Http.Abstractions assembly is not available. 🤖 Co-Authored-By: Claude Code <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Fixes incorrect span parenting in isolated Azure Functions when using ASP.NET Core integration. Worker process spans are now correctly parented to the ASP.NET Core request span, instead of being incorrectly parented to the root host span.
Reason for change
When using isolated Azure Functions with ASP.NET Core integration and HTTP proxying enabled, spans created in the worker process were being parented to the wrong span, causing disconnected or incorrectly structured traces. This made it difficult to understand the complete request flow and latency attribution.
Current (incorrect) behavior:
Fixed (correct) behavior:
Implementation details
The root cause was that
AsyncLocalcontext doesn't flow correctly through Azure Functions middleware, causing the worker'sazure_functions.invokespan to have no local parent. The instrumentation would then fall back to extracting trace context from gRPC message headers, which contained stale context (the host's root span context), resulting in incorrect parenting.The Fix: Use
HttpContext.Itemsas an explicit bridge to pass scope between ASP.NET Core and Azure Functions middleware layers:Store scope in HttpContext.Items (
AspNetCoreHttpRequestHandler.cs:159-171)aspnet_core.requestscope, store it inHttpContext.Items[HttpContextActiveScopeKey]Skip stale gRPC header extraction (
AzureFunctionsCommon.cs:243-259)"HttpRequestContext"key inFunctionContext.ItemsRetrieve scope from
HttpContext.Items(AzureFunctionsCommon.cs:287-367)tracer.InternalActiveScopeis null (AsyncLocal didn't flow), callGetAspNetCoreScope()FunctionContext.Items["HttpRequestContext"](set byFunctionsHttpProxyingMiddleware)HttpContext.Items[HttpContextActiveScopeKey]Added Items property to
IFunctionContext(IFunctionContext.cs:21)IDictionary<object, object>? Items { get; }for duck-typed access to FunctionContext.ItemsThis preserves existing behavior for non-proxying scenarios (timer triggers, non-ASP.NET Core HTTP triggers) while fixing the proxying case.
Test coverage
Covered by existing tests in
AzureFunctionsTests.cs.Fixed test snapshots to reflect correct span counts and hierarchy.
Screenshots
Before
https://dd-dotnet.datadoghq.com/apm/trace/69129102000000009bd07f7872769a84
After
https://dd-dotnet.datadoghq.com/apm/trace/69150c5500000000cb655a7e34732dd6
More recent examples with these changes:
Other details
Fixes APMSVLS-58