getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
582 stars 206 forks source link

Sentry does not log failed db queries on EF Core 7 #2044

Open mattjohnsonpint opened 1 year ago

mattjohnsonpint commented 1 year ago

Using Entity Framework Core 7 (on either .NET 6 or .NET 7), when an error occurs in a database query, Sentry will capture the exception, but it will not correctly capture the query that caused the exception like it does on EF Core 6 and lower.

Initially this was logged as an issue with the EF Core team however later Matt concludes it's related to an issue with SqlClient when an OUTPUT clause is present (or when batch statements are executed):

Currently, diagnostic events are supported on SqlConnection, SqlCommand, and SqlTrnsaction. When an OUTPUT clause is used in an insert expression by calling the ExecuteReader() function, the exception defers to the SqlDataReader.Read() function call, which is out of scope of diagnostic support in SqlClient at the moment.

The fact we discovered this in EFCore 7 is incidental as EFCore 7 changed the way inserts are handled when capturing the inserted IDs to use OUTPUT statements rather than selecting identity records.

There's no known workaround at this time. We'll keep this issue open to track the problem in Sentry until it is resolved upstream by Microsoft.

mattjohnsonpint commented 1 year ago

Update: This might be a SqlClient issue, not an EF Core issue. Reported again at https://github.com/dotnet/SqlClient/issues/1839

bitsandfoxes commented 3 months ago

It's probably worth checking if this is still relevant.

jamescrosswell commented 2 months ago

It's probably worth checking if this is still relevant.

It's still relevant yes, and not limited to EF Core 7. It looks like this affects any batch queries or any queries making use of OUTPUT statements when using SqlClient... not great - I've chased up to see if there are any plans to fill this gap.

jamescrosswell commented 2 months ago

@bitsandfoxes Microsoft have no plans to fix this in the SqlDataReader. The issue has been marked as up for grabs but I'm not sure I'd know enough about the MS codebase to contribute personally.

I think we either leave this issue open (hoping the SqlDataReader issue gets reprioritised at some point) and/or we document the limitations of DB logging via DiagnosticListener.