[EFCore] [SqlClient] Remove SetDbQueryParameters (#3081)

Co-authored-by: Piotr Kiełkowicz <[email protected]>

Author: martincostello

src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt

@@ -2,8 +2,6 @@ OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentation
 OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EntityFrameworkInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.Filter.get -> System.Func<string?, System.Data.IDbCommand!, bool>?
 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.EnrichWithIDbCommand.get -> System.Action<System.Diagnostics.Activity!, System.Data.IDbCommand!>?
 OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.set -> void
 OpenTelemetry.Trace.TracerProviderBuilderExtensions

src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md

@@ -16,8 +16,10 @@
   DB2, MongoDB, MySQL, Oracle, PostgreSQL and SQLite.
   ([#3025](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3025))
 * Add `db.query.parameter.<key>` attribute(s) to query spans if opted into using
-  the `SetDbQueryParameters` option.
-  ([#3015](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3015))
+  the `OTEL_DOTNET_EXPERIMENTAL_EFCORE_ENABLE_TRACE_DB_QUERY_PARAMETERS`
+  environment variable.
+  ([#3015](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3015),
+  [#3081](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3081))
 * 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))

src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs

@@ -26,6 +26,12 @@ internal EntityFrameworkInstrumentationOptions(IConfiguration configuration)
         var databaseSemanticConvention = GetSemanticConventionOptIn(configuration);
         this.EmitOldAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.Old);
         this.EmitNewAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.New);
+
+        if (configuration["OTEL_DOTNET_EXPERIMENTAL_EFCORE_ENABLE_TRACE_DB_QUERY_PARAMETERS"] is { Length: > 0 } value &&
+            bool.TryParse(value, out var setDbQueryParameters))
+        {
+            this.SetDbQueryParameters = setDbQueryParameters;
+        }
     }
 
     /// <summary>
@@ -70,7 +76,7 @@ internal EntityFrameworkInstrumentationOptions(IConfiguration configuration)
     /// contain any sensitive data.</b>
     /// </para>
     /// </remarks>
-    public bool SetDbQueryParameters { get; set; }
+    internal bool SetDbQueryParameters { get; set; }
 
     /// <summary>
     /// Gets or sets a value indicating whether the old database attributes should be emitted.

src/OpenTelemetry.Instrumentation.EntityFrameworkCore/README.md

@@ -141,30 +141,25 @@ services.AddOpenTelemetry()
         .AddConsoleExporter());
 ```
 
-### SetDbQueryParameters
+## Experimental features
 
-`SetDbQueryParameters` controls whether `db.query.parameter.<key>` attributes
-are emitted.
+> [!NOTE]
+> Experimental features are not enabled by default and can only be activated with
+> environment variables. They are subject to change or removal in future releases.
 
-Query parameters may contain sensitive data, so only enable `SetDbQueryParameters`
+### DB query parameters
+
+The `OTEL_DOTNET_EXPERIMENTAL_EFCORE_ENABLE_TRACE_DB_QUERY_PARAMETERS` environment
+variable controls whether `db.query.parameter.<key>` attributes are emitted.
+
+Query parameters may contain sensitive data, so only enable this experimental feature
 if your queries and/or environment are appropriate for enabling this option.
 
-`SetDbQueryParameters` is _false_ by default. When set to `true`, the
-instrumentation will set
+`OTEL_DOTNET_EXPERIMENTAL_EFCORE_ENABLE_TRACE_DB_QUERY_PARAMETERS` is implicitly
+`false` by default. When set to `true`, the instrumentation will set
 [`db.query.parameter.<key>`](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.
 
-To enable capturing of parameter names and values use the
-following configuration.
-
-```csharp
-using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-    .AddEntityFrameworkCoreInstrumentation(
-        options => options.SetDbQueryParameters = true)
-    .AddConsoleExporter()
-    .Build();
-```
-
 ## References
 
 * [OpenTelemetry Project](https://opentelemetry.io/)

src/OpenTelemetry.Instrumentation.SqlClient/.publicApi/PublicAPI.Unshipped.txt

@@ -6,8 +6,6 @@ OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.Fil
 OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.Filter.set -> void
 OpenTelemetry.Instrumentation.SqlClient.SqlClientTraceInstrumentationOptions.RecordException.get -> bool
 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.SqlClientTraceInstrumentationOptions() -> void
 OpenTelemetry.Metrics.SqlClientMeterProviderBuilderExtensions
 OpenTelemetry.Trace.TracerProviderBuilderExtensions

src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md

@@ -3,8 +3,10 @@
 ## Unreleased
 
 * Add `db.query.parameter.<key>` attribute(s) to query spans if opted into using
-  the `SetDbQueryParameters` option. Not supported on .NET Framework.
-  ([#3015](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3015))
+  the `OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_TRACE_DB_QUERY_PARAMETERS`
+  environment variable. Not supported on .NET Framework.
+  ([#3015](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3015),
+  [#3081](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/3081))
 
 * Fix activities not being stopped on .NET Framework when using a global activity
   listener.

src/OpenTelemetry.Instrumentation.SqlClient/README.md

@@ -121,33 +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`.
 
-### SetDbQueryParameters
-
-> [!NOTE]
-> SetDbQueryParameters is not supported on .NET Framework.
-
-`SetDbQueryParameters` controls whether `db.query.parameter.<key>` attributes
-are emitted.
-
-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
-instrumentation will set
-[`db.query.parameter.<key>`](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.
-
-To enable capturing of parameter names and values use the
-following configuration.
-
-```csharp
-using var tracerProvider = Sdk.CreateTracerProviderBuilder()
-    .AddSqlClientInstrumentation(
-        options => options.SetDbQueryParameters = true)
-    .AddConsoleExporter()
-    .Build();
-```
-
 ### Enrich
 
 > [!NOTE]
@@ -251,6 +224,28 @@ command to set [traceparent](https://www.w3.org/TR/trace-context/#traceparent-he
 information for the current connection, which results in
 **an additional round-trip to the database**.
 
+## Experimental features
+
+> [!NOTE]
+> Experimental features are not enabled by default and can only be activated with
+> environment variables. They are subject to change or removal in future releases.
+
+### DB query parameters
+
+> [!NOTE]
+> This feature is available on .NET runtimes only.
+
+The `OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_TRACE_DB_QUERY_PARAMETERS` environment
+variable controls whether `db.query.parameter.<key>` attributes are emitted.
+
+Query parameters may contain sensitive data, so only enable this experimental feature
+if your queries and/or environment are appropriate for enabling this option.
+
+`OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_TRACE_DB_QUERY_PARAMETERS` is implicitly
+`false` by default. When set to `true`, the instrumentation will set
+[`db.query.parameter.<key>`](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.
+
 ## Activity Duration calculation
 
 `Activity.Duration` represents the time the underlying connection takes to

src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs

@@ -19,6 +19,7 @@ namespace OpenTelemetry.Instrumentation.SqlClient;
 public class SqlClientTraceInstrumentationOptions
 {
     internal const string ContextPropagationLevelEnvVar = "OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_TRACE_CONTEXT_PROPAGATION";
+    internal const string SetDbQueryParametersEnvVar = "OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_TRACE_DB_QUERY_PARAMETERS";
 
     /// <summary>
     /// Initializes a new instance of the <see cref="SqlClientTraceInstrumentationOptions"/> class.
@@ -44,6 +45,14 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration)
         {
             this.EnableTraceContextPropagation = enableTraceContextPropagation;
         }
+
+        if (configuration!.TryGetBoolValue(
+                SqlClientInstrumentationEventSource.Log,
+                SetDbQueryParametersEnvVar,
+                out var setDbQueryParameters))
+        {
+            this.SetDbQueryParameters = setDbQueryParameters;
+        }
 #endif
     }
 
@@ -98,6 +107,7 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration)
     /// </remarks>
     public bool RecordException { get; set; }
 
+#if !NETFRAMEWORK
     /// <summary>
     /// Gets or sets a value indicating whether or not the <see cref="SqlClientInstrumentation"/>
     /// should add the names and values of query parameters as the <c>db.query.parameter.{key}</c> tag.
@@ -110,10 +120,11 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration)
     /// contain any sensitive data.</b>
     /// </para>
     /// <para>
-    /// <b>SetDbQueryParameters is only supported on .NET and .NET Core runtimes.</b>
+    /// <b>SetDbQueryParameters is only supported on .NET runtimes.</b>
     /// </para>
     /// </remarks>
-    public bool SetDbQueryParameters { get; set; }
+    internal bool SetDbQueryParameters { get; set; }
+#endif
 
     /// <summary>
     /// Gets or sets a value indicating whether the old database attributes should be emitted.

test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkInstrumentationOptionsTests.cs

@@ -39,4 +39,26 @@ public void ShouldEmitNewAttributesWhenStabilityOptInIsDatabase()
         Assert.False(options.EmitOldAttributes);
         Assert.True(options.EmitNewAttributes);
     }
+
+    [Fact]
+    public void ShouldNotEmitDatabaseQueryParametersByDefault()
+    {
+        var configuration = new ConfigurationBuilder().Build();
+        var options = new EntityFrameworkInstrumentationOptions(configuration);
+        Assert.False(options.SetDbQueryParameters);
+    }
+
+    [Theory]
+    [InlineData("", false)]
+    [InlineData("invalid", false)]
+    [InlineData("false", false)]
+    [InlineData("true", true)]
+    public void ShouldAssignSetDatabaseQueryParametersFromEnvironmentVariable(string value, bool expected)
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> { ["OTEL_DOTNET_EXPERIMENTAL_EFCORE_ENABLE_TRACE_DB_QUERY_PARAMETERS"] = value })
+            .Build();
+        var options = new EntityFrameworkInstrumentationOptions(configuration);
+        Assert.Equal(expected, options.SetDbQueryParameters);
+    }
 }

test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs

@@ -216,6 +216,28 @@ public void ShouldNotCollectTelemetryAndShouldNotPropagateExceptionWhenFilterThr
 
         Assert.Empty(activities);
     }
+
+    [Fact]
+    public void ShouldNotEmitDatabaseQueryParametersByDefault()
+    {
+        var configuration = new ConfigurationBuilder().Build();
+        var options = new SqlClientTraceInstrumentationOptions(configuration);
+        Assert.False(options.SetDbQueryParameters);
+    }
+
+    [Theory]
+    [InlineData("", false)]
+    [InlineData("invalid", false)]
+    [InlineData("false", false)]
+    [InlineData("true", true)]
+    public void ShouldAssignSetDatabaseQueryParametersFromEnvironmentVariable(string value, bool expected)
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> { ["OTEL_DOTNET_EXPERIMENTAL_SQLCLIENT_ENABLE_TRACE_DB_QUERY_PARAMETERS"] = value })
+            .Build();
+        var options = new SqlClientTraceInstrumentationOptions(configuration);
+        Assert.Equal(expected, options.SetDbQueryParameters);
+    }
 #endif
 
     private static void ActivityEnrichment(Activity activity, string method, object obj)