dotnet / dotnet/SqlClient

Feature | Surface batch information on the SqlClient diagnostic events

Open
#4,545 3 comments 0 reactions 0 assignees View on GitHub
Area\Open Telemetry Up-for-Grabs :raised_hands:
Dominant language
C#
Stars
989
Forks
340
Avg merge
4d 18h
Merged PRs (30d)
69

Description

### Is your feature request related to a problem? Please describe.
A `SqlBatch` is invisible to `DiagnosticSource` subscribers, so nothing outside SqlClient can distinguish a batch from a single command or determine how many operations it contained.
`SqlBatch.ExecuteNonQuery()` delegates the batch to a single private `SqlCommand` field, `_batchCommand`, which `SetupBatchCommandExecute()` puts into batch-RPC mode ([SqlBatch.cs#L89-L94](https://github.com/dotnet/SqlClient/blob/v7.0.2/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBatch.cs#L89-L94), [#L220-L240](https://github.com/dotnet/SqlClient/blob/v7.0.2/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBatch.cs#L220-L240)).

For the subscriber, it has the following consequences:
- `SqlCommand` exposes no public member revealing batch state. The operation count exists only in the private `_RPCList` field.
* `AddBatchCommand` **overwrites** `CommandText` ([SqlCommand.Batch.cs#L25](https://github.com/dotnet/SqlClient/blob/v7.0.2/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Batch.cs#L25)), making only the last statement available.

For `OpenTelemetry.Instrumentation.SqlClient` this means `db.operation.batch.size` can't be emitted.

### Describe the solution you'd like
Expose the batch on the command diagnostic payloads in `Microsoft.Data.SqlClient.Diagnostics`.
Example:
```csharp
public IReadOnlyList? BatchCommands { get; }
```
Suggested classes where this would be present: `SqlClientCommandBefore`, `SqlClientCommandAfter` and `SqlClientCommandError`.
Alternatively, only operation count could be exposed, but in that case only the last statement of the batch would be present in query text

### Describe alternatives you've considered
Use reflection to read `_RPCList` field, but the idea was rejected since any future change could break the functionality in `OpenTelemetry.Instrumentation.SqlClient`.

### Additional context
- Observed on Microsoft.Data.SqlClient 7.0.2
- The impact on `OpenTelemetry.Instrumentation.SqlClient` can be seen here ([open-telemetry/opentelemetry-dotnet-contrib#2240](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/2240))
- This looks like the extension case from #2210 comment: https://github.com/dotnet/SqlClient/issues/2210#issuecomment-1802069961
- Happy to create a PR myself if the team is open to it.

Contributor guide

Open the contributing guide

Research direction

Start with SqlBatch.cs, especially SetupBatchCommandExecute(), and SqlCommand.Batch.cs at AddBatchCommand to understand how batch commands are represented. Then inspect SqlClientCommandBefore, SqlClientCommandAfter, and SqlClientCommandError in Microsoft.Data.SqlClient.Diagnostics. Done means diagnostic subscribers can distinguish batches and access their operation information without reflection.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, sql
Domain
database, observability
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
56/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.