Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propogate ExecutionContext when run in IIS #31

Closed
SergeyKanzhelev opened this issue Mar 8, 2017 · 20 comments
Closed

Propogate ExecutionContext when run in IIS #31

SergeyKanzhelev opened this issue Mar 8, 2017 · 20 comments

Comments

@SergeyKanzhelev
Copy link

Moving bug from the codeplex as it is still an issue for us with no easy solution.

If you develop OWIN middleware for monitoring you need to use ThreadAsync or CallContext to keep the context across the async/await. And it is working fine. However if you try to use the same middleware when hosting your application in IIS - it doesn't propagate the context any longer.

Even better if there will be a possibility to set the context from HttModule's Begin callback that will be preserved to the controller execution.

@davidfowl
Copy link
Member

Why not use httpContext.Items?

@SergeyKanzhelev
Copy link
Author

httpContext will not be propagated across async/await. So we need to set AsyncLocal. And if we set it in middleware or http module - it will be reset before the controller execution

@davidfowl
Copy link
Member

httpContext will not be propagated across async/await

What exactly do you mean by that?

So we need to set AsyncLocal

The IIS pipeline isn't a single asynchronous call chain and I don't know if ASP.NET captures and restores the execution context when pipeline events execute.

@SergeyKanzhelev
Copy link
Author

Sorry for mixing up the issues. I think there are two I mostly concerned of:

  1. The context set in middleware will not be propagated to the Controller execution (when running in IIS) and will propagate when running as a self host.
  2. If we will take dependency on HttpContext.Current.Items in middleware (making a special case for IIS) to set context - this context will not propagate into await part of controller when it will run any async task. So we will not be able to correlate telemetry from those parts with the initial request.

I know the limitation of HttpModule and that Begin callback will not always execute on the same managed thread as other callbacks. But it is a separate issue.

@davidfowl
Copy link
Member

The context set in middleware will not be propagated to the Controller execution (when running in IIS) and will propagate when running as a self host.

By "context" you mean async local right?

If we will take dependency on HttpContext.Current.Items in middleware (making a special case for IIS) to set context - this context will not propagate into await part of controller when it will run any async task. So we will not be able to correlate telemetry from those parts with the initial request.

I don't get that part (forgive me). If you set state in httpContext.Items, why wouldn't that flow to the controller action? Also it should be available in HttpContext.Current...

@SergeyKanzhelev
Copy link
Author

In this code condition if (System.Web.HttpContext.Current != null) be false. So I need a callback before the Controller execution where I can set the AsyncLocal context. Middleware is a perfect place, but it doesn't work in IIS as this context will be cleared by the host (actually I only checked that CallContext will be cleaned, not AsyncLocal. But I assume it will also be cleaned up. And ideally I want to be able to correlate older FWs as well).

So far the only solution I know if to register Action Filter (like I explained here). It will give the callback I need. However it looks like overkill to register both - middleware for time and other middlewares tracking and Action Filter for correlation inside the Controller.

        public async Task<ActionResult> Index()
        {
            System.Web.HttpContext.Current.Items["a"] = "ctx";

            HomeController.ctx.Value = "ctx";

            HttpClient client = new HttpClient();
            await client.GetStringAsync("http://bing.com").ContinueWith((a) => 
            {
                if (System.Web.HttpContext.Current != null)
                {
                    var ctxValue = System.Web.HttpContext.Current.Items["a"];
                }

                var alCtxValue = HomeController.ctx.Value;
            });

@davidfowl
Copy link
Member

Why not just always store the context in the httpContext? Don't use async locals when you're in System.Web...

@SergeyKanzhelev
Copy link
Author

SergeyKanzhelev commented Mar 8, 2017

Because the httpcontext is null inside the .ContinueWith. See the example above. So a lot of telemetry will not be correlated. HttpContext is using SynchronizationContext that is not propagated in tasks well.

@davidfowl
Copy link
Member

davidfowl commented Mar 8, 2017

But it does work without the .ContinueWith right?

await client.GetStringAsync("http://bing.com");
            
if (System.Web.HttpContext.Current != null)
{
    var ctxValue = System.Web.HttpContext.Current.Items["a"];
}

var alCtxValue = HomeController.ctx.Value;

@davidfowl
Copy link
Member

ContinueWith does not preserve the SynchornizationContext (which is the thing that sets httpcontext.current in ASP.NET).

@SergeyKanzhelev
Copy link
Author

yes, I have Http Context before and after the await in your example. However it's not enough.

For instance, when on FW 4.5 we collect http calls using EventSource exposed by HttpWebRequest - those event source calls happens in async context and not being correlated.

Also quite often I need to correlate exception happened in those ContinueWith.

Yes, sorry I meant SynchronizationContext, not LogicalCallContext

@davidfowl davidfowl changed the title Propogate async context when run in IIS Propogate ExecutionContext when run in IIS Mar 8, 2017
@davidfowl
Copy link
Member

I've updated the title of the bug to accurately reflect the request. I don't think it's katana's job to flow the execution context across IIS events.

@SergeyKanzhelev
Copy link
Author

Thank you! I do not know enough, but my understanding was that middleware and Controller executes in the same managed thread and Katana explicitly clear up context when in IIS. It would be great if it will not do it when possible

@davidfowl
Copy link
Member

Thank you! I do not know enough, but my understanding was that middleware and Controller executes in the same managed thread and Katana explicitly clear up context when in IIS

It doesn't have anything to do with the same thread per se but the fact that they run in different IIS events is the problem. The execution context isn't preserved between these 2 events. You can see this by looking at the "call stack" during controller execution. You'll see the middleware pipeline isn't there.

@SergeyKanzhelev
Copy link
Author

Just discussed it with @lmolkova. So re-setting the AsyncLocal in PreRequestHandlerExecute will solve the issue? If so - we can close the issue as it is a good workaround for me

@davidfowl
Copy link
Member

Yes that works if the handler completes synchronously. If it's async then it won't flow to the controller.

@muratg
Copy link

muratg commented Jun 12, 2017

@SergeyKanzhelev I assume you got unblocked, so closing this. But if that's not the case, let me know and I'll reactivate.

@muratg muratg closed this as completed Jun 12, 2017
@ysmoradi
Copy link

Using something like HttpContext.Current is not a good idea at all. It does not work outside IIS, and inside IIS it has a lot of limitations. I've configured Owin, web API, OData, signalr, entity framework with Autofac, and I pass a per request instance of my own ILogger into all classes I need logging using constructor injection, so in my continueWith call I've a direct reference to my ILogger. Combining this with DRY concept will make a very powerful approach for logging which works anywhere

@ysmoradi
Copy link

Remember that things have changed too in .NET Core. my approach is working fine on .NET Core 2 too

@maximcus
Copy link

maximcus commented Oct 25, 2018

Yes that works if the handler completes synchronously. If it's async then it won't flow to the controller.

@davidfowl , can you please elaborate on this. Did you mean that ExecutionContext can be lost between PreRequestHandlerExecute and the next step if Action (on a Controller) processing the request is an async method? If so then why?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants