[SqlClient] Add db.response.returned_rows - #4625
Open
martincostello wants to merge 20 commits into
Open
Conversation
Add experimental opt-in support for `db.response.returned_rows`. Resolves open-telemetry#3898.
Address feedback to handle per-connection statistics not per query.
github-actions
Bot
added
the
comp:instrumentation.sqlclient
Add more tests to extend patch coverage.
martincostello
commented
Jul 1, 2026
martincostello
commented
Jul 1, 2026
Add PR number.
Remove redundant `using`.
Restore using, but only for `netX.0`.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds experimental, opt-in support in OpenTelemetry.Instrumentation.SqlClient to emit the db.response.returned_rows span attribute (aligned to semconv v1.42.0), with documentation and test coverage updates.
Changes:
- Add
db.response.returned_rowssemantic convention constant and implement SqlClient listener logic to populate it when enabled. - Introduce an experimental env-var toggle (
OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_RECORD_RETURNED_ROWS) and corresponding option wiring/snapshotting. - Update SqlClient semantic conventions version references to v1.42.0 and add unit/integration tests covering the new behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs | Adds unit tests for default behavior, env-var parsing, and returned-rows tagging scenarios. |
| test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs | Adds Linux Docker integration test validating returned-rows tagging end-to-end when enabled via env var. |
| test/OpenTelemetry.Instrumentation.SqlClient.Tests/MockCommandExecutor.cs | Extends mock diagnostic payloads to include Statistics for row-count scenarios. |
| test/OpenTelemetry.Instrumentation.SqlClient.Tests/FakeDbConnection.cs | Adds a lightweight fake IDbConnection for testing baseline/connection-statistics edge cases. |
| test/OpenTelemetry.Instrumentation.SqlClient.Tests/FakeDbCommand.cs | Adds a lightweight fake IDbCommand for testing returned-rows logic without real SqlClient. |
| src/Shared/SemanticConventions.cs | Adds AttributeDbResponseReturnedRows constant used by instrumentation/tests. |
| src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs | Adds env-var parsing and internal option flag for enabling returned-rows tagging. |
| src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs | Includes the new option in the options snapshot/aggregation logic. |
| src/OpenTelemetry.Instrumentation.SqlClient/README.md | Documents the experimental returned-rows feature and updates semconv links to v1.42.0. |
| src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlTelemetryHelper.cs | Updates the semantic conventions version to 1.42.0. |
| src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs | Implements statistics baseline capture + delta calculation + span tagging for returned rows. |
| src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md | Adds unreleased entries for semconv bump and returned-rows feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix bug with delegate dispatch to get the statistics.
martincostello
marked this pull request as draft
July 8, 2026 08:28
martincostello
commented
Jul 8, 2026
martincostello
commented
Jul 8, 2026
martincostello
commented
Jul 8, 2026
martincostello
commented
Jul 8, 2026
martincostello
commented
Jul 8, 2026
martincostello
commented
Jul 8, 2026
Bump to v1.43.0.
martincostello
marked this pull request as ready for review
July 8, 2026 09:04
martincostello
added
the
keep-open
matt-hensley
reviewed
Jul 9, 2026
matt-hensley
reviewed
Jul 9, 2026
- Use a single `AsyncLocal` for the counts. - Add missing check for `Activity,IsAllDataRequested`.
matt-hensley
approved these changes
Jul 13, 2026
Pull request dashboard statusWaiting on maintainers · refreshed 2026-08-14 16:20 UTC Merge when ready. Status above doesn't look right?
|
Member
Author
|
Deleted the associated branch by mistake 🤦♂️ |
- Fix issues with tracking using `AsyncLocal<T>`. - Fix broken semantic convention links. - Accept Visual Studio refactoring suggestions in touched files.
Remove redundant `IDisposable` implementation.
martincostello
enabled auto-merge
August 4, 2026 12:49
matt-hensley
approved these changes
Aug 4, 2026
martincostello
commented
Aug 5, 2026
Bump to the latest version.
matt-hensley
approved these changes
Aug 5, 2026
alanwest
approved these changes
Aug 6, 2026
Comment on lines
+271
to
+275
| > The span ends when the command finishes executing. For `ExecuteReader` this | ||
| > happens **before** the returned rows are read by the application, so the | ||
| > attribute reflects the rows observed at that point and may be `0` for queries | ||
| > whose results are consumed after the command returns. The value is most | ||
| > meaningful for `ExecuteNonQuery` (rows affected) commands. |
Member
There was a problem hiding this comment.
Does it make sense to attempt to only apply the attribute when we think it will actually be meaningful? That is, don't apply it for ExecuteReader?
Member
Author
There was a problem hiding this comment.
Made some changes based on this feedback and others.
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Aug 6, 2026
Better handle commands that don't return meaningful row counts.
- Do not include inserts, updates and deletes in returned row counts. - Add note to enable statistics for correct behaviour.
Combine note paragraphs.
matt-hensley
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3898
Changes
db.response.returned_rows.db.client.operation.durationmetric reporting an incorrect duration for commands which are not recorded as spans.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)