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

Add instrumentation for Microsoft.Data.SqlClient for .Net Framework target #783

Closed
vhatsura opened this issue Mar 22, 2020 · 10 comments
Closed
Labels
enhancement New feature or request

Comments

@vhatsura
Copy link
Contributor

During implementation auto instrumentation for System.Data.SqlClient it was decided to skip it for Microsoft.Data.SqlClient due to issue which is described in a comment at #704 (comment).

Shortly, the issue is in the same event source names for System.Data.SqlClient and Microsoft.Data.SqlClient. It was changed in dotnet/SqlClient#399 and the event source name for Microsoft.Data.SqlClient can be found at https://github.com/JRahnama/SqlClient/blob/27f3bba874d55ef3d6670b945cc584e7cd2de69c/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs#L11. The fix is going to be released in v2.0.0-preview2 and as a result will be available in 2.0.0 GA version.

So, it means that we can add support of auto instrumentation for Microsoft.Data.SqlClient package for .Net Framework target.

I open the issue with the following intention:

  1. To not forget that we have such limitation
  2. To understand how users are interested in a feature
@vhatsura vhatsura added the enhancement New feature or request label Mar 22, 2020
@PlymouthUniversitySolDev

I've just finished integrating Elastic.Apm.SqlClient in my Asp.NET full framework project (4.7.2). I was able to piece together instructions by following the "issue" 700 and PR 784.

  1. download the nuget from your CI and add from local
  2. add the SqlClientDiagnosticSubscriber in global.asax
    As a result I am getting instrumentation for some db activity.
    Do I understand correctly in that you plan to remove support for System.Data.SqlClient and support Microsoft.Data.SqlClient going forward?

@vhatsura
Copy link
Contributor Author

@PlymouthUniversitySolDev, I'm glad to hear that you were able to integrate Elastic.Apm.SqlClient package into your solution. Also, I believe it will be easier in the near future as Elastic.Apm.SqlClient is going to be publicly available in the next release (as @gregkalapos mentioned it in #784 (review))

As for plans, there are no plans to drop support for System.Data.SqlClient, it will be supported. At the moment, only Microsoft.Data.SqlClient for .Net Framework isn't supported.

@dominikskiba
Copy link

Gents, may I ask if it's possible (and if so - how exactly?) to add SqlClientDiagnosticSubscriber to Global.asax in my case. I have a 3rd party full .net framework application and no source code. Gloabl.asax contains only this line:
<%@ Application Codebehind="Global.asax.cs" Inherits="X.Y.WebUI.Portal.Global" %>
I suppose I need to override Init() method in some way, but I'm not sure how to do it. Googled a little, but because I'm not a developer it's a little tricky.
I already use Elastic.Apm.AspNetFullFramework module with this app and I'd like to extend instrumentation with database calls.

Thanks in advance.

@vhatsura
Copy link
Contributor Author

Hey, @dominikskiba.

As I understand correctly, you already installed Elastic.Apm.AspNetFullFramework module and configured in a proper way. If so, that means you also need to download Elastic.Apm.SqlClient NuGet package from CI (to be honest, I didn't do it and it would be nice to ask for more details @gregkalapos ) or wait for public release.
After that, you can enable instrumentation for database calls by the following code:

if (Agent.IsConfigured) Agent.Subscribe(new SqlClientDiagnosticSubscriber());

As for place, where you need to put it. Yeap, it can be Init method of Global.asax.cs file, however, I cannot guarantee that will work in your case due to it depends on a codebase. Although in general, it should work and safe to do it. If you still want to do it, you need to look into Global.asax.cs file instead of Global.asax.

I hope, it will help you in your case

@gregkalapos
Copy link
Contributor

To follow up on @vhatsura's comment:

You can download the NuGet package from our CI - that's what you need to add to your app.

Also, since you say in your Global.asax you have <%@ Application Codebehind="Global.asax.cs" ... I assume you'll also have a Global.asax.cs file - it must be somewhere, since it's referenced.

Also (I know this won't help now but...) the mid-term plan is to have another IIS module that not only captures the Web Request but also turns on all other components including DB capturing. #336 is about that. So the idea is that you'll reference another package which you turn on the same way as you turned on Elastic.Apm.AspNetFullFramework, but it'll also turn on capturing everything that the agent can capture.

@dominikskiba
Copy link

Thank you @vhatsura and @gregkalapos. The trick here is I only see Global.asax that contains reference to Global.asax.cs, yet there is no Global.asax.cs file in deployed application. There is a Global class compiled in X.Y.WebUI.Portal.dll file that contains methods like Application_Start() that I would expect to find in Global.asax. Honestly I'm not sure how exactly it works without Global.asax.cs file. I guess I can approach that 3rd party and perhaps their devs can help modifying code so that it allows loading monitoring agent.
As for [#336], my application is using System.Data.SqlClient library and not EF.
I'd love to be able to enable instrumentation using only web.config file, but based on [#700] I don't think it's possible.

Dominik

@gregkalapos
Copy link
Contributor

As for [#336], my application is using System.Data.SqlClient library and not EF.
I'd love to be able to enable instrumentation using only web.config file, but based on [#700] I don't think it's possible.

@dominikskiba if you don't find any way to run code in the app, then probably the only way would be to wait until #336 is done.

@anujsethi06
Copy link

@vhatsura - were you guys able to make progress on this one, we have recently added APM to our .net full framework asp net website and would be interested in getting command text also logged, which i believe is possible with use of Microsoft.Data.SQLClient packages.But given APM Agent for full framework dont currently support it, no point of me changing to it. Can we look forward to it being there in a near future release?
if not enabled by default may be which event source to be used can be a configuration setting it takes? so that people can choose between System.Data.SqlClient or Microsoft.Data.SqlClient

@Yanotek
Copy link

Yanotek commented Dec 2, 2020

protected void Application_Start() { Task.Run(async () => { await Task.Delay(TimeSpan.FromSeconds(5)); if (Agent.IsConfigured) { Agent.Subscribe(new SqlClientDiagnosticSubscriber()); /* Enable tracing of outgoing db requests */ } }); }
It's works for me. On moment of Application_Start agent not configured yet. I'm use Asp.Net web api 2 and .net4.8.

@russcam
Copy link
Contributor

russcam commented Nov 4, 2021

Support for common SqlClient providers has been implemented as part of profiler auto instrumentation in #1534. This includes capturing command text on all target frameworks that the agent supports. Profiler auto instrumentation is scheduled to be released as beta in the next release, 1.12.

@russcam russcam closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants