diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt index 75d53ce70e..a667c87386 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt @@ -4,10 +4,6 @@ OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentation OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.Filter.set -> void OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbQueryParameters.get -> bool OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbQueryParameters.set -> void -OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForStoredProcedure.get -> bool -OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForStoredProcedure.set -> void -OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForText.get -> bool -OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForText.set -> void OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.get -> System.Action? OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.set -> void OpenTelemetry.Trace.TracerProviderBuilderExtensions diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index c50af0c433..df15b2fcb0 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -21,9 +21,13 @@ * Add the `db.query.summary` attribute and use it for the trace span name when opted into using the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. ([#3022](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3022)) -* The `db.statement` and `db.query.text` attributes added when `SetDbStatementForText` - is `true` are now sanitized when using specific SQL-like EFCore providers. +* The `db.statement` and `db.query.text` attributes are now sanitized when using + specific SQL-like EFCore providers. ([#3022](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3022)) +* **Breaking change**: The `SetDbStatementForStoredProcedure` and + `SetDbStatementForText` properties have been removed. Behaviors related to this + option are now always enabled. + ([#3072](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3072)) ## 1.12.0-beta.2 diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs index a2dd0abfec..4d473a1bd9 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs @@ -28,16 +28,6 @@ internal EntityFrameworkInstrumentationOptions(IConfiguration configuration) this.EmitNewAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.New); } - /// - /// Gets or sets a value indicating whether or not the should add the names of commands as the tag. Default value: True. - /// - public bool SetDbStatementForStoredProcedure { get; set; } = true; - - /// - /// Gets or sets a value indicating whether or not the should add the text of commands as the tag. Default value: False. - /// - public bool SetDbStatementForText { get; set; } - /// /// Gets or sets an action to enrich an Activity from the db command. /// diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 7ba0b0ceab..6ea6d94efc 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -165,32 +165,24 @@ public override void OnEventWritten(string name, object? payload) switch (commandType) { case CommandType.StoredProcedure: - if (this.options.SetDbStatementForStoredProcedure) - { - DatabaseSemanticConventionHelper.ApplyConventionsForStoredProcedure( - activity, - commandText, - this.options.EmitOldAttributes, - this.options.EmitNewAttributes); - } - + DatabaseSemanticConventionHelper.ApplyConventionsForStoredProcedure( + activity, + commandText, + this.options.EmitOldAttributes, + this.options.EmitNewAttributes); break; case CommandType.Text: - if (this.options.SetDbStatementForText) - { - // Only SQL-like providers support sanitization as we are not - // able to sanitize arbitrary commands for other query dialects. - bool sanitizeQuery = IsSqlLikeProvider(providerName); - - DatabaseSemanticConventionHelper.ApplyConventionsForQueryText( - activity, - commandText, - this.options.EmitOldAttributes, - this.options.EmitNewAttributes, - sanitizeQuery); - } - + // Only SQL-like providers support sanitization as we are not + // able to sanitize arbitrary commands for other query dialects. + bool sanitizeQuery = IsSqlLikeProvider(providerName); + + DatabaseSemanticConventionHelper.ApplyConventionsForQueryText( + activity, + commandText, + this.options.EmitOldAttributes, + this.options.EmitNewAttributes, + sanitizeQuery); break; case CommandType.TableDirect: diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/README.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/README.md index 86be4c08bd..c37d0850ca 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/README.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/README.md @@ -141,29 +141,6 @@ services.AddOpenTelemetry() .AddConsoleExporter()); ``` -### SetDbStatementForText - -`SetDbStatementForText` controls whether the `db.statement` attribute is -emitted. The behavior of `SetDbStatementForText` depends on the runtime used, -see below for more details. - -Query text may contain sensitive data, so when `SetDbStatementForText` is -enabled the raw query text is sanitized by automatically replacing literal -values with a `?` character. - -> [!NOTE] -> Query sanitization is only supported for the following SQL-like providers: -> -> * Firebird -> * GCP Spanner -> * IBM DB2 -> * Microsoft SQL Server -> * MySQL -> * Oracle -> * PostgreSQL -> * SQLite -> * Teradata - ### SetDbQueryParameters `SetDbQueryParameters` controls whether `db.query.parameter.` attributes @@ -183,7 +160,7 @@ following configuration. ```csharp using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddEntityFrameworkCoreInstrumentation( - options => options.SetDbStatementForText = true) + options => options.SetDbQueryParameters = true) .AddConsoleExporter() .Build(); ``` diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Shipped.txt b/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Shipped.txt index e69de29bb2..7dc5c58110 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Shipped.txt +++ b/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Shipped.txt @@ -0,0 +1 @@ +#nullable enable diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Unshipped.txt index 0b71d2e8ba..3aeba4a8c5 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Unshipped.txt @@ -8,8 +8,6 @@ OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.Rec OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.RecordException.set -> void OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.SetDbQueryParameters.get -> bool OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.SetDbQueryParameters.set -> void -OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.SetDbStatementForText.get -> bool -OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.SetDbStatementForText.set -> void OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.SqlClientTraceInstrumentationOptions() -> void OpenTelemetry.Metrics.SqlClientMeterProviderBuilderExtensions OpenTelemetry.Trace.TracerProviderBuilderExtensions diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/net8.0/PublicAPI.Shipped.txt b/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/net8.0/PublicAPI.Shipped.txt new file mode 100644 index 0000000000..7dc5c58110 --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/net8.0/PublicAPI.Shipped.txt @@ -0,0 +1 @@ +#nullable enable diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md index 50b81ada7c..b83af79aa0 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md @@ -10,6 +10,10 @@ listener. ([#3041](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3041)) +* **Breaking change**: The `SetDbStatementForText` property has been removed. + Behaviors related to this option are now always enabled. + ([#3072](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3072)) + ## 1.12.0-beta.2 Released 2025-Jul-15 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs index 3f2699f255..c2c773ffe7 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs @@ -162,15 +162,11 @@ public override void OnEventWritten(string name, object? payload) break; case CommandType.Text: - if (options.SetDbStatementForText) - { - DatabaseSemanticConventionHelper.ApplyConventionsForQueryText( - activity, - commandText, - options.EmitOldAttributes, - options.EmitNewAttributes); - } - + DatabaseSemanticConventionHelper.ApplyConventionsForQueryText( + activity, + commandText, + options.EmitOldAttributes, + options.EmitNewAttributes); break; case CommandType.TableDirect: diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs index a18c9a8015..db739dc25e 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs @@ -157,7 +157,7 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) if (activity.IsAllDataRequested) { var commandText = (string)eventData.Payload[3]; - if (!string.IsNullOrEmpty(commandText) && options.SetDbStatementForText) + if (!string.IsNullOrEmpty(commandText)) { var sqlStatementInfo = SqlProcessor.GetSanitizedSql(commandText); if (options.EmitOldAttributes) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/README.md b/src/OpenTelemetry.Instrumentation.SqlClient/README.md index d8678b6811..0e0f635265 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/README.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/README.md @@ -121,76 +121,6 @@ For an ASP.NET application, adding instrumentation is typically done in the This instrumentation can be configured to change the default behavior by using `SqlClientTraceInstrumentationOptions`. -### SetDbStatementForText - -`SetDbStatementForText` controls whether the `db.statement` attribute is -emitted. The behavior of `SetDbStatementForText` depends on the runtime used, -see below for more details. - -Query text may contain sensitive data, so when `SetDbStatementForText` is -enabled the raw query text is sanitized by automatically replacing literal -values with a `?` character. - -#### .NET - -On .NET, `SetDbStatementForText` controls whether or not -this instrumentation will set the -[`db.statement`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes) -attribute to the `CommandText` of the `SqlCommand` being executed when the `CommandType` -is `CommandType.Text`. The -[`db.statement`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes) -attribute is always captured for `CommandType.StoredProcedure` because the `SqlCommand.CommandText` -only contains the stored procedure name. - -`SetDbStatementForText` is _false_ by default. When set to `true`, the -instrumentation will set -[`db.statement`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes) -attribute to the text of the SQL command being executed. - -To enable capturing of `SqlCommand.CommandText` for `CommandType.Text` use the -following configuration. - -```csharp -using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddSqlClientInstrumentation( - options => options.SetDbStatementForText = true) - .AddConsoleExporter() - .Build(); -``` - -#### .NET Framework - -On .NET Framework, there is no way to determine the type of command being -executed, so the `SetDbStatementForText` property always controls whether -or not this instrumentation will set the -[`db.statement`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes) -attribute to the `CommandText` of the `SqlCommand` being executed. The -`CommandText` could be the name of a stored procedure (when -`CommandType.StoredProcedure` is used) or the full text of a `CommandType.Text` -query. - -`SetDbStatementForText` is _false_ by default. When set to `true`, the -instrumentation will set -[`db.statement`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes) -attribute to the text of the SQL command being executed. - -To enable capturing of `SqlCommand.CommandText` for `CommandType.Text` use the -following configuration. - -```csharp -using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddSqlClientInstrumentation( - options => options.SetDbStatementForText = true) - .AddConsoleExporter() - .Build(); -``` - -> [!NOTE] -> When using the built-in `System.Data.SqlClient`, only stored procedure command -names will be captured. To capture query text, other command text and -stored procedure command names, you need to use the `Microsoft.Data.SqlClient` -NuGet package (v1.1+). - ### SetDbQueryParameters > [!NOTE] @@ -199,10 +129,10 @@ NuGet package (v1.1+). `SetDbQueryParameters` controls whether `db.query.parameter.` attributes are emitted. -Query parameters may contain sensitive data, so only enable `SetDbStatementForText` +Query parameters may contain sensitive data, so only enable `SetDbQueryParameters` if your queries and/or environment are appropriate for enabling this option. -`SetDbQueryParameters` is _false_ by default. When set to `true`, the +`SetDbQueryParameters` is `false` by default. When set to `true`, the instrumentation will set [`db.query.parameter.`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#span-definition) attributes for each of the query parameters associated with a database command. @@ -213,7 +143,7 @@ following configuration. ```csharp using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSqlClientInstrumentation( - options => options.SetDbStatementForText = true) + options => options.SetDbQueryParameters = true) .AddConsoleExporter() .Build(); ``` @@ -221,7 +151,7 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder() ### Enrich > [!NOTE] -> Enrich is supported on .NET and .NET Core runtimes only. +> Enrich is available on .NET 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 @@ -258,7 +188,7 @@ access to `SqlCommand` object. ### RecordException > [!NOTE] -> RecordException is supported on .NET and .NET Core runtimes only. +> RecordException is available on .NET runtimes only. This option can be set to instruct the instrumentation to record SqlExceptions as Activity @@ -277,7 +207,7 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder() ### Filter > [!NOTE] -> Filter is supported on .NET and .NET Core runtimes only. +> Filter is available on .NET runtimes only. This option can be used to filter out activities based on the properties of the `SqlCommand` object being instrumented using a `Func`. The diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs index 21dc0ef74f..ee2d8f7a90 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs @@ -1,13 +1,11 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System.Data; using System.Diagnostics; using Microsoft.Extensions.Configuration; #if NET using OpenTelemetry.Instrumentation.SqlClient.Implementation; #endif -using OpenTelemetry.Trace; using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.SqlClient; @@ -49,43 +47,12 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration) #endif } - /// - /// Gets or sets a value indicating whether or not the - /// should add the text of commands as the tag. - /// Default value: . - /// - /// - /// - /// WARNING: SetDbStatementForText will capture the raw - /// CommandText. Make sure your CommandText property never - /// contains any sensitive data. - /// - /// SetDbStatementForText is supported on all runtimes. - /// - /// On .NET and .NET Core SetDbStatementForText only applies to - /// SqlCommands with . - /// On .NET Framework SetDbStatementForText applies to all - /// SqlCommands regardless of . - /// - /// When using System.Data.SqlClient use - /// SetDbStatementForText to capture StoredProcedure command - /// names. - /// When using Microsoft.Data.SqlClient use - /// SetDbStatementForText to capture Text, StoredProcedure, and all - /// other command text. - /// - /// - /// - /// - public bool SetDbStatementForText { get; set; } - /// /// Gets or sets an action to enrich an with the /// raw SqlCommand object. /// /// - /// Enrich is only executed on .NET and .NET Core - /// runtimes. + /// Enrich is only executed on .NET runtimes. /// The parameters passed to the enrich action are: /// /// The being enriched. @@ -103,8 +70,7 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration) /// collect telemetry about a command. /// /// - /// Filter is only executed on .NET and .NET Core - /// runtimes. + /// Filter is only executed on .NET runtimes. /// Notes: /// /// The first parameter passed to the filter function is the raw @@ -126,8 +92,7 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration) /// langword="false"/>. /// /// - /// RecordException is only supported on .NET and .NET Core - /// runtimes. + /// RecordException is only supported on .NET runtimes. /// For specification details see: . /// diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs index 063fb7c9b0..b38034b159 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs @@ -42,7 +42,7 @@ public EntityFrameworkIntegrationTests( this.sqlServerFixture = fixture; } - public static TheoryData RawSqlTestCases() + public static TheoryData RawSqlTestCases() { (string, Type, bool, string, string)[] providers = [ @@ -56,34 +56,31 @@ public EntityFrameworkIntegrationTests( (SqlServerProvider, typeof(SqlCommand), true, "microsoft.sql_server", "master"), ]; - var testCases = new TheoryData(); + var testCases = new TheoryData(); foreach ((var provider, var commandType, var useNewConventions, var system, var database) in providers) { - var expectedSpanNameWhenCaptureTextCommandContent = useNewConventions + var expectedSpanName = useNewConventions ? "select" : database; - testCases.Add(provider, "select 1/1", false, false, useNewConventions, commandType, database, system, database, null); - testCases.Add(provider, "select 1/1", true, false, useNewConventions, commandType, expectedSpanNameWhenCaptureTextCommandContent, system, database, null); + testCases.Add(provider, "select 1/1", false, useNewConventions, commandType, expectedSpanName, system, database, null); // For some reason, SQLite does not throw an exception for division by zero if (provider == PostgresProvider) { - testCases.Add(provider, "select 1/0", false, true, useNewConventions, commandType, database, system, database, "22012: division by zero"); - testCases.Add(provider, "select 1/0", true, true, useNewConventions, commandType, expectedSpanNameWhenCaptureTextCommandContent, system, database, "22012: division by zero"); + testCases.Add(provider, "select 1/0", true, useNewConventions, commandType, expectedSpanName, system, database, "22012: division by zero"); } else if (provider == SqlServerProvider) { - testCases.Add(provider, "select 1/0", false, true, useNewConventions, commandType, database, system, database, "Divide by zero error encountered."); - testCases.Add(provider, "select 1/0", true, true, useNewConventions, commandType, expectedSpanNameWhenCaptureTextCommandContent, system, database, "Divide by zero error encountered."); + testCases.Add(provider, "select 1/0", true, useNewConventions, commandType, expectedSpanName, system, database, "Divide by zero error encountered."); } } return testCases; } - public static TheoryData DataContextTestCases() + public static TheoryData DataContextTestCases() { (string, Type, string, string)[] providers = [ @@ -95,23 +92,20 @@ public static TheoryData DataCon bool[] trueFalse = [true, false]; - var testCases = new TheoryData(); + var testCases = new TheoryData(); foreach ((var provider, var commandType, var system, var database) in providers) { - foreach (var captureTextCommandContent in trueFalse) + foreach (var shouldEnrich in trueFalse) { - foreach (var shouldEnrich in trueFalse) - { - testCases.Add(provider, captureTextCommandContent, shouldEnrich, false, commandType, system, database); - } + testCases.Add(provider, shouldEnrich, false, commandType, system, database); } } return testCases; } - public static TheoryData QuerySanitizationTestCases() + public static TheoryData QuerySanitizationTestCases() { string[] providers = [ @@ -121,14 +115,12 @@ public static TheoryData DataCon SqlServerProvider, ]; - var testCases = new TheoryData(); + var testCases = new TheoryData(); foreach (var provider in providers) { - testCases.Add(provider, "select 1/1", false, false, null, null); - testCases.Add(provider, "select 1/1", false, true, null, null); - testCases.Add(provider, "select 1/1", true, false, "select ?/?", null); - testCases.Add(provider, "select 1/1", true, true, "select ?/?", "select"); + testCases.Add(provider, "select 1/1", false, "select ?/?", null); + testCases.Add(provider, "select 1/1", true, "select ?/?", "select"); } return testCases; @@ -139,7 +131,6 @@ public static TheoryData DataCon public async Task TracesRawSql( string provider, string commandText, - bool captureTextCommandContent, bool isFailure, bool useNewConventions, Type expectedCommandType, @@ -173,15 +164,8 @@ bool ActivityFilter(string? providerName, IDbCommand command) var activities = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddInMemoryExporter(activities) - .AddSqlClientInstrumentation(options => - { - options.SetDbStatementForText = captureTextCommandContent; - }) - .AddEntityFrameworkCoreInstrumentation(options => - { - options.Filter = ActivityFilter; - options.SetDbStatementForText = captureTextCommandContent; - }) + .AddSqlClientInstrumentation() + .AddEntityFrameworkCoreInstrumentation(options => options.Filter = ActivityFilter) .Build(); var optionsBuilder = new DbContextOptionsBuilder(); @@ -208,7 +192,6 @@ bool ActivityFilter(string? providerName, IDbCommand command) Assert.NotNull(activity); VerifyActivityData( - captureTextCommandContent, isFailure, conventions, expectedSpanName, @@ -243,7 +226,6 @@ bool ActivityFilter(string? providerName, IDbCommand command) [MemberData(nameof(DataContextTestCases))] public async Task TracesDataContext( string provider, - bool captureTextCommandContent, bool shouldEnrich, bool useNewConventions, Type expectedCommandType, @@ -278,14 +260,10 @@ bool ActivityFilter(string? providerName, IDbCommand command) var activities = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddInMemoryExporter(activities) - .AddSqlClientInstrumentation(options => - { - options.SetDbStatementForText = captureTextCommandContent; - }) + .AddSqlClientInstrumentation() .AddEntityFrameworkCoreInstrumentation(options => { options.Filter = ActivityFilter; - options.SetDbStatementForText = captureTextCommandContent; if (shouldEnrich) { options.EnrichWithIDbCommand = ActivityEnrichment; @@ -364,14 +342,11 @@ await context.Database.ExecuteSqlRawAsync( [EnabledOnDockerPlatformTheory(DockerPlatform.Linux)] [MemberData(nameof(QuerySanitizationTestCases))] - [InlineData(SqlServerProvider, "sp_who", false, false, null, null)] - [InlineData(SqlServerProvider, "sp_who", false, true, null, null)] - [InlineData(SqlServerProvider, "sp_who", true, false, "sp_who", null)] - [InlineData(SqlServerProvider, "sp_who", true, true, "sp_who", null)] + [InlineData(SqlServerProvider, "sp_who", false, "sp_who", null)] + [InlineData(SqlServerProvider, "sp_who", true, "sp_who", null)] public async Task SqlQueriesAreSanitized( string provider, string commandText, - bool captureTextCommandContent, bool useNewConventions, string? expectedCommandText, string? expectedQuerySummary) @@ -382,7 +357,7 @@ public async Task SqlQueriesAreSanitized( var activities = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddInMemoryExporter(activities) - .AddEntityFrameworkCoreInstrumentation(options => options.SetDbStatementForText = captureTextCommandContent) + .AddEntityFrameworkCoreInstrumentation() .Build(); var optionsBuilder = new DbContextOptionsBuilder(); @@ -421,7 +396,6 @@ public async Task SqlQueriesAreSanitized( }; private static void VerifyActivityData( - bool captureTextCommandContent, bool isFailure, SemanticConvention conventions, string expectedSpanName, @@ -447,22 +421,15 @@ private static void VerifyActivityData( Assert.Equal(expectedSystemName, activity.GetTagValue(conventions.System)); Assert.Equal(expectedDatabaseName, activity.GetTagValue(conventions.Database)); - if (captureTextCommandContent) - { - Assert.NotNull(activity.GetTagValue(conventions.QueryText)); + Assert.NotNull(activity.GetTagValue(conventions.QueryText)); - if (conventions.EmitsNewAttributes) - { - Assert.NotNull(activity.GetTagValue(conventions.QuerySummary)); - } - else - { - Assert.DoesNotContain(activity.TagObjects, tag => tag.Key == conventions.QuerySummary); - } + if (conventions.EmitsNewAttributes) + { + Assert.NotNull(activity.GetTagValue(conventions.QuerySummary)); } else { - Assert.Null(activity.GetTagValue(conventions.QueryText)); + Assert.DoesNotContain(activity.TagObjects, tag => tag.Key == conventions.QuerySummary); } Assert.DoesNotContain(activity.TagObjects, tag => tag.Key.StartsWith("db.query.parameter.", StringComparison.Ordinal)); diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs index 1be721fe94..f3cfa370a4 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs @@ -26,25 +26,18 @@ public SqlClientIntegrationTests(SqlClientIntegrationTestsFixture fixture) } [EnabledOnDockerPlatformTheory(DockerPlatform.Linux)] - [InlineData(CommandType.Text, "select 1/1")] - [InlineData(CommandType.Text, "select 1/1", true, "select ?/?")] - [InlineData(CommandType.Text, "select 1/0", false, null, true)] - [InlineData(CommandType.Text, "select 1/0", false, null, true, true)] + [InlineData(CommandType.Text, "select 1/1", "select ?/?")] + [InlineData(CommandType.Text, "select 1/0", "select ?/?", true)] + [InlineData(CommandType.Text, "select 1/0", "select ?/?", true, true)] #if NET - [InlineData(CommandType.Text, GetContextInfoQuery, false, null, false, false, false)] - [InlineData(CommandType.Text, GetContextInfoQuery, false, null, false, false, true)] + [InlineData(CommandType.Text, GetContextInfoQuery, GetContextInfoQuery, false, false, false)] + [InlineData(CommandType.Text, GetContextInfoQuery, GetContextInfoQuery, false, false, true)] #endif -#if NETFRAMEWORK - [InlineData(CommandType.StoredProcedure, "sp_who", false, null)] -#else - [InlineData(CommandType.StoredProcedure, "sp_who", false, "sp_who")] -#endif - [InlineData(CommandType.StoredProcedure, "sp_who", true, "sp_who")] + [InlineData(CommandType.StoredProcedure, "sp_who", "sp_who")] public void SuccessfulCommandTest( CommandType commandType, string commandText, - bool captureTextCommandContent = false, - string? sanitizedCommandText = null, + string? sanitizedCommandText, bool isFailure = false, bool recordException = false, bool enableTransaction = false) @@ -64,11 +57,7 @@ public void SuccessfulCommandTest( using var tracerProvider = Sdk.CreateTracerProviderBuilder() .SetSampler(sampler) .AddInMemoryExporter(activities) - .AddSqlClientInstrumentation(options => - { - options.SetDbStatementForText = captureTextCommandContent; - options.RecordException = recordException; - }) + .AddSqlClientInstrumentation(options => options.RecordException = recordException) .Build(); using var sqlConnection = new SqlConnection(this.GetConnectionString()); @@ -106,7 +95,7 @@ public void SuccessfulCommandTest( var activity = Assert.Single(activities); VerifyContextInfo(commandText, commandResult, activity); - VerifyActivityData(commandType, sanitizedCommandText, captureTextCommandContent, isFailure, recordException, activity); + VerifyActivityData(commandType, sanitizedCommandText, isFailure, recordException, activity); VerifySamplingParameters(sampler.LatestSamplingParameters); if (isFailure) @@ -226,7 +215,6 @@ private static void VerifyContextInfo( private static void VerifyActivityData( CommandType commandType, string? commandText, - bool captureTextCommandContent, bool isFailure, bool recordException, Activity activity, @@ -297,22 +285,14 @@ private static void VerifyActivityData( break; case CommandType.Text: - if (captureTextCommandContent) + if (emitOldAttributes) { - if (emitOldAttributes) - { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); - } - - if (emitNewAttributes) - { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); - } + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); } - else + + if (emitNewAttributes) { - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); } break; diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs index 9b3902bc8a..117cf67408 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs @@ -141,11 +141,7 @@ public void SpansAndMetricsGeneratedOnlyWhenEnabled(bool tracesEnabled, bool met if (tracesEnabled) { tracerProviderBuilder - .AddSqlClientInstrumentation(options => - { - options.SetDbStatementForText = true; - options.RecordException = true; - }) + .AddSqlClientInstrumentation(options => options.RecordException = true) .AddInMemoryExporter(activities); } @@ -323,7 +319,11 @@ private static void VerifySamplingParametersOldConventions(SqlClientTestCase tes } } - private void RunSqlClientTestCase(SqlClientTestCase testCase, SqlClientLibrary library, bool emitOldAttributes = false, bool emitNewAttributes = true) + private void RunSqlClientTestCase( + SqlClientTestCase testCase, + SqlClientLibrary library, + bool emitOldAttributes = false, + bool emitNewAttributes = true) { var activities = new List(); var metrics = new List(); @@ -337,7 +337,6 @@ private void RunSqlClientTestCase(SqlClientTestCase testCase, SqlClientLibrary l .SetSampler(sampler) .AddSqlClientInstrumentation(options => { - options.SetDbStatementForText = true; options.RecordException = true; options.EmitOldAttributes = emitOldAttributes; options.EmitNewAttributes = emitNewAttributes; @@ -350,7 +349,12 @@ private void RunSqlClientTestCase(SqlClientTestCase testCase, SqlClientLibrary l .AddInMemoryExporter(metrics) .Build(); - MockCommandExecutor.ExecuteCommand(testCase.Input.ConnectionString, testCase.Input.CommandType, testCase.Input.CommandText, testCase.Expected.ErrorType != null, library); + MockCommandExecutor.ExecuteCommand( + testCase.Input.ConnectionString, + testCase.Input.CommandType, + testCase.Input.CommandText, + testCase.Expected.ErrorType != null, + library); traceProvider.ForceFlush(); meterProvider.ForceFlush(); diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs index c7df634a19..ab19ef502f 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs @@ -69,17 +69,14 @@ public void SqlClient_NamedOptions() Assert.Equal(1, namedExporterOptionsConfigureOptionsInvocations); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void DbQueryTextCollectedWhenEnabled(bool captureTextCommandContent) + [Fact] + public void DbQueryTextCollected() { var activities = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSqlClientInstrumentation(options => { - options.SetDbStatementForText = captureTextCommandContent; options.EmitOldAttributes = true; options.EmitNewAttributes = true; }) @@ -93,20 +90,10 @@ public void DbQueryTextCollectedWhenEnabled(bool captureTextCommandContent) tracerProvider.ForceFlush(); Assert.Equal(2, activities.Count); - if (captureTextCommandContent) - { - Assert.Equal(commandText, activities[0].GetTagValue(SemanticConventions.AttributeDbStatement)); - Assert.Equal(commandText, activities[0].GetTagValue(SemanticConventions.AttributeDbQueryText)); - Assert.Equal(commandText, activities[1].GetTagValue(SemanticConventions.AttributeDbStatement)); - Assert.Equal(commandText, activities[1].GetTagValue(SemanticConventions.AttributeDbQueryText)); - } - else - { - Assert.Null(activities[0].GetTagValue(SemanticConventions.AttributeDbStatement)); - Assert.Null(activities[0].GetTagValue(SemanticConventions.AttributeDbQueryText)); - Assert.Null(activities[1].GetTagValue(SemanticConventions.AttributeDbStatement)); - Assert.Null(activities[1].GetTagValue(SemanticConventions.AttributeDbQueryText)); - } + Assert.Equal(commandText, activities[0].GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.Equal(commandText, activities[0].GetTagValue(SemanticConventions.AttributeDbQueryText)); + Assert.Equal(commandText, activities[1].GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.Equal(commandText, activities[1].GetTagValue(SemanticConventions.AttributeDbQueryText)); } #if !NETFRAMEWORK diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs index 5d82019a05..f1c1f97143 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs @@ -15,38 +15,55 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Tests; [Collection("SqlClient")] public class SqlEventSourceTests { - public static IEnumerable EventSourceFakeTestCases() + public static TheoryData EventSourceFakeTestCases() { /* netfx driver can't capture queries, only stored procedure names */ /* always emit some attribute */ var bools = new[] { true, false }; - return from eventSourceType in new[] { typeof(FakeBehavingAdoNetSqlEventSource), typeof(FakeBehavingMdsSqlEventSource) } - from commandType in new[] { CommandType.StoredProcedure, CommandType.Text } - from isFailure in bools - from captureText in bools - from emitOldAttributes in bools - from emitNewAttributes in bools - from tracingEnabled in bools - from metricsEnabled in bools - where !(commandType == CommandType.Text && captureText) - where emitOldAttributes && emitNewAttributes - let commandText = commandType == CommandType.Text - ? (!isFailure ? "select 1/1" : "select 1/0") - : "sp_who" - let sqlExceptionNumber = 0 - select new object[] - { - eventSourceType, - commandType, - commandText, - captureText, - isFailure, - sqlExceptionNumber, - emitOldAttributes, - emitNewAttributes, - tracingEnabled, - metricsEnabled, - }; + var query = + from eventSourceType in new[] { typeof(FakeBehavingAdoNetSqlEventSource), typeof(FakeBehavingMdsSqlEventSource) } + from commandType in new[] { CommandType.StoredProcedure, CommandType.Text } + from isFailure in bools + from emitOldAttributes in bools + from emitNewAttributes in bools + from tracingEnabled in bools + from metricsEnabled in bools + where !(commandType == CommandType.Text) + where emitOldAttributes && emitNewAttributes + let commandText = commandType == CommandType.Text + ? (!isFailure ? "select 1/1" : "select 1/0") + : "sp_who" + let sqlExceptionNumber = 0 + select new + { + eventSourceType, + commandType, + commandText, + isFailure, + sqlExceptionNumber, + emitOldAttributes, + emitNewAttributes, + tracingEnabled, + metricsEnabled, + }; + + var testCases = new TheoryData(); + + foreach (var item in query) + { + testCases.Add( + item.eventSourceType, + item.commandType, + item.commandText, + item.isFailure, + item.sqlExceptionNumber, + item.emitOldAttributes, + item.emitNewAttributes, + item.tracingEnabled, + item.metricsEnabled); + } + + return testCases; } [Theory] @@ -55,13 +72,12 @@ public void EventSourceFakeTests( Type eventSourceType, CommandType commandType, string commandText, - bool captureText, - bool isFailure = false, - int sqlExceptionNumber = 0, - bool emitOldAttributes = true, - bool emitNewAttributes = false, - bool tracingEnabled = true, - bool metricsEnabled = true) + bool isFailure, + int sqlExceptionNumber, + bool emitOldAttributes, + bool emitNewAttributes, + bool tracingEnabled, + bool metricsEnabled) { using var fakeSqlEventSource = (IFakeBehavingSqlEventSource)Activator.CreateInstance(eventSourceType); @@ -75,7 +91,6 @@ public void EventSourceFakeTests( traceProviderBuilder.AddInMemoryExporter(activities) .AddSqlClientInstrumentation(options => { - options.SetDbStatementForText = captureText; options.EmitOldAttributes = emitOldAttributes; options.EmitNewAttributes = emitNewAttributes; }); @@ -123,7 +138,7 @@ public void EventSourceFakeTests( if (tracingEnabled) { activity = Assert.Single(activities); - VerifyActivityData(commandText, captureText, isFailure, dataSource, activity, emitOldAttributes, emitNewAttributes); + VerifyActivityData(commandText, isFailure, dataSource, activity, emitOldAttributes, emitNewAttributes); } var dbClientOperationDurationMetrics = metrics @@ -199,48 +214,8 @@ public void EventSourceFakeInvalidPayloadTest(Type eventSourceType) Assert.Empty(exportedItems); } - [Theory] - [InlineData(typeof(FakeBehavingAdoNetSqlEventSource))] - [InlineData(typeof(FakeBehavingMdsSqlEventSource))] - public void DefaultCaptureTextFalse(Type eventSourceType) - { - using var fakeSqlEventSource = (IFakeBehavingSqlEventSource)Activator.CreateInstance(eventSourceType); - - var exportedItems = new List(); - var shutdownSignal = Sdk.CreateTracerProviderBuilder() - .AddInMemoryExporter(exportedItems) - .AddSqlClientInstrumentation() - .Build(); - - var objectId = Guid.NewGuid().GetHashCode(); - - const string commandText = "TestCommandTest"; - fakeSqlEventSource.WriteBeginExecuteEvent(objectId, "127.0.0.1", "master", commandText); - - // success is stored in the first bit in compositeState 0b001 - var successFlag = 1; - - // isSqlException is stored in the second bit in compositeState 0b010 - var isSqlExceptionFlag = 2; - - // synchronous state is stored in the third bit in compositeState 0b100 - var synchronousFlag = 4; - - var compositeState = successFlag | isSqlExceptionFlag | synchronousFlag; - - fakeSqlEventSource.WriteEndExecuteEvent(objectId, compositeState, 0); - shutdownSignal.Dispose(); - Assert.Single(exportedItems); - - var activity = exportedItems[0]; - - const bool captureText = false; - VerifyActivityData(commandText, captureText, false, "127.0.0.1", activity, false); - } - private static void VerifyActivityData( string commandText, - bool captureText, bool isFailure, string dataSource, Activity activity, @@ -295,22 +270,14 @@ private static void VerifyActivityData( Assert.Equal("instanceName.master", activity.GetTagValue(SemanticConventions.AttributeDbNamespace)); } - if (captureText) + if (emitOldAttributes) { - if (emitOldAttributes) - { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); - } - - if (emitNewAttributes) - { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); - } + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); } - else + + if (emitNewAttributes) { - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); } if (!isFailure)