SqlClient public API renaming proposal for db.statement properties (#1645)

* Rename dbstatement api bools

* Docs update

* Clarify netfx behavior.

* Markdown cleanup

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
This commit is contained in:
mbakalov 2021-01-20 14:20:16 -05:00 committed by GitHub
parent c7e5f07037
commit 97ed5f54d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 75 additions and 54 deletions

View File

@ -3,8 +3,8 @@ OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.EnableCo
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.EnableConnectionLevelAttributes.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Enrich.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetStatementText.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetStatementText.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatement.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatement.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SqlClientInstrumentationOptions() -> void
OpenTelemetry.Trace.TracerProviderBuilderExtensions
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddSqlClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions> configureSqlClientInstrumentationOptions = null) -> OpenTelemetry.Trace.TracerProviderBuilder

View File

@ -3,8 +3,8 @@ OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.EnableCo
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.EnableConnectionLevelAttributes.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Enrich.get -> System.Action<System.Diagnostics.Activity, string, object>
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Enrich.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetStatementText.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetStatementText.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatement.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatement.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SqlClientInstrumentationOptions() -> void
OpenTelemetry.Trace.TracerProviderBuilderExtensions
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddSqlClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions> configureSqlClientInstrumentationOptions = null) -> OpenTelemetry.Trace.TracerProviderBuilder

View File

@ -5,10 +5,10 @@ OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Enrich.g
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Enrich.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.RecordException.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.RecordException.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetStoredProcedureCommandName.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetStoredProcedureCommandName.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetTextCommandContent.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetTextCommandContent.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatementForStoredProcedure.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatementForStoredProcedure.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatementForText.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatementForText.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SqlClientInstrumentationOptions() -> void
OpenTelemetry.Trace.TracerProviderBuilderExtensions
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddSqlClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions> configureSqlClientInstrumentationOptions = null) -> OpenTelemetry.Trace.TracerProviderBuilder

View File

@ -6,8 +6,10 @@
on .NET Framework.
([#1599](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1599))
* SqlClientInstrumentationOptions API changes: `SetStoredProcedureCommandName`
and `SetTextCommandContent` are now only available on .NET Core. On .NET
Framework they are replaced by a single `SetStatementText` property.
and `SetTextCommandContent` have been renamed to
`SetDbStatementForStoredProcedure` and `SetDbStatementForText`. They are now
only available on .NET Core. On .NET Framework they are replaced by a single
`SetDbStatement` property.
* On .NET Framework, "db.statement_type" attribute is no longer set for
activities created by the instrumentation.
* New setting on SqlClientInstrumentationOptions on .NET Core: `RecordException`

View File

@ -93,7 +93,7 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Implementation
{
case CommandType.StoredProcedure:
activity.SetTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.StoredProcedure));
if (this.options.SetStoredProcedureCommandName)
if (this.options.SetDbStatementForStoredProcedure)
{
activity.SetTag(SemanticConventions.AttributeDbStatement, (string)commandText);
}
@ -102,7 +102,7 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Implementation
case CommandType.Text:
activity.SetTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.Text));
if (this.options.SetTextCommandContent)
if (this.options.SetDbStatementForText)
{
activity.SetTag(SemanticConventions.AttributeDbStatement, (string)commandText);
}

View File

@ -144,7 +144,7 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Implementation
this.options.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity);
string commandText = (string)eventData.Payload[3];
if (!string.IsNullOrEmpty(commandText) && this.options.SetStatementText)
if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatement)
{
activity.SetTag(SemanticConventions.AttributeDbStatement, commandText);
}

View File

