SqlClient db metrics #2026
SqlClient db metrics #2026
Conversation
github-actions
bot
added
the
comp:instrumentation.sqlclient
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
| { | ||
| if (this.exceptionFetcher.TryFetch(payload, out Exception? exception) && exception != null) | ||
| { | ||
| tags.Add(SemanticConventions.AttributeErrorType, exception.Message); |
There was a problem hiding this comment.
the message has high cardinality, this should be the full name of the exception type.
We also added db.response.status_code recently - ptal at some details on how to capture it
https://github.com/open-telemetry/semantic-conventions/blob/v1.28.0/docs/database/sql.md
| { | ||
| if (hasError) | ||
| { | ||
| var exceptionType = activity.Tags.SingleOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType); |
There was a problem hiding this comment.
The exception.type attribute is currently never set on the activity because it is a new attribute and the trace instrumentation has not yet been adapted to the new conventions.
Was your plan to update the trace instrumentation to follow the new conventions in this PR? Or were you looking to split the work up some how?
|
|
||
| if (!string.IsNullOrEmpty(connectionDetails.Port)) | ||
| { | ||
| // TODO: Should we continue to emit this if the default port (1433) is being used? |
There was a problem hiding this comment.
No, it should not be set if it is the default. See the following footnote from the conventions
[11]: If using a port other than the default port for this DBMS and if server.address is set.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Changes
Opening draft PR for feedback.
Some TODOs, including flaky tests, likely due to state.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes