From dc7be37862e4669638bb9df42db83794cbb667d7 Mon Sep 17 00:00:00 2001 From: Aleksandr Golenkevich Date: Wed, 24 Apr 2024 23:54:39 +0900 Subject: [PATCH] [Instrumentation.ElasticsearchClient] Fix sensitive data in url.full (former db.url) tag (#1684) --- opentelemetry-dotnet-contrib.sln | 1 + .../CHANGELOG.md | 3 ++ ...searchRequestPipelineDiagnosticListener.cs | 5 ++- ...Instrumentation.ElasticsearchClient.csproj | 1 + src/Shared/SemanticConventions.cs | 1 - src/Shared/UriHelper.cs | 27 ++++++++++++++ .../ElasticsearchClientTests.cs | 37 +++++++++++++++++++ 7 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 src/Shared/UriHelper.cs diff --git a/opentelemetry-dotnet-contrib.sln b/opentelemetry-dotnet-contrib.sln index 6d755deb10..c6ba36f8b1 100644 --- a/opentelemetry-dotnet-contrib.sln +++ b/opentelemetry-dotnet-contrib.sln @@ -288,6 +288,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{1FCC8E src\Shared\ServerCertificateValidationProvider.cs = src\Shared\ServerCertificateValidationProvider.cs src\Shared\SpanAttributeConstants.cs = src\Shared\SpanAttributeConstants.cs src\Shared\SpanHelper.cs = src\Shared\SpanHelper.cs + src\Shared\UriHelper.cs = src\Shared\UriHelper.cs EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "OpenTelemetry.ResourceDetectors.AWS", "src\OpenTelemetry.ResourceDetectors.AWS\OpenTelemetry.ResourceDetectors.AWS.csproj", "{71BABAC0-E299-48BF-93E2-C11C3840B037}" diff --git a/src/OpenTelemetry.Instrumentation.ElasticsearchClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.ElasticsearchClient/CHANGELOG.md index f38801644e..7fb2da394c 100644 --- a/src/OpenTelemetry.Instrumentation.ElasticsearchClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.ElasticsearchClient/CHANGELOG.md @@ -8,6 +8,9 @@ ([#1624](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1624)) * Update OpenTelemetry SDK version to `1.8.1`. ([#1668](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1668)) +* Replace `db.url` attribute with `url.full` to comply with [semantic conventions](https://github.com/open-telemetry/semantic-conventions/blob/v1.25.0/docs/database/elasticsearch.md#attributes). + Redact `username` and `password` part of the `url.full`. + ([#1684](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1684)) ## 1.0.0-beta.5 diff --git a/src/OpenTelemetry.Instrumentation.ElasticsearchClient/Implementation/ElasticsearchRequestPipelineDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.ElasticsearchClient/Implementation/ElasticsearchRequestPipelineDiagnosticListener.cs index e706dcf14a..af907e627d 100644 --- a/src/OpenTelemetry.Instrumentation.ElasticsearchClient/Implementation/ElasticsearchRequestPipelineDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.ElasticsearchClient/Implementation/ElasticsearchRequestPipelineDiagnosticListener.cs @@ -171,6 +171,9 @@ private void OnStartActivity(Activity activity, object payload) return; } + // remove sensitive information like user and password information + uri = UriHelper.ScrubUserInfo(uri); + ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client); @@ -210,7 +213,7 @@ private void OnStartActivity(Activity activity, object payload) activity.SetTag(AttributeDbMethod, method.ToString()); } - activity.SetTag(SemanticConventions.AttributeDbUrl, uri.OriginalString); + activity.SetTag(SemanticConventions.AttributeUrlFull, uri.OriginalString); try { diff --git a/src/OpenTelemetry.Instrumentation.ElasticsearchClient/OpenTelemetry.Instrumentation.ElasticsearchClient.csproj b/src/OpenTelemetry.Instrumentation.ElasticsearchClient/OpenTelemetry.Instrumentation.ElasticsearchClient.csproj index 1d378439e9..fd3972389a 100644 --- a/src/OpenTelemetry.Instrumentation.ElasticsearchClient/OpenTelemetry.Instrumentation.ElasticsearchClient.csproj +++ b/src/OpenTelemetry.Instrumentation.ElasticsearchClient/OpenTelemetry.Instrumentation.ElasticsearchClient.csproj @@ -28,5 +28,6 @@ + diff --git a/src/Shared/SemanticConventions.cs b/src/Shared/SemanticConventions.cs index 6c70b8c54c..2fa0c8144c 100644 --- a/src/Shared/SemanticConventions.cs +++ b/src/Shared/SemanticConventions.cs @@ -53,7 +53,6 @@ internal static class SemanticConventions public const string AttributeDbStatement = "db.statement"; public const string AttributeDbOperation = "db.operation"; public const string AttributeDbInstance = "db.instance"; - public const string AttributeDbUrl = "db.url"; public const string AttributeDbCassandraKeyspace = "db.cassandra.keyspace"; public const string AttributeDbHBaseNamespace = "db.hbase.namespace"; public const string AttributeDbRedisDatabaseIndex = "db.redis.database_index"; diff --git a/src/Shared/UriHelper.cs b/src/Shared/UriHelper.cs new file mode 100644 index 0000000000..bccd1a96e0 --- /dev/null +++ b/src/Shared/UriHelper.cs @@ -0,0 +1,27 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System; + +namespace OpenTelemetry.Trace; + +internal static class UriHelper +{ + private const string RedactedText = "REDACTED"; + + public static Uri ScrubUserInfo(Uri uri) + { + var uriBuilder = new UriBuilder(uri); + if (!string.IsNullOrEmpty(uriBuilder.UserName)) + { + uriBuilder.UserName = RedactedText; + } + + if (!string.IsNullOrEmpty(uriBuilder.Password)) + { + uriBuilder.Password = RedactedText; + } + + return uriBuilder.Uri; + } +} diff --git a/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/ElasticsearchClientTests.cs b/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/ElasticsearchClientTests.cs index 8507a98adf..f9db99cbcb 100644 --- a/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/ElasticsearchClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/ElasticsearchClientTests.cs @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -814,4 +815,40 @@ public async Task DbStatementIsDisplayedWhenSetDbStatementForRequestIsUsingTheDe var tags = searchActivity.Tags.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); Assert.NotNull(searchActivity.GetTagValue(SemanticConventions.AttributeDbStatement)); } + + [Fact] + public async Task ShouldRemoveSensitiveInformation() + { + var expectedResource = ResourceBuilder.CreateDefault().AddService("test-service"); + var exportedItems = new List(); + + var sensitiveConnectionString = new Uri($"http://sensitiveUsername:sensitivePassword@localhost:9200"); + + var client = new ElasticClient(new ConnectionSettings( + new SingleNodeConnectionPool(sensitiveConnectionString), new InMemoryConnection()).DefaultIndex("customer")); + + using (Sdk.CreateTracerProviderBuilder() + .SetSampler(new AlwaysOnSampler()) + .AddElasticsearchClientInstrumentation(o => o.SetDbStatementForRequest = false) + .SetResourceBuilder(expectedResource) + .AddInMemoryExporter(exportedItems) + .Build()) + { + var searchResponse = await client.SearchAsync(s => s.Query(q => q.Bool(b => b.Must(m => m.Term(f => f.Id, "123"))))); + Assert.NotNull(searchResponse); + Assert.True(searchResponse.ApiCall.Success); + Assert.NotEmpty(searchResponse.ApiCall.AuditTrail); + + var failed = searchResponse.ApiCall.AuditTrail.Where(a => a.Event == AuditEvent.BadResponse); + Assert.Empty(failed); + } + + Assert.Single(exportedItems); + var searchActivity = exportedItems[0]; + + string dbUrl = (string)searchActivity.GetTagValue(SemanticConventions.AttributeUrlFull); + + Assert.DoesNotContain("sensitive", dbUrl); + Assert.Contains("REDACTED:REDACTED", dbUrl); + } }