search

elastic / apm-agent-dotnet

https://www.elastic.co/guide/en/apm/agent/dotnet/current/index.html
Apache License 2.0
582 stars 209 forks source link

Full Framework: Support ASP.NET (IIS hosted) #83

Closed SergeyKleyman closed 4 years ago

SergeyKleyman commented 5 years ago

It should be possible to monitor ASP.NET (IIS hosted) applications using full framework with the following additional requirements:

Subtasks:

SergeyKleyman commented 5 years ago

Created child issues:

SergeyKleyman commented 5 years ago

We agreed that for initial implementation of monitoring Full Framework (#83) we should try to find a way to monitor Entity Framework 6 (#335) instead ADO.NET (#114).

sangchatb commented 5 years ago

Thanks for the work you're putting into supporting the IIS Hosted services. @HonzaKral mentioned I should reach out to y'all. We're adding APM to our services that are written in a variety of languages.

We have quite a few in IIS hosted dotNet services such as WCF and Web API 2. I've pulled master and I'm testing the HttpModule with WCF and Web API hosted on IIS for the Full dotNet Framework.

It appears the AsyncLocal value is not available in Service or Controller functions respectively. Based on issues in other projects (https://github.com/aspnet/AspNetKatana/issues/31) , it appears IIS doesn't synchronize the AsyncLocal context. The behavior I've observed is that when Transactions is set in the HTTP module or in Owin Middleware, the value is null in the service function or controller function.

I've tested creating a transaction which sets AsyncLocal higher in the stack, such as in an ActionFilter, and that appears to work. I'm looking for feedback to see if y'all have encountered this issue and have a work around or for guidance addressing it. I'm testing another solution that makes TransactionContainer injected, so that ambient transactions storage can be configured. In this case, I'm testing a version that stores transactions in HttpContext.Current.Items, when available, and AsyncLocal. I may have more feedback then.

I'm also willing to contribute. In addition to addressing this issue, I'm adding helper functions to manage spans that measure IDbCommand executions and pull driver specific information without having a reference to the specific drivers, e.g. OracleConnection.DataSource.

The tests performed were on WCF and Web API 2 projects created around 2016. System.Net.Http is redirected to 4.0.0 from 4.2.0 and the dot Net Framework has been upgraded to 4.6.1 from 4.5.

SergeyKleyman commented 5 years ago

@sangchatb First of all thank you for your interest and contributions are very welcome. Unfortunately we didn't have enough time to focus on legacy .NET Framework technologies so far and we only tested Elastic.Apm.AspNetFullFramework.ElasticApmModule on a simple ASP.NET MVC application so we didn't test on IIS hosted WCF and WebAPI applications. It's definitely in our short term backlog. If you would like to contribute in this area - any and all of feature/enhancement implementations, tests, sample applications would be great.

BTW regarding

I'm testing another solution that makes TransactionContainer injected, so that ambient transactions storage can be configured

We have already laid some groundwork in upcoming PR - https://github.com/elastic/apm-agent-dotnet/pull/391/files#diff-b9ca0e610fac5d34034c2efd38fee0f3R24 . We will have to see how to extend it so that we can cover ways to implement ambient context for current transaction/span by means other than AsyncLocal.

sangchatb commented 5 years ago

@SergeyKleyman I've explored a few options. I think I have 2 viable ideas. The first is tested. The second is only an idea.

  1. IIS uses HttpContext to manage state for a given request. AsyncLocal context is lost in the IIS integrated pipeline. This behavior affects HttpModules and Owin/Katana middleware. HTTPContext is available to MVC, WebAPI and WCF but not for any async Tasks generated in those frameworks. An ActionFilter (MVC, Web API) or a MessageInspector (WCF) can synchronize the HttpContext and AsyncLocal transaction before any async Tasks are generated in user code. The ActionFilter or MessageInspector is applied to all controllers or services, respectively. Action Filters and MessageInspectors also allow transactions names to be derived from Class and Method Names instead of URLs. Using this method, I have Web API and WCF logging correctly. I backed out the modifications to TransactionContainer for this test. CurrentTransaction is being set with reflection since it has a protection level of internal.
  2. IHttpModule introduces OnExecuteRequestStep in the .Net Framework 4.7.1. It looks to be designed to solve this problem. It should allow the synchronization step to occur in the HttpModule. I didn't opt to test this due to the framework version requirement. The target framework may not have to be 4.7.1, it may be 4.6.1. The function may be callable using reflection allowing developers to target 4.6.1 if 4.7.1 is installed.

Also, MVC/WebAPI and WCF both have hosting options other than IIS, though that's outside the scope of this issue. Those frameworks have OwinContext and OperationContext, respectively. Those contexts can provide the HTTP transaction info. I suspect AsyncLocal would be preserved in those environments due to the lack of the IIS Integrated Pipeline avoiding the need to store the Transaction in either context.

On a side note, I had to install System.Diagnostics.DiagnosticSource version 4.5.1 over 4.0.0 for the outgoing HTTP requests to be logged.

SergeyKleyman commented 5 years ago

@sangchatb I'm not sure I can follow all the details from your comment but it's to be expected since it's quite technical and natural language is not the best fit to convey technical aspects. If you think you found a solid solution how do you feel about a PR or PR-s? We can start with sample application showcasing all the relevant use cases and then proceed to the implementation or we can do it in one PR - whichever way you find more convenient. The final step would be to add automated tests (I assume as part of test\Elastic.Apm.AspNetFullFramework.Tests) but we can discuss it when we get there. I think we should start by opening a dedicated issue for this feature - what we want is to implement support for automatic tracing of IIS hosted WCF and WebAPI applications, right?

SergeyKleyman commented 4 years ago

Closing this issue as all the child issues are done.

Created a follow-up issue - https://github.com/elastic/apm-agent-dotnet/issues/602 to address the problem of the current transaction being null for ASP.NET Web API mentioned above.

kodigo35 commented 4 years ago

Hello, still having issues with WCF support on 1.2 release. currentTransaction remains null due to asyncLocal is there a workaround for this issue plz ?

i fixed it temporarly using httpcontext.current to store CurrentTransaction and CurrentSpan.