diff --git a/OpenTelemetry.sln b/OpenTelemetry.sln index 7a98293e6db..e9839f4190d 100644 --- a/OpenTelemetry.sln +++ b/OpenTelemetry.sln @@ -270,6 +270,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{A49299 src\Shared\PeerServiceResolver.cs = src\Shared\PeerServiceResolver.cs src\Shared\PeriodicExportingMetricReaderHelper.cs = src\Shared\PeriodicExportingMetricReaderHelper.cs src\Shared\PooledList.cs = src\Shared\PooledList.cs + src\Shared\RedactionHelper.cs = src\Shared\RedactionHelper.cs src\Shared\RequestMethodHelper.cs = src\Shared\RequestMethodHelper.cs src\Shared\ResourceSemanticConventions.cs = src\Shared\ResourceSemanticConventions.cs src\Shared\SemanticConventions.cs = src\Shared\SemanticConventions.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreTraceInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreTraceInstrumentationOptions.cs index 38791ee2eba..f5ffb7962f8 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreTraceInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreTraceInstrumentationOptions.cs @@ -32,6 +32,14 @@ internal AspNetCoreTraceInstrumentationOptions(IConfiguration configuration) { this.EnableGrpcAspNetCoreSupport = enableGrpcInstrumentation; } + + if (configuration.TryGetBoolValue( + AspNetCoreInstrumentationEventSource.Log, + "OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_DISABLE_URL_QUERY_REDACTION", + out var disableUrlQueryRedaction)) + { + this.DisableUrlQueryRedaction = disableUrlQueryRedaction; + } } /// @@ -94,4 +102,14 @@ internal AspNetCoreTraceInstrumentationOptions(IConfiguration configuration) /// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md. /// internal bool EnableGrpcAspNetCoreSupport { get; set; } + + /// + /// Gets or sets a value indicating whether the url query value should be redacted or not. + /// + /// + /// The query parameter values are redacted with value set as Redacted. + /// e.g. `?key1=value1` is set as `?key1=Redacted`. + /// The redaction can be disabled by setting this property to . + /// + internal bool DisableUrlQueryRedaction { get; set; } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index baf8bd6a324..d8f74ef6cc5 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -1,6 +1,16 @@ # Changelog -## Unreleased +## 1.8.1 + +Released 2024-Apr-12 + +* **Breaking Change**: Fixed tracing instrumentation so that by default any + values detected in the query string component of requests are replaced with + the text `Redacted` when building the `url.query` tag. For example, + `?key1=value1&key2=value2` becomes `?key1=Redacted&key2=Redacted`. You can + disable this redaction by setting the environment variable + `OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_DISABLE_URL_QUERY_REDACTION` to `true`. + ([#5532](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5532)) ## 1.8.0 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 7d8652637fb..7b5942a1084 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -193,8 +193,14 @@ public void OnStartActivity(Activity activity, object payload) if (request.QueryString.HasValue) { - // QueryString should be sanitized. see: https://github.com/open-telemetry/opentelemetry-dotnet/issues/4571 - activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value); + if (this.options.DisableUrlQueryRedaction) + { + activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value); + } + else + { + activity.SetTag(SemanticConventions.AttributeUrlQuery, RedactionHelper.GetRedactedQueryString(request.QueryString.Value)); + } } RequestMethodHelper.SetHttpMethodTag(activity, request.Method); diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj index a95dd1c1a14..e60747b9501 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj @@ -17,6 +17,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index e51a41be4a8..f8ef36ef2d8 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md @@ -75,7 +75,11 @@ for more details about each individual attribute: * `server.address` * `server.port` * `url.path` -* `url.query` +* `url.query` - By default, the values in the query component are replaced with + the text `Redacted`. For example, `?key1=value1&key2=value2` becomes + `?key1=Redacted&key2=Redacted`. You can disable this redaction by setting the + environment variable + `OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_DISABLE_URL_QUERY_REDACTION` to `true`. * `url.scheme` [Enrich Api](#enrich) can be used if any additional attributes are diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index c3f7330208d..829e5952d79 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -1,6 +1,16 @@ # Changelog -## Unreleased +## 1.8.1 + +Released 2024-Apr-12 + +* **Breaking Change**: Fixed tracing instrumentation so that by default any + values detected in the query string component of requests are replaced with + the text `Redacted` when building the `url.full` tag. For example, + `?key1=value1&key2=value2` becomes `?key1=Redacted&key2=Redacted`. You can + disable this redaction by setting the environment variable + `OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTION` to `true`. + ([#5532](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5532)) ## 1.8.0 diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs index 6015c183f2c..5f97d0342fc 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationTracerProviderBuilderExtensions.cs @@ -59,6 +59,8 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( { services.Configure(name, configureHttpClientTraceInstrumentationOptions); } + + services.RegisterOptionsFactory(configuration => new HttpClientTraceInstrumentationOptions(configuration)); }); #if NETFRAMEWORK diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientTraceInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientTraceInstrumentationOptions.cs index 95bb6dab7b0..a5fbec23cdc 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientTraceInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientTraceInstrumentationOptions.cs @@ -3,10 +3,11 @@ using System.Diagnostics; using System.Net; +using System.Runtime.CompilerServices; #if NETFRAMEWORK using System.Net.Http; #endif -using System.Runtime.CompilerServices; +using Microsoft.Extensions.Configuration; using OpenTelemetry.Instrumentation.Http.Implementation; namespace OpenTelemetry.Instrumentation.Http; @@ -16,6 +17,27 @@ namespace OpenTelemetry.Instrumentation.Http; /// public class HttpClientTraceInstrumentationOptions { + /// + /// Initializes a new instance of the class. + /// + public HttpClientTraceInstrumentationOptions() + : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) + { + } + + internal HttpClientTraceInstrumentationOptions(IConfiguration configuration) + { + Debug.Assert(configuration != null, "configuration was null"); + + if (configuration.TryGetBoolValue( + HttpInstrumentationEventSource.Log, + "OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTION", + out var disableUrlQueryRedaction)) + { + this.DisableUrlQueryRedaction = disableUrlQueryRedaction; + } + } + /// /// Gets or sets a filter function that determines whether or not to /// collect telemetry on a per request basis. @@ -125,6 +147,16 @@ public class HttpClientTraceInstrumentationOptions /// public bool RecordException { get; set; } + /// + /// Gets or sets a value indicating whether the url query value should be redacted or not. + /// + /// + /// The query parameter values are redacted with value set as Redacted. + /// e.g. `?key1=value1` is set as `?key1=Redacted`. + /// The redaction can be disabled by setting this property to . + /// + internal bool DisableUrlQueryRedaction { get; set; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool EventFilterHttpRequestMessage(string activityName, object arg1) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 6b3bea0db4a..8dda9051812 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -149,7 +149,7 @@ public void OnStartActivity(Activity activity, object payload) activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); - activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); + activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri, this.options.DisableUrlQueryRedaction)); try { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpInstrumentationEventSource.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpInstrumentationEventSource.cs index 360fa496701..4f04f9b6671 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpInstrumentationEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpInstrumentationEventSource.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics.Tracing; +using Microsoft.Extensions.Configuration; using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.Http.Implementation; @@ -10,7 +11,7 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation; /// EventSource events emitted from the project. /// [EventSource(Name = "OpenTelemetry-Instrumentation-Http")] -internal sealed class HttpInstrumentationEventSource : EventSource +internal sealed class HttpInstrumentationEventSource : EventSource, IConfigurationExtensionsLogger { public static HttpInstrumentationEventSource Log = new(); @@ -100,4 +101,15 @@ public void UnknownErrorProcessingEvent(string handlerName, string eventName, st { this.WriteEvent(7, handlerName, eventName, ex); } + + [Event(8, Message = "Configuration key '{0}' has an invalid value: '{1}'", Level = EventLevel.Warning)] + public void InvalidConfigurationValue(string key, string value) + { + this.WriteEvent(8, key, value); + } + + void IConfigurationExtensionsLogger.LogInvalidConfigurationValue(string key, string value) + { + this.InvalidConfigurationValue(key, value); + } } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs index 82d7fd11bf2..0b5d7184fd0 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs @@ -1,6 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using OpenTelemetry.Internal; + namespace OpenTelemetry.Instrumentation.Http.Implementation; /// @@ -12,15 +14,18 @@ internal static class HttpTagHelper /// Gets the OpenTelemetry standard uri tag value for a span based on its request . /// /// . + /// Indicates whether query parameter should be redacted or not. /// Span uri value. - public static string GetUriTagValueFromRequestUri(Uri uri) + public static string GetUriTagValueFromRequestUri(Uri uri, bool disableQueryRedaction) { - if (string.IsNullOrEmpty(uri.UserInfo)) + if (string.IsNullOrEmpty(uri.UserInfo) && disableQueryRedaction) { return uri.OriginalString; } - return string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.PathAndQuery, uri.Fragment); + var query = disableQueryRedaction ? uri.Query : RedactionHelper.GetRedactedQueryString(uri.Query); + + return string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.AbsolutePath, query, uri.Fragment); } public static string GetProtocolVersionString(Version httpVersion) => (httpVersion.Major, httpVersion.Minor) switch diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 1595a0a71f4..1418bc0f5ae 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -105,7 +105,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); - activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); + activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri, tracingOptions.DisableUrlQueryRedaction)); try { diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj index 55158028cde..35e43a09e68 100644 --- a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj +++ b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj @@ -12,8 +12,12 @@ + + + + @@ -29,6 +33,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.Http/README.md b/src/OpenTelemetry.Instrumentation.Http/README.md index e9177766ec4..b3fec4ebe45 100644 --- a/src/OpenTelemetry.Instrumentation.Http/README.md +++ b/src/OpenTelemetry.Instrumentation.Http/README.md @@ -68,7 +68,11 @@ for more details about each individual attribute: * `network.protocol.version` * `server.address` * `server.port` -* `url.full` +* `url.full` - By default, the values in the query component of the url are + replaced with the text `Redacted`. For example, `?key1=value1&key2=value2` + becomes `?key1=Redacted&key2=Redacted`. You can disable this redaction by + setting the environment variable + `OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTION` to `true`. [Enrich Api](#enrich-httpclient-api) can be used if any additional attributes are required on activity. diff --git a/src/Shared/RedactionHelper.cs b/src/Shared/RedactionHelper.cs new file mode 100644 index 00000000000..05944efada5 --- /dev/null +++ b/src/Shared/RedactionHelper.cs @@ -0,0 +1,59 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#nullable enable + +using System.Text; + +namespace OpenTelemetry.Internal; + +internal class RedactionHelper +{ + private static readonly string RedactedText = "Redacted"; + + public static string? GetRedactedQueryString(string query) + { + int length = query.Length; + int index = 0; + + // Preallocate some size to avoid re-sizing multiple times. + // Since the size will increase, allocating twice as much. + // TODO: Check to see if we can borrow from https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/System/Text/ValueStringBuilder.cs + // to improve perf. + StringBuilder queryBuilder = new(2 * length); + while (index < query.Length) + { + // Check if the character is = for redacting value. + if (query[index] == '=') + { + // Append = + queryBuilder.Append('='); + index++; + + // Append redactedText in place of original value. + queryBuilder.Append(RedactedText); + + // Move until end of this key/value pair. + while (index < length && query[index] != '&') + { + index++; + } + + // End of key/value. + if (index < length && query[index] == '&') + { + queryBuilder.Append(query[index]); + } + } + else + { + // Keep adding to the result + queryBuilder.Append(query[index]); + } + + index++; + } + + return queryBuilder.ToString(); + } +} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 360f9b5427b..03f6f4c162e 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -1052,6 +1053,78 @@ public async Task NoSiblingActivityCreatedWhenTraceFlagsNone() } #endif + [Theory] + [InlineData("?a", "?a", false)] + [InlineData("?a=bdjdjh", "?a=Redacted", false)] + [InlineData("?a=b&", "?a=Redacted&", false)] + [InlineData("?c=b&", "?c=Redacted&", false)] + [InlineData("?c=a", "?c=Redacted", false)] + [InlineData("?a=b&c", "?a=Redacted&c", false)] + [InlineData("?a=b&c=1123456&", "?a=Redacted&c=Redacted&", false)] + [InlineData("?a=b&c=1&a1", "?a=Redacted&c=Redacted&a1", false)] + [InlineData("?a=ghgjgj&c=1deedd&a1=", "?a=Redacted&c=Redacted&a1=Redacted", false)] + [InlineData("?a=b&c=11&a1=&", "?a=Redacted&c=Redacted&a1=Redacted&", false)] + [InlineData("?c&c&c&", "?c&c&c&", false)] + [InlineData("?a&a&a&a", "?a&a&a&a", false)] + [InlineData("?&&&&&&&", "?&&&&&&&", false)] + [InlineData("?c", "?c", false)] + [InlineData("?a", "?a", true)] + [InlineData("?a=bdfdfdf", "?a=bdfdfdf", true)] + [InlineData("?a=b&", "?a=b&", true)] + [InlineData("?c=b&", "?c=b&", true)] + [InlineData("?c=a", "?c=a", true)] + [InlineData("?a=b&c", "?a=b&c", true)] + [InlineData("?a=b&c=111111&", "?a=b&c=111111&", true)] + [InlineData("?a=b&c=1&a1", "?a=b&c=1&a1", true)] + [InlineData("?a=b&c=1&a1=", "?a=b&c=1&a1=", true)] + [InlineData("?a=b123&c=11&a1=&", "?a=b123&c=11&a1=&", true)] + [InlineData("?c&c&c&", "?c&c&c&", true)] + [InlineData("?a&a&a&a", "?a&a&a&a", true)] + [InlineData("?&&&&&&&", "?&&&&&&&", true)] + [InlineData("?c", "?c", true)] + [InlineData("?c=%26&", "?c=Redacted&", false)] + public async Task ValidateUrlQueryRedaction(string urlQuery, string expectedUrlQuery, bool disableQueryRedaction) + { + var exportedItems = new List(); + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { ["OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_DISABLE_URL_QUERY_REDACTION"] = disableQueryRedaction.ToString() }) + .Build(); + + var path = "/api/values" + urlQuery; + + // Arrange + using var traceprovider = Sdk.CreateTracerProviderBuilder() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + using (var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient()) + { + try + { + using var response = await client.GetAsync(path); + } + catch (Exception) + { + // ignore errors + } + + WaitForActivityExport(exportedItems, 1); + } + + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + Assert.Equal(expectedUrlQuery, activity.GetTagValue(SemanticConventions.AttributeUrlQuery)); + } + public void Dispose() { this.tracerProvider?.Dispose(); diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs index ee2195c6a4c..d5777dbc6f3 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs @@ -27,7 +27,7 @@ public IncomingRequestsCollectionsIsAccordingToTheSpecTests(WebApplicationFactor [Theory] [InlineData("/api/values", null, "user-agent", 200, null)] - [InlineData("/api/values", "?query=1", null, 200, null)] + [InlineData("/api/values", null, null, 200, null)] [InlineData("/api/exception", null, null, 503, null)] [InlineData("/api/exception", null, null, 503, null, true)] public async Task SuccessfulTemplateControllerCallGeneratesASpan_New( @@ -49,7 +49,10 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan_New( services.AddSingleton(new TestCallbackMiddlewareImpl(statusCode, reasonPhrase)); services.AddOpenTelemetry() .WithTracing(builder => builder - .AddAspNetCoreInstrumentation(options => options.RecordException = recordException) + .AddAspNetCoreInstrumentation(options => + { + options.RecordException = recordException; + }) .AddInMemoryExporter(exportedItems)); }); builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index e6c0191b435..d9d58399951 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -2,8 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; +using Microsoft.Extensions.Configuration; + #if NETFRAMEWORK using System.Net.Http; + #endif using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Context.Propagation; @@ -666,6 +669,67 @@ public async Task DoesNotReportExceptionEventOnErrorResponseWithGetStringAsync() Assert.Empty(exportedItems[0].Events); } + [Theory] + [InlineData("?a", "?a", false)] + [InlineData("?a=bdjdjh", "?a=Redacted", false)] + [InlineData("?a=b&", "?a=Redacted&", false)] + [InlineData("?c=b&", "?c=Redacted&", false)] + [InlineData("?c=a", "?c=Redacted", false)] + [InlineData("?a=b&c", "?a=Redacted&c", false)] + [InlineData("?a=b&c=1123456&", "?a=Redacted&c=Redacted&", false)] + [InlineData("?a=b&c=1&a1", "?a=Redacted&c=Redacted&a1", false)] + [InlineData("?a=ghgjgj&c=1deedd&a1=", "?a=Redacted&c=Redacted&a1=Redacted", false)] + [InlineData("?a=b&c=11&a1=&", "?a=Redacted&c=Redacted&a1=Redacted&", false)] + [InlineData("?c&c&c&", "?c&c&c&", false)] + [InlineData("?a&a&a&a", "?a&a&a&a", false)] + [InlineData("?&&&&&&&", "?&&&&&&&", false)] + [InlineData("?c", "?c", false)] + [InlineData("?a", "?a", true)] + [InlineData("?a=bdfdfdf", "?a=bdfdfdf", true)] + [InlineData("?a=b&", "?a=b&", true)] + [InlineData("?c=b&", "?c=b&", true)] + [InlineData("?c=a", "?c=a", true)] + [InlineData("?a=b&c", "?a=b&c", true)] + [InlineData("?a=b&c=111111&", "?a=b&c=111111&", true)] + [InlineData("?a=b&c=1&a1", "?a=b&c=1&a1", true)] + [InlineData("?a=b&c=1&a1=", "?a=b&c=1&a1=", true)] + [InlineData("?a=b123&c=11&a1=&", "?a=b123&c=11&a1=&", true)] + [InlineData("?c&c&c&", "?c&c&c&", true)] + [InlineData("?a&a&a&a", "?a&a&a&a", true)] + [InlineData("?&&&&&&&", "?&&&&&&&", true)] + [InlineData("?c", "?c", true)] + [InlineData("?c=%26&", "?c=Redacted&", false)] + public async Task ValidateUrlQueryRedaction(string urlQuery, string expectedUrlQuery, bool disableQueryRedaction) + { + var exportedItems = new List(); + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { ["OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTION"] = disableQueryRedaction.ToString() }) + .Build(); + + // Arrange + using var traceprovider = Sdk.CreateTracerProviderBuilder() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddHttpClientInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + using var c = new HttpClient(); + try + { + await c.GetStringAsync($"{this.url}path{urlQuery}"); + } + catch + { + } + + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + var expectedUrl = $"{this.url}path{expectedUrlQuery}"; + Assert.Equal(expectedUrl, activity.GetTagValue(SemanticConventions.AttributeUrlFull)); + } + [Theory] [InlineData(true, true)] [InlineData(true, false)] diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs index 33708c34d10..4235f98e536 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -35,6 +35,8 @@ static HttpWebRequestActivitySourceTests() ValidateBaggage(httpWebRequest); } }, + + DisableUrlQueryRedaction = true, }; HttpWebRequestActivitySource.TracingOptions = options; diff --git a/test/OpenTelemetry.Tests/Internal/RedactionHelperTest.cs b/test/OpenTelemetry.Tests/Internal/RedactionHelperTest.cs new file mode 100644 index 00000000000..dda28455c72 --- /dev/null +++ b/test/OpenTelemetry.Tests/Internal/RedactionHelperTest.cs @@ -0,0 +1,32 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using OpenTelemetry.Internal; +using Xunit; + +namespace OpenTelemetry.Tests.Internal; + +public class RedactionHelperTest +{ + [Theory] + [InlineData("?a", "?a")] + [InlineData("?a=b", "?a=Redacted")] + [InlineData("?a=b&", "?a=Redacted&")] + [InlineData("?c=b&", "?c=Redacted&")] + [InlineData("?c=a", "?c=Redacted")] + [InlineData("?a=b&c", "?a=Redacted&c")] + [InlineData("?a=b&c=1&", "?a=Redacted&c=Redacted&")] + [InlineData("?a=b&c=1&a1", "?a=Redacted&c=Redacted&a1")] + [InlineData("?a=b&c=1&a1=", "?a=Redacted&c=Redacted&a1=Redacted")] + [InlineData("?a=b&c=11&a1=&", "?a=Redacted&c=Redacted&a1=Redacted&")] + [InlineData("?c&c&c&", "?c&c&c&")] + [InlineData("?a&a&a&a", "?a&a&a&a")] + [InlineData("?&&&&&&&", "?&&&&&&&")] + [InlineData("?c", "?c")] + [InlineData("?=c", "?=Redacted")] + [InlineData("?=c&=", "?=Redacted&=Redacted")] + public void QueryStringIsRedacted(string input, string expected) + { + Assert.Equal(expected, RedactionHelper.GetRedactedQueryString(input)); + } +} diff --git a/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj b/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj index 86fe7e36844..6d2b3619bd6 100644 --- a/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj +++ b/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj @@ -24,6 +24,7 @@ +