diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md index e1e5f5c1ee..b22e0c290d 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md @@ -10,6 +10,11 @@ to set default explicit buckets following the [OpenTelemetry Specification](https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/database/database-metrics.md). ([#2430](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2430)) +* Enabling `SetDbStatementForText` will no longer capture the raw query text. + The query is now sanitized. Literal values in the query text are replaced + by a `?` character. + ([#2446](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2446)) + ## 1.10.0-beta.1 Released 2024-Dec-09 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs index 00342c6c58..c990695a54 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs @@ -31,7 +31,7 @@ internal sealed class SqlClientDiagnosticListener : ListenerHandler private readonly PropertyFetcher dataSourceFetcher = new("DataSource"); private readonly PropertyFetcher databaseFetcher = new("Database"); private readonly PropertyFetcher commandTypeFetcher = new("CommandType"); - private readonly PropertyFetcher commandTextFetcher = new("CommandText"); + private readonly PropertyFetcher commandTextFetcher = new("CommandText"); private readonly PropertyFetcher exceptionFetcher = new("Exception"); private readonly PropertyFetcher exceptionNumberFetcher = new("Number"); private readonly AsyncLocal beginTimestamp = new(); @@ -102,9 +102,8 @@ public override void OnEventWritten(string name, object? payload) return; } - _ = this.commandTextFetcher.TryFetch(command, out var commandText); - - if (this.commandTypeFetcher.TryFetch(command, out var commandType)) + if (this.commandTypeFetcher.TryFetch(command, out var commandType) && + this.commandTextFetcher.TryFetch(command, out var commandText)) { switch (commandType) { @@ -126,14 +125,15 @@ public override void OnEventWritten(string name, object? payload) case CommandType.Text: if (options.SetDbStatementForText) { + var sanitizedSql = SqlProcessor.GetSanitizedSql(commandText); if (options.EmitOldAttributes) { - activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + activity.SetTag(SemanticConventions.AttributeDbStatement, sanitizedSql); } if (options.EmitNewAttributes) { - activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); + activity.SetTag(SemanticConventions.AttributeDbQueryText, sanitizedSql); } } diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj index cdeb2cd9a4..cbb8e70bff 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj +++ b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj @@ -26,6 +26,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/README.md b/src/OpenTelemetry.Instrumentation.SqlClient/README.md index d77db1ce59..f70e6b39ba 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/README.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/README.md @@ -123,12 +123,13 @@ This instrumentation can be configured to change the default behavior by using ### SetDbStatementForText -Capturing the text of a database query may run the risk of capturing sensitive data. -`SetDbStatementForText` controls whether the -[`db.statement`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes) -attribute is captured in scenarios where there could be a risk of exposing -sensitive data. The behavior of `SetDbStatementForText` depends on the runtime -used. +`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 diff --git a/src/Shared/SqlProcessor.cs b/src/Shared/SqlProcessor.cs index c5c5ee9a62..c23a7c8a28 100644 --- a/src/Shared/SqlProcessor.cs +++ b/src/Shared/SqlProcessor.cs @@ -1,19 +1,49 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System.Collections; using System.Text; namespace OpenTelemetry.Instrumentation; internal static class SqlProcessor { - public static string GetSanitizedSql(string sql) + private const int CacheCapacity = 1000; + private static readonly Hashtable Cache = []; + + public static string GetSanitizedSql(string? sql) { if (sql == null) { return string.Empty; } + if (Cache[sql] is not string sanitizedSql) + { + sanitizedSql = SanitizeSql(sql); + + if (Cache.Count == CacheCapacity) + { + return sanitizedSql; + } + + lock (Cache) + { + if ((Cache[sql] as string) == null) + { + if (Cache.Count < CacheCapacity) + { + Cache[sql] = sanitizedSql; + } + } + } + } + + return sanitizedSql!; + } + + private static string SanitizeSql(string sql) + { var sb = new StringBuilder(capacity: sql.Length); for (var i = 0; i < sql.Length; ++i) { diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs index 2185963f9e..bb72593691 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs @@ -24,15 +24,16 @@ public SqlClientIntegrationTests(SqlClientIntegrationTestsFixture fixture) [EnabledOnDockerPlatformTheory(EnabledOnDockerPlatformTheoryAttribute.DockerPlatform.Linux)] [InlineData(CommandType.Text, "select 1/1")] - [InlineData(CommandType.Text, "select 1/1", true)] - [InlineData(CommandType.Text, "select 1/0", false, true)] - [InlineData(CommandType.Text, "select 1/0", false, true, false, false)] - [InlineData(CommandType.Text, "select 1/0", false, true, true, false)] - [InlineData(CommandType.StoredProcedure, "sp_who")] + [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, false, false)] + [InlineData(CommandType.Text, "select 1/0", false, null, true, true, false)] + [InlineData(CommandType.StoredProcedure, "sp_who", false, "sp_who")] public void SuccessfulCommandTest( CommandType commandType, string commandText, bool captureTextCommandContent = false, + string? sanitizedCommandText = null, bool isFailure = false, bool recordException = false, bool shouldEnrich = true) @@ -84,7 +85,7 @@ public void SuccessfulCommandTest( Assert.Single(activities); var activity = activities[0]; - SqlClientTests.VerifyActivityData(commandType, commandText, captureTextCommandContent, isFailure, recordException, shouldEnrich, activity); + SqlClientTests.VerifyActivityData(commandType, sanitizedCommandText, captureTextCommandContent, isFailure, recordException, shouldEnrich, activity); SqlClientTests.VerifySamplingParameters(sampler.LatestSamplingParameters); if (isFailure) diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs index c31cd18c40..759cf81445 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs @@ -408,7 +408,7 @@ public void ShouldNotCollectTelemetryAndShouldNotPropagateExceptionWhenFilterThr internal static void VerifyActivityData( CommandType commandType, - string commandText, + string? commandText, bool captureTextCommandContent, bool isFailure, bool recordException,