Skip to content

Commit

Permalink
[Sql Instrumentation] Unify exposed public API (#3900)
Browse files Browse the repository at this point in the history
* Unify public api exposed in sql instrumentation package.

* CHANGELOG patch.

* Code review.

* README updates.

* README tweaks.
  • Loading branch information
CodeBlanch authored Nov 14, 2022
1 parent 018dc9c commit f770047
Show file tree
Hide file tree
Showing 8 changed files with 187 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ public class HttpClientInstrumentationOptions
/// collect telemetry about requests on a per request basis.
/// </summary>
/// <remarks>
/// Notes:
/// <list type="bullet">
/// <item><b>FilterHttpRequestMessage is only executed on .NET and .NET
/// <para><b>FilterHttpRequestMessage is only executed on .NET and .NET
/// Core runtimes. <see cref="HttpClient"/> and <see
/// cref="HttpWebRequest"/> on .NET and .NET Core are both implemented
/// using <see cref="HttpRequestMessage"/>.</b></item>
/// using <see cref="HttpRequestMessage"/>.</b></para>
/// Notes:
/// <list type="bullet">
/// <item>The return value for the filter function is interpreted as:
/// <list type="bullet">
/// <item>If filter returns <see langword="true" />, the request is
Expand All @@ -51,38 +51,32 @@ public class HttpClientInstrumentationOptions
public Func<HttpRequestMessage, bool> FilterHttpRequestMessage { get; set; }

/// <summary>
/// Gets or sets an action to enrich an Activity with <see cref="HttpRequestMessage"/>.
/// Gets or sets an action to enrich an <see cref="Activity"/> with <see cref="HttpRequestMessage"/>.
/// </summary>
/// <remarks>
/// <para><b>EnrichWithHttpRequestMessage is only executed on .NET and .NET
/// Core runtimes. <see cref="HttpClient"/> and <see
/// cref="HttpWebRequest"/> on .NET and .NET Core are both implemented
/// using <see cref="HttpRequestMessage"/>.</b></para>
/// <para><see cref="Activity"/>: the activity being enriched.</para>
/// <para><see cref="HttpRequestMessage"/> object from which additional information can be extracted to enrich the activity.</para>
/// </remarks>
public Action<Activity, HttpRequestMessage> EnrichWithHttpRequestMessage { get; set; }

/// <summary>
/// Gets or sets an action to enrich an Activity with <see cref="HttpResponseMessage"/>.
/// Gets or sets an action to enrich an <see cref="Activity"/> with <see cref="HttpResponseMessage"/>.
/// </summary>
/// <remarks>
/// <para><b>EnrichWithHttpResponseMessage is only executed on .NET and .NET
/// Core runtimes. <see cref="HttpClient"/> and <see
/// cref="HttpWebRequest"/> on .NET and .NET Core are both implemented
/// using <see cref="HttpRequestMessage"/>.</b></para>
/// <para><see cref="Activity"/>: the activity being enriched.</para>
/// <para><see cref="HttpResponseMessage"/> object from which additional information can be extracted to enrich the activity.</para>
/// </remarks>
public Action<Activity, HttpResponseMessage> EnrichWithHttpResponseMessage { get; set; }

/// <summary>
/// Gets or sets an action to enrich an Activity with <see cref="Exception"/>.
/// Gets or sets an action to enrich an <see cref="Activity"/> with <see cref="Exception"/>.
/// </summary>
/// <remarks>
/// <para><b>EnrichWithException is called for all runtimes.</b></para>
/// <para><see cref="Activity"/>: the activity being enriched.</para>
/// <para><see cref="Exception"/> object from which additional information can be extracted to enrich the activity.</para>
/// </remarks>
public Action<Activity, Exception> EnrichWithException { get; set; }

Expand All @@ -91,12 +85,12 @@ public class HttpClientInstrumentationOptions
/// collect telemetry about requests on a per request basis.
/// </summary>
/// <remarks>
/// Notes:
/// <list type="bullet">
/// <item><b>FilterHttpWebRequest is only executed on .NET Framework
/// <para><b>FilterHttpWebRequest is only executed on .NET Framework
/// runtimes. <see cref="HttpClient"/> and <see cref="HttpWebRequest"/>
/// on .NET Framework are both implemented using <see
/// cref="HttpWebRequest"/>.</b></item>
/// cref="HttpWebRequest"/>.</b></para>
/// Notes:
/// <list type="bullet">
/// <item>The return value for the filter function is interpreted as:
/// <list type="bullet">
/// <item>If filter returns <see langword="true" />, the request is
Expand All @@ -109,36 +103,37 @@ public class HttpClientInstrumentationOptions
public Func<HttpWebRequest, bool> FilterHttpWebRequest { get; set; }

/// <summary>
/// Gets or sets an action to enrich an Activity with <see cref="HttpWebRequest"/>.
/// Gets or sets an action to enrich an <see cref="Activity"/> with <see cref="HttpWebRequest"/>.
/// </summary>
/// <remarks>
/// <para><b>EnrichWithHttpWebRequest is only executed on .NET Framework
/// runtimes. <see cref="HttpClient"/> and <see cref="HttpWebRequest"/>
/// on .NET Framework are both implemented using <see
/// cref="HttpWebRequest"/>.</b></para>
/// <para><see cref="Activity"/>: the activity being enriched.</para>
/// <para><see cref="HttpWebRequest"/> object from which additional information can be extracted to enrich the activity.</para>
/// </remarks>
public Action<Activity, HttpWebRequest> EnrichWithHttpWebRequest { get; set; }

/// <summary>
/// Gets or sets an action to enrich an Activity with <see cref="HttpWebResponse"/>.
/// Gets or sets an action to enrich an <see cref="Activity"/> with <see cref="HttpWebResponse"/>.
/// </summary>
/// <remarks>
/// <para><b>EnrichWithHttpWebResponse is only executed on .NET Framework
/// runtimes. <see cref="HttpClient"/> and <see cref="HttpWebRequest"/>
/// on .NET Framework are both implemented using <see
/// cref="HttpWebRequest"/>.</b></para>
/// <para><see cref="Activity"/>: the activity being enriched.</para>
/// <para><see cref="HttpWebResponse"/> object from which additional information can be extracted to enrich the activity.</para>
/// </remarks>
public Action<Activity, HttpWebResponse> EnrichWithHttpWebResponse { get; set; }

/// <summary>
/// Gets or sets a value indicating whether exception will be recorded as an <see cref="ActivityEvent"/> or not.
/// Gets or sets a value indicating whether exception will be recorded
/// as an <see cref="ActivityEvent"/> or not. Default value: <see
/// langword="false"/>.
/// </summary>
/// <remarks>
/// See: <see href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md" />.
/// <para><b>RecordException is supported on all runtimes.</b></para>
/// <para>For specification details see: <see
/// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md"
/// />.</para>
/// </remarks>
public bool RecordException { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ 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.SetDbStatement.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.SetDbStatement.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Filter.get -> System.Func<object, bool>
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.Filter.set -> void
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.RecordException.get -> bool
OpenTelemetry.Instrumentation.SqlClient.SqlClientInstrumentationOptions.RecordException.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
8 changes: 8 additions & 0 deletions src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

## Unreleased

* **Breaking change**: The same API is now exposed for `net462` and
`netstandard2.0` targets. `SetDbStatement` has been removed. Use
`SetDbStatementForText` to capture command text and stored procedure names on
.NET Framework. Note: `Enrich`, `Filter`, `RecordException`, and
`SetDbStatementForStoredProcedure` options are NOT supported on .NET
Framework.
([#3900](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3900))

## 1.0.0-rc9.9

Released 2022-Nov-07
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

#if NETFRAMEWORK
using System;
using System.Diagnostics;
Expand All @@ -30,13 +31,9 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Implementation
/// We hook into these event sources and process their BeginExecute/EndExecute events.
/// </summary>
/// <remarks>
/// Note that before version 2.0.0, Microsoft.Data.SqlClient used "Microsoft-AdoNet-SystemData"
/// EventSource (same as System.Data.SqlClient), but since 2.0.0 has switched to "Microsoft.Data.SqlClient.EventSource".
///
/// Due to the limitation of the "Microsoft-AdoNet-SystemData", it is not possible to capture sql statement text
/// for CommandType.Text when using that EventSource. It only reports text for CommandType.StoredProcedure.
///
/// "Microsoft.Data.SqlClient.EventSource" doesn't have that issue.
/// Note that before version 2.0.0, Microsoft.Data.SqlClient used
/// "Microsoft-AdoNet-SystemData" (same as System.Data.SqlClient), but since
/// 2.0.0 has switched to "Microsoft.Data.SqlClient.EventSource".
/// </remarks>
internal sealed class SqlEventSourceListener : EventListener
{
Expand Down Expand Up @@ -115,8 +112,12 @@ private void OnBeginExecute(EventWrittenEventArgs eventData)
[3] -> CommandText
Note:
- For "Microsoft-AdoNet-SystemData": [3] CommandText = (CommandType == CommandType.StoredProcedure ? CommandText : string.Empty;
- For "Microsoft.Data.SqlClient.EventSource": [3] CommandText = sqlCommand.CommandText (so it is set for all command types).
- For "Microsoft-AdoNet-SystemData" v1.0: [3] CommandText = CommandType == CommandType.StoredProcedure ? CommandText : string.Empty; (so it is set for only StoredProcedure command types)
(https://github.com/dotnet/SqlClient/blob/v1.0.19239.1/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs#L6369)
- For "Microsoft-AdoNet-SystemData" v1.1: [3] CommandText = sqlCommand.CommandText (so it is set for all command types)
(https://github.com/dotnet/SqlClient/blob/v1.1.0/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs#L7459)
- For "Microsoft.Data.SqlClient.EventSource" v2.0+: [3] CommandText = sqlCommand.CommandText (so it is set for all command types).
(https://github.com/dotnet/SqlClient/blob/f4568ce68da21db3fe88c0e72e1287368aaa1dc8/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs#L6641)
*/

if ((eventData?.Payload?.Count ?? 0) < 4)
Expand Down Expand Up @@ -148,7 +149,7 @@ private void OnBeginExecute(EventWrittenEventArgs eventData)
this.options.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity);

string commandText = (string)eventData.Payload[3];
if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatement)
if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatementForText)
{
activity.SetTag(SemanticConventions.AttributeDbStatement, commandText);
}
Expand Down
88 changes: 44 additions & 44 deletions src/OpenTelemetry.Instrumentation.SqlClient/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,20 @@ 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`.

### Capturing 'db.statement'
### Capturing database statements

The `SqlClientInstrumentationOptions` class exposes several properties that can
be used to configure how the
The `SqlClientInstrumentationOptions` class exposes two properties that can be
used to configure how the
[`db.statement`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md#call-level-attributes)
attribute is captured upon execution of a query.
attribute is captured upon execution of a query but the behavior depends on the
runtime used.

#### .NET Core - SetDbStatementForStoredProcedure and SetDbStatementForText
#### .NET and .NET Core

On .NET Core, two properties are available: `SetDbStatementForStoredProcedure`
and `SetDbStatementForText`. These properties control capturing of
`CommandType.StoredProcedure` and `CommandType.Text` respectively.
On .NET and .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/main/specification/trace/semantic_conventions/database.md#call-level-attributes)
Expand Down Expand Up @@ -115,25 +117,16 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.Build();
```

#### .NET Framework - SetDbStatement
#### .NET Framework

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
On .NET Framework, the `SetDbStatementForText` property controls whether or not
this instrumentation will set the
[`db.statement`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/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.

On .NET Framework, 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:

* [`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.
attribute to the text of the `SqlCommand` being executed. This could either be
the name of a stored procedure (when `CommandType.StoredProcedure` is used) or
the full text of a `CommandType.Text` query. `SetDbStatementForStoredProcedure`
is ignored because on .NET Framework there is no way to determine the type of
command being executed.

Since `CommandType.Text` might contain sensitive data, all SQL capturing is
_disabled_ by default to protect against accidentally sending full query text to
Expand All @@ -144,12 +137,14 @@ using the options like below:
```csharp
using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddSqlClientInstrumentation(
options => options.SetDbStatement = true)
options => options.SetDbStatementForText = true)
.AddConsoleExporter()
.Build();
```

## EnableConnectionLevelAttributes
### EnableConnectionLevelAttributes

**Note:** EnableConnectionLevelAttributes is supported on all runtimes.

By default, `EnabledConnectionLevelAttributes` is disabled and this
instrumentation sets the `peer.service` attribute to the
Expand All @@ -170,13 +165,14 @@ 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` action is
called only when `activity.IsAllDataRequested` is `true`. It contains the
activity itself (which can be enriched), the name of the event, and the actual
raw object.
**Note:** Enrich is supported on .NET and .NET Core runtimes only.

This option can be used to enrich the activity with additional information from
the raw `SqlCommand` object. The `Enrich` action is called only when
`activity.IsAllDataRequested` is `true`. It contains the activity itself (which
can be enriched), the name of the event, and the actual raw object.

Currently there is only one event name reported, "OnCustom". The actual object
is `Microsoft.Data.SqlClient.SqlCommand` for `Microsoft.Data.SqlClient` and
Expand Down Expand Up @@ -207,8 +203,10 @@ access to `SqlCommand` object.

### RecordException

This option, available on .NET Core only, can be set to instruct the
instrumentation to record SqlExceptions as Activity
**Note:** RecordException is supported on .NET and .NET Core runtimes only.

This option can be set to instruct the instrumentation to record SqlExceptions
as Activity
[events](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md).

The default value is `false` and can be changed by the code like below.
Expand All @@ -221,16 +219,18 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.Build();
```

## Filter
### Filter

**Note:** Filter is supported on .NET and .NET Core runtimes only.

This option allows to filter out activities based on the properties of the
`SqlCommand` object being instrumented using a `Func<object, bool>`.
The function receives an instance of the raw `SqlCommand` and should return
`true` if the telemetry is to be collected, and `false` if it should not.
The parameter of the Func delegate is of type `object` and needs to
be cast to the appropriate type of `SqlCommand`, either
`Microsoft.Data.SqlClient.SqlCommand` or `System.Data.SqlClient.SqlCommand`.
The example below filters out all commands that are not stored procedures.
This option can be used to filter out activities based on the properties of the
`SqlCommand` object being instrumented using a `Func<object, bool>`. The
function receives an instance of the raw `SqlCommand` and should return `true`
if the telemetry is to be collected, and `false` if it should not. The parameter
of the Func delegate is of type `object` and needs to be cast to the appropriate
type of `SqlCommand`, either `Microsoft.Data.SqlClient.SqlCommand` or
`System.Data.SqlClient.SqlCommand`. The example below filters out all commands
that are not stored procedures.

```csharp
using var traceProvider = Sdk.CreateTracerProviderBuilder()
Expand Down
Loading

0 comments on commit f770047

Please sign in to comment.