[SqlClient] Add support for Filter expression

[Driedas]

Changes

Adds support for using a Filter expression for SqlClient Intrumentation to optionally filter out telemetry based on the current SqlCommand. Heavily inspired by the AspNetCore Instrumentation filter implementation. Updated the readme.md file with a short description and a sample.

@cijothomas Initially intended as completion of PR 3342 but started from scratch instead.

• Appropriate CHANGELOG.md updated for non-trivial changes
• Design discussion issue #
• Changes in public API reviewed

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Driedas / name: Pavel Steinl (87fc4b6)

[codecov]

Codecov Report

Merging #3743 (48e069e) into main (1a2103d) will increase coverage by 0.16%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3743      +/-   ##
==========================================
+ Coverage   87.18%   87.34%   +0.16%     
==========================================
  Files         277      277              
  Lines       10209    10228      +19     
==========================================
+ Hits         8901     8934      +33     
+ Misses       1308     1294      -14     

Impacted Files	Coverage Δ
...ient/Implementation/SqlClientDiagnosticListener.cs	85.00% <100.00%> (+2.39%)	⬆️
...lementation/SqlClientInstrumentationEventSource.cs	75.00% <100.00%> (+10.29%)	⬆️
...ation.SqlClient/SqlClientInstrumentationOptions.cs	98.66% <100.00%> (+0.01%)	⬆️
src/OpenTelemetry/ProviderExtensions.cs	81.81% <0.00%> (-9.10%)	⬇️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	73.52% <0.00%> (-5.89%)	⬇️
...Telemetry/Metrics/PeriodicExportingMetricReader.cs	72.72% <0.00%> (-5.46%)	⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs	97.65% <0.00%> (+0.78%)	⬆️
src/OpenTelemetry/BatchExportProcessor.cs	84.11% <0.00%> (+1.86%)	⬆️
...metryProtocol/Implementation/ActivityExtensions.cs	95.13% <0.00%> (+3.24%)	⬆️
...ZPages/Implementation/ZPagesExporterEventSource.cs	62.50% <0.00%> (+6.25%)	⬆️
... and 3 more

[vishweshbankwar]

@cijothomas @utpilla - Could you please approve running the workflow?

[cijothomas]

@Driedas Can you resolve the conflict?

[cijothomas]

Thanks for your contribution! Changes looks good to me.

[Driedas]

@Driedas Can you resolve the conflict?

Github doesn't let me do it via its UI (even though it says it's been resolved), I can rebase the whole PR to HEAD main though...

Also, not sure what to do with those 2 failing checks, neither of those seem related to the work I've done and all tests pass locally on my machine 🤷‍♂️

[vishweshbankwar]

@Driedas Can you resolve the conflict?

Github doesn't let me do it via its UI (even though it says it's been resolved), I can rebase the whole PR to HEAD main though...

Also, not sure what to do with those 2 failing checks, neither of those seem related to the work I've done and all tests pass locally on my machine 🤷‍♂️

@Driedas - The failing check is from linux environment.
Failed OpenTelemetry.Instrumentation.SqlClient.Tests.SqlClientTests.ShouldNotCollectTelemetryWhenFilterEvaluatesToFalse [91 ms]
Error Message:
System.PlatformNotSupportedException : LocalDB is not supported on this platform.
Stack Trace:
at Microsoft.Data.SqlClient.SNI.LocalDB.GetLocalDBConnectionString(String localDbInstance)
at Microsoft.Data.SqlClient.SNI.SNIProxy.GetLocalDBDataSource(String fullServerName, Boolean& error)
at Microsoft.Data.SqlClient.SNI.SNIProxy.CreateConnectionHandle(Object callbackObject, String fullServerName, Boolean ignoreSniOpenTimeout, Int64

For the merge conflicts - you could resolve it locally and push changes.

[Driedas]

@vishweshbankwar ok, will resolve locally, but what do we do about that failing test? Should I replace the connection string with SqlConnectionString that the SuccessfulCommandTest is using? Is that a database that is available in the CI environment?

[vishweshbankwar]

@vishweshbankwar ok, will resolve locally, but what do we do about that failing test? Should I replace the connection string with SqlConnectionString that the SuccessfulCommandTest is using? Is that a database that is available in the CI environment?

@Driedas I think you can copy this test into a new one to include filter.

[Driedas]

@vishweshbankwar Done, replaced the implementation of the test helper method, should now hopefully run on Linux as well

[vishweshbankwar]

LGTM

[Driedas]

@vishweshbankwar also, squashed it all into a single commit and updated to latest main to fix the README.md conflict

[Driedas]

@vishweshbankwar I've moved the private helper further down in the class, as per the SA1204 error, and rebased to latest main...

[cijothomas]

https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.AspNetCore#filter the wording here is slightly better. Not blocking. There are plans to change the return type from bool to more easy-to-use Enums like DropActivity, ReportActivity etc, soon.

[cijothomas]

Hold off from merging, until @utpilla completes a release today. This PR itself is good, but we might want to make some breaking changes to the Filter signature across all libraries. If that happens, want to do it in one shot.