@ -60,61 +60,78 @@ For an ASP.NET application, adding instrumentation is typically done in the
This instrumentation can be configured to change the default behavior by using
`SqlClientInstrumentationOptions`.
### SetStoredProcedureCommandName (.NET Core)
### Capturing 'db.statement'
By default, when CommandType is CommandType.StoredProcedure this
instrumentation will set the `db.statement` attribute to the stored procedure
command name. This behavior can be disabled by setting the
`SetStoredProcedureCommandName` to false.
The `SqlClientInstrumentationOptions` class exposes several properties that can be
used to configure how the [`db.statement`](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md#call-level-attributes)
attribute is captured upon execution of a query.
The following example shows how to use `SetStoredProcedureCommandName`.
#### .NET Core - SetDbStatementForStoredProcedure and SetDbStatementForText
On .NET Core, two properties are available: `SetDbStatementForStoredProcedure`
and `SetDbStatementForText`. These properties control capturing of
`CommandType.StoredProcedure` and `CommandType.Text` respectively.
`SetDbStatementForStoredProcedure` is _true_ by default and will set
[`db.statement`](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md#call-level-attributes)
attribute to the stored procedure command name.
`SetDbStatementForText` is _false_ by default (to prevent accidental capture of
sensitive data that might be part of the SQL statement text). When set to
`true`, the instrumentation will set [`db.statement`](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md#call-level-attributes)
attribute to the text of the SQL command being executed.
To disable capturing stored procedure commands use configuration like below.
```csharp
using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddSqlClientInstrumentation(
options => options.SetStoredProcedureCommandName = false)
options => options.SetDbStatementForStoredProcedure = false)
.AddConsoleExporter()
.Build();
```
### SetTextCommandContent (.NET Core)
By default, when CommandType is CommandType.Text, this instrumentation will not
set the `db.statement` attribute. This behavior can be enabled by setting
`SetTextCommandContent` to true.
The following example shows how to use `SetTextCommandContent`.
To enable capturing of `sqlCommand.CommandText` for `CommandType.Text` use the
following configuration.
```csharp
using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddSqlClientInstrumentation(
options => options.SetTextCommandContent = true)
options => options.SetDbStatementForText = true)
.AddConsoleExporter()
.Build();
```
## SetStatementText (.NET Framework)
#### .NET Framework - SetDbStatement
For .NET Framework, `SetTextCommandContent` and `SetStoredProcedureCommandName`
are not available. Instead, `SetStatementText` should be used to control whether
this instrumentation should set the `db.statement` attribute to the text of the
`SqlCommand` being executed.
For .NET Framework, `SetDbStatementForStoredProcedure` and
`SetDbStatementForText` are not available. Instead, a single `SetDbStatement`
property should be used to control whether this instrumentation should set the
[`db.statement`](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md#call-level-attributes)
attribute to the text of the `SqlCommand` being executed. This could either be
a name of a stored procedure or a full text of a `CommandType.Text` query.
Text capturing is _disabled_ by default. If enabled, the instrumentation will
capture both `CommandType.Text` and `CommandType.StoredProcedure` when using
[`Microsoft.Data.SqlClient`](https://www.nuget.org/packages/Microsoft.Data.SqlClient/),
and only `CommandType.StoredProcedure` when using `System.Data.SqlClient`.
On .NET Framwork, unlike .NET Core, the instrumentation capabilities for both
[`Microsoft.Data.SqlClient`](https://www.nuget.org/packages/Microsoft.Data.SqlClient/)
and `System.Data.SqlClient` are limited:
To turn statement capturing on, use the options like in below example. Be
aware that `CommandType.Text` SQL might contain sensitive data.
On [`Microsoft.Data.SqlClient`](https://www.nuget.org/packages/Microsoft.Data.SqlClient/)
only set this to `true` if you are absolutely sure that you are using
exclusively stored procedures, or have no sensitive data in your `sqlCommand.CommandText`.
* [`Microsoft.Data.SqlClient`](https://www.nuget.org/packages/Microsoft.Data.SqlClient/)
always exposes both the stored procedure name and the full query text but
doesn't allow for more granular control to turn either on/off depending on
`CommandType`.
* `System.Data.SqlClient` only exposes stored procedure names and not the full
query text.
Since `CommandType.Text` might contain sensitive data, all SQL capturing is
_disabled_ by default to protect against accidentally sending full query text
to a telemetry backend. If you are only using stored procedures or have no
sensitive data in your `sqlCommand.CommandText`, you can enable SQL capturing
using the options like below:
```csharp
using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddSqlClientInstrumentation(
options => options.SetStatementText = true)
options => options.SetDbStatement = true)
.AddConsoleExporter()
.Build();
```
@ -140,7 +157,7 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.Build();
```
### Enrich
## Enrich
This option, available on .NET Core only, allows one to enrich the activity
with additional information from the raw `SqlCommand` object. The `Enrich`
@ -193,3 +210,5 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder()
## References
* [OpenTelemetry Project](https://opentelemetry.io/)
* [OpenTelemetry semantic conventions for database calls](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md)

View File

@ -80,17 +80,17 @@ namespace OpenTelemetry.Instrumentation.SqlClient
/// When using <c>System.Data.SqlClient</c>, the instrumentation will only capture <c>sqlCommand.CommandText</c> for <see cref="CommandType.StoredProcedure"/> commands.
/// </para>
/// </remarks>
public bool SetStatementText { get; set; }
public bool SetDbStatement { get; set; }
#else
/// <summary>
/// Gets or sets a value indicating whether or not the <see cref="SqlClientInstrumentation"/> should add the names of <see cref="CommandType.StoredProcedure"/> commands as the <see cref="SemanticConventions.AttributeDbStatement"/> tag. Default value: True.
/// </summary>
public bool SetStoredProcedureCommandName { get; set; } = true;
public bool SetDbStatementForStoredProcedure { get; set; } = true;
/// <summary>
/// Gets or sets a value indicating whether or not the <see cref="SqlClientInstrumentation"/> should add the text of <see cref="CommandType.Text"/> commands as the <see cref="SemanticConventions.AttributeDbStatement"/> tag. Default value: False.
/// </summary>
public bool SetTextCommandContent { get; set; }
public bool SetDbStatementForText { get; set; }
#endif
/// <summary>

View File

@ -91,11 +91,11 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Tests
.AddSqlClientInstrumentation(options =>
{
#if !NETFRAMEWORK
options.SetStoredProcedureCommandName = captureStoredProcedureCommandName;
options.SetTextCommandContent = captureTextCommandContent;
options.SetDbStatementForStoredProcedure = captureStoredProcedureCommandName;
options.SetDbStatementForText = captureTextCommandContent;
options.RecordException = recordException;
#else
options.SetStatementText = captureStoredProcedureCommandName;
options.SetDbStatement = captureStoredProcedureCommandName;
#endif
if (shouldEnrich)
{
@ -165,8 +165,8 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Tests
.AddSqlClientInstrumentation(
(opt) =>
{
opt.SetTextCommandContent = captureTextCommandContent;
opt.SetStoredProcedureCommandName = captureStoredProcedureCommandName;
opt.SetDbStatementForText = captureTextCommandContent;
opt.SetDbStatementForStoredProcedure = captureStoredProcedureCommandName;
if (shouldEnrich)
{
opt.Enrich = ActivityEnrichment;

View File

@ -55,7 +55,7 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Tests
.AddProcessor(activityProcessor.Object)
.AddSqlClientInstrumentation(options =>
{
options.SetStatementText = captureText;
options.SetDbStatement = captureText;
})
.Build();
@ -112,7 +112,7 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Tests
.AddProcessor(activityProcessor.Object)
.AddSqlClientInstrumentation(options =>
{
options.SetStatementText = captureText;
options.SetDbStatement = captureText;
options.EnableConnectionLevelAttributes = enableConnectionLevelAttributes;
})
.Build();