Skip to content

RecordException in SqlClient instrumentation#1592

Merged
cijothomas merged 15 commits into
open-telemetry:masterfrom
mbakalov:sqlclient-exceptions
Dec 7, 2020
Merged

RecordException in SqlClient instrumentation#1592
cijothomas merged 15 commits into
open-telemetry:masterfrom
mbakalov:sqlclient-exceptions

Conversation

@mbakalov
Copy link
Copy Markdown
Contributor

@mbakalov mbakalov commented Nov 19, 2020

Fixes #1570.

Changes

Record exceptions as ActivityEvents in SqlClient instrumentation.

  • Add unit-tests
  • Changes in public API reviewed - there is a public API change! SqlClientInstrumentationOptions new public property RecordException (netcore only).
  • CHANGELOG.md updated for non-trivial changes

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 19, 2020

Codecov Report

Merging #1592 (dc8bf40) into master (1fb7695) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1592      +/-   ##
==========================================
+ Coverage   81.76%   81.79%   +0.02%     
==========================================
  Files         245      245              
  Lines        6587     6590       +3     
==========================================
+ Hits         5386     5390       +4     
+ Misses       1201     1200       -1     
Impacted Files Coverage Δ
...ient/Implementation/SqlClientDiagnosticListener.cs 77.94% <100.00%> (+2.18%) ⬆️
...ation.SqlClient/SqlClientInstrumentationOptions.cs 98.00% <100.00%> (+0.04%) ⬆️

@mbakalov mbakalov marked this pull request as ready for review November 30, 2020 14:11
@mbakalov mbakalov requested a review from a team November 30, 2020 14:11
@mbakalov
Copy link
Copy Markdown
Contributor Author

This is now ready for review.

Highlights of changes:

  • Added bool RecordException (default false) to the SqlClientInstrumentationOptions public API.
  • Added tests for it.
  • On netcore we are getting an actual exception so activity.RecordException() extension is used but on netfx we just stay it is either Microsoft.Data.SqlClient.SqlException or System.Data.SqlClient.SqlException based on the event source that EndExecute is coming from.
  • CHANGELOG updated

@cijothomas cijothomas merged commit 4808e05 into open-telemetry:master Dec 7, 2020
@mbakalov mbakalov deleted the sqlclient-exceptions branch February 26, 2021 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SqlClient instrumentation does not record Exception.

2 participants