DiagnosticObserver does not use null connection

[flipdoubt]

DiagnosticObserver returns rather than using a possible null SqlConnection that may come from another component.

[yang-xiaodong]

Do we consider removing the subscription key start with System.Data.SqlClient.XXXXX so that we will not receive events from System.Data.SqlClient ?

[flipdoubt]

I agree listening to a more focused subset of events sounds like the better way to go, but I'm not so familiar with this code nor how to test it. It appears that the subscription is setup in DiagnosticProcessorObserver. I searched but cannot find a different listener name using https://github.com/dotnet/SqlClient/search?q=diagnosticlistener.

[flipdoubt]

Inserting some debug output shows only the following DiagnosticListeners are registered:

• CapDiagnosticListener
• SqlClientDiagnosticListener
• SqlClientDiagnosticListener
• Microsoft.EntityFrameworkCore
• Microsoft.AspNetCore

It appears Microsoft.Data.SqlClient uses the same diagnostic listener here.

I still think guarding against a null cast has value in environments where different components use different libraries. In a truly heterogenous environment, you might have both kinds of connections.

[yang-xiaodong]

@flipdoubt Do you know what scenarios the Connection property will be null? It sounds like an impossible thing in the transaction commit process

[yang-xiaodong]

LGTM

[flipdoubt]

@flipdoubt Do you know what scenarios the Connection property will be null?

@yang-xiaodong, you are correct that the underlying connection is never null, but using as Microsoft.Data.SqlClient.SqlConnection yields null when the underlying connection is a System.Data.SqlClient.SqlConnection, such as when Hangfire creates the connection. The original (SqlConnection) GetProperty(...) throws InvalidCastException because Hangfire uses System.Data.SqlClient.SqlConnection instead of Microsoft.Data.SqlClient.SqlConnection. Guarding against null is just defensive programming where other components still use System.Data.SqlClient.SqlConnection.

Another work around would be to duplicate this code for System.Data.SqlClient.SqlConnection, but that requires referencing the "system" package.