Skip to content

AspNet: manage scopes in HttpContext.Current.Items#356

Merged
colin-higgins merged 3 commits intoDataDog:developfrom
ksjoberg:aspnet-scopemanager-fix
May 20, 2019
Merged

AspNet: manage scopes in HttpContext.Current.Items#356
colin-higgins merged 3 commits intoDataDog:developfrom
ksjoberg:aspnet-scopemanager-fix

Conversation

@ksjoberg
Copy link
Contributor

This addresses the issue in #336.

There are several pieces that could/should be improved upon, perhaps mainly in the way in which the default IScopeManager is replaced. Up for discussion is also whether AspNetScopeManager should do the fallback the way it currently does it.

@ksjoberg ksjoberg requested a review from a team as a code owner May 17, 2019 11:30
The AspNetScopeManager also stores the scope in AsyncLocal/CallContext to handle async code.
@ksjoberg ksjoberg force-pushed the aspnet-scopemanager-fix branch from b230ca8 to 68c9dff Compare May 17, 2019 11:39
@lucaspimentel lucaspimentel added area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:bug labels May 17, 2019
@lucaspimentel
Copy link
Member

Thanks, @ksjoberg! Someone from the team will review this soon.

@colin-higgins colin-higgins self-assigned this May 20, 2019
@colin-higgins
Copy link
Member

Thanks for the contribution @ksjoberg
I added a test for web forms that verifies nested async calls work as we expect still.
Your changes have sparked some discussion here about how to handle what you mentioned with changing out IScopeManager and how the fallbacks should work.
I will be creating a PR to adjust our API for this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants