Adding a flag to enable/disable collection of SQL Command text

[stebet]

Adding a boolean flag to the DependencyTrackingTelemetryModule which instructs the SQL processors to enable/disable collecting SQL command text.

Also added tests to verify the functionality.

Perhaps there is a better place for that flag than on the module itself since it's propagated down to the other classes, but it made sense to put it there.

This relates to https://github.com/microsoft/ApplicationInsights-aspnetcore/issues/980 as well as
#1282

• I ran Unit Tests locally.

[cijothomas]

Need better messaging and possibly link to doc. Else customers might that this flag alone will give full sql query.
https://docs.microsoft.com/en-us/azure/azure-monitor/app/asp-net-dependencies#advanced-sql-tracking-to-get-full-sql-query

[cijothomas]

I think we should default to False. In .NET Core we are true by default (with no option to turn it off). This is a bug and sql query collection should be always opt-in, to address privacy concerns
cc: @Dmitry-Matveev

[stebet]

Yeah I'll update the description and try to make it more clear. I'll also default it to false, but people that are maybe counting on it today would need to be made aware that this default behavior is changing then. Perhaps a post in the applicationinsights-announcements repo would be good to explain the reasoning.

[cijothomas]

Agree its a breaking change. But I could think of the original behavior as a "bug" and we are now fixing it.
Yes I can surely do a post in announcements repo to make .net core customers aware of the need to opt-in. Will do this in changelog.md for aspnetcore repo as well.

[cijothomas]

@Dmitry-Matveev @SergeyKanzhelev I am fine with this PR changes. I could use one of your review as well especially on the part where we are changing SQL Text to be opt-in for .net core customers now.

[lmolkova]

LGTM.

regarding defaults, I suggest

• keep collecting SQL command test for existing scenarios: (for System.Data.SqlClient) by default
• stop collecting command text for Microsoft.Data.SqlClient by default

[Dmitry-Matveev]

Looks good. Regarding defaults, can you please check with Dave? It may very well be a privacy requirement to have it opt-in only anyway (reqs recently were updated for several privacy considerations), in this case, yes, we're fixing a bug.

[stebet]

Distinguishing between the libraries in the collection logic would make it a bit more complicated as well, instead of just treating both libraries the same. Will look into it though if you want to split the default logic between the old and new library.

[TimothyMothra]

Adding a reminder here, we need to inform the support teams about this change.
Although it's a bug, we're changing the default behavior and this may negatively impact customers.

[stebet]

Is there anything else needed from me on this PR? Any changes required?

[cijothomas]

@stebet nothing needed from you. We are discussing internally about the default setting. Will be back with finding here. (about whether we want sqltext ON by default or OFF by default)