Add HttpContextCurrentExecutionSegmentsContainer #992
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
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Steps errors
Expand to view the steps failures
|
This commit adds an implementation of ICurrentExecutionSegmentsContainer that gets/sets the current transaction and span in HttpContext.Items, in addition to AsyncLocal<T>, making them available within the ExecutionContext of IIS. AsyncLocal<T> are not propagated across the ASP.NET ExecutionContext running in IIS, resulting in the current transaction set in Application_BeginRequest returning as null from Agent.Tracer.CurrentTransaction in later subsequent IIS events, when such events execute asynchronously. ASP.NET does however propagate the current HttpContext through HttpContext.Current, allowing values to be stored in HttpContext.Items hashtable and shared across IIS events. The HttpContextCurrentExecutionSegmentsContainer uses both AsyncLocal<T> and HttpContext.Items to get/set the current transaction and span, allowing them to be available in both IIS events and async contexts. AsyncLocal<T> is still needed for places where HttpContext.Current is null, for example, DiagnosticSourceListeners running on a separate thread. It seems it may be possible for the current transaction and current span instances stored in AsyncLocal<T> and HttpContext.Items to be different in such scenarios where one is available but not the other on beginning the transaction/span, and where availability is reversed on end. Unsure how likely this is in practice however, and using both AsyncLocal<T> and HttpContext.Items is the common approach in resolving this issue in < .NET 4.7.1. For .NET 4.7.1, a new OnExecuteRequestStep method has been added to HttpApplication, to allow the execution context to be restored/saved: https://devblogs.microsoft.com/dotnet/net-framework-4-7-1-asp-net-and-configuration-features/#asp-net-execution-step-feature Closes elastic#934 Closes elastic#972
russcam
force-pushed
the
fix/httpcontext-current-execution-segments-container
branch
from
28278da
to
6cc95b5
Compare
Codecov Report
@@ Coverage Diff @@
## master #992 +/- ##
==========================================
- Coverage 80.73% 80.04% -0.69%
==========================================
Files 138 136 -2
Lines 6742 6335 -407
==========================================
- Hits 5443 5071 -372
+ Misses 1299 1264 -35
Continue to review full report at Codecov.
|
This commit adds an integration test for HttpContextCurrentExecutionSegmentsContainer that makes multiple concurrent requests and asserts that the captured transactions and span ids align with expectations.
|
jenkins run the tests please |
russcam
deleted the
fix/httpcontext-current-execution-segments-container
branch
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.
This commit adds an implementation of
ICurrentExecutionSegmentsContainerthat gets/sets the current transaction and span inHttpContext.Items, in addition toAsyncLocal<T>, making them available within the ExecutionContext of IIS.AsyncLocal<T>are not propagated across the ASP.NET ExecutionContext running in IIS, resulting in the current transaction set inApplication_BeginRequestreturning as null fromAgent.Tracer.CurrentTransactionin later subsequent IIS events, when such events execute asynchronously. ASP.NET does however propagate the current HttpContext throughHttpContext.Current, allowing values to be stored inHttpContext.Itemshashtable and shared across IIS events. TheHttpContextCurrentExecutionSegmentsContaineruses bothAsyncLocal<T>andHttpContext.Itemsto get/set the current transaction and span, allowing them to be available in both IIS events and async contexts.AsyncLocal<T>is still needed for places whereHttpContext.Currentis null, for example, DiagnosticSourceListeners running on a separate thread in an async flow; in such casesHttpContext.Current, which is set/get onCallContext.HostContext, is not propagated over asyncflows.
It seems it may be possible for the current transaction and current span instances stored in
AsyncLocal<T>andHttpContext.Itemsto be different in such scenarios where one is available but not the other on beginningthe transaction/span, and where availability is reversed on end. Unsure how likely this is in practice however, and using both
AsyncLocal<T>andHttpContext.Itemsis the common approach in resolving this issue in < .NET 4.7.1.For .NET 4.7.1, a new
OnExecuteRequestStepmethod has been added toHttpApplication, to allow the execution context to be restored/saved:https://devblogs.microsoft.com/dotnet/net-framework-4-7-1-asp-net-and-configuration-features/#asp-net-execution-step-feature
Closes #934
Closes #972