diff --git a/opentelemetry-dotnet-contrib.sln b/opentelemetry-dotnet-contrib.sln index 776f0dbc0c..1e884823d6 100644 --- a/opentelemetry-dotnet-contrib.sln +++ b/opentelemetry-dotnet-contrib.sln @@ -280,8 +280,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{1FCC8E src\Shared\ListenerHandler.cs = src\Shared\ListenerHandler.cs src\Shared\MultiTypePropertyFetcher.cs = src\Shared\MultiTypePropertyFetcher.cs src\Shared\NullableAttributes.cs = src\Shared\NullableAttributes.cs - src\Shared\PropertyFetcher.cs = src\Shared\PropertyFetcher.cs src\Shared\PropertyFetcher.AOT.cs = src\Shared\PropertyFetcher.AOT.cs + src\Shared\PropertyFetcher.cs = src\Shared\PropertyFetcher.cs + src\Shared\RedactionHelper.cs = src\Shared\RedactionHelper.cs src\Shared\ResourceSemanticConventions.cs = src\Shared\ResourceSemanticConventions.cs src\Shared\SemanticConventions.cs = src\Shared\SemanticConventions.cs src\Shared\SpanAttributeConstants.cs = src\Shared\SpanAttributeConstants.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetTraceInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetTraceInstrumentationOptions.cs index 809e1453e0..f5ec023288 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetTraceInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/AspNetTraceInstrumentationOptions.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.Web; +using OpenTelemetry.Instrumentation.AspNet.Implementation; namespace OpenTelemetry.Instrumentation.AspNet; @@ -12,6 +13,27 @@ namespace OpenTelemetry.Instrumentation.AspNet; /// public class AspNetTraceInstrumentationOptions { + private const string DisableQueryRedactionEnvVar = "OTEL_DOTNET_EXPERIMENTAL_ASPNET_DISABLE_URL_QUERY_REDACTION"; + + /// + /// Initializes a new instance of the class. + /// + public AspNetTraceInstrumentationOptions() + { + try + { + var disableQueryRedaction = Environment.GetEnvironmentVariable(DisableQueryRedactionEnvVar); + if (disableQueryRedaction != null && disableQueryRedaction.Equals("true", StringComparison.OrdinalIgnoreCase)) + { + this.DisableUrlQueryRedaction = true; + } + } + catch (Exception ex) + { + AspNetInstrumentationEventSource.Log.FailedToReadEnvironmentVariable(DisableQueryRedactionEnvVar, ex); + } + } + /// /// Gets or sets a filter callback function that determines on a per /// request basis whether or not to collect telemetry. @@ -46,4 +68,14 @@ public class AspNetTraceInstrumentationOptions /// See: . /// 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; } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 679198547b..0fcaeef82d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -1,6 +1,16 @@ # Changelog -## Unreleased +## 1.8.0-beta.2 + +Released 2024-Apr-17 + +* **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` attribute. For example, + `?key1=value1&key2=value2` becomes `?key1=Redacted&key2=Redacted`. You can + disable this redaction by setting the environment variable + `OTEL_DOTNET_EXPERIMENTAL_ASPNET_DISABLE_URL_QUERY_REDACTION` to `true`. + ([#1656](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1656)) ## 1.8.0-beta.1 diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs index 167e616253..7e118e74ef 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs @@ -33,6 +33,15 @@ public void EnrichmentException(string eventName, Exception ex) } } + [NonEvent] + public void FailedToReadEnvironmentVariable(string envVarName, Exception ex) + { + if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) + { + this.EnrichmentException(envVarName, ex.ToInvariantString()); + } + } + [Event(1, Message = "Request is filtered out and will not be collected. Operation='{0}'", Level = EventLevel.Verbose)] public void RequestIsFilteredOut(string operationName) { @@ -50,4 +59,10 @@ public void EnrichmentException(string eventName, string exception) { this.WriteEvent(3, eventName, exception); } + + [Event(4, Message = "Failed to read environment variable='{0}': {1}", Level = EventLevel.Error)] + public void FailedToReadEnvironmentVariable(string envVarName, string exception) + { + this.WriteEvent(4, envVarName, exception); + } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index 0337c56eaa..5879be98be 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -91,14 +91,8 @@ private void OnStartActivity(Activity activity, HttpContext context) var query = url.Query; if (!string.IsNullOrEmpty(query)) { - if (query.StartsWith("?", StringComparison.InvariantCulture)) - { - activity.SetTag(SemanticConventions.AttributeUrlQuery, query.Substring(1)); - } - else - { - activity.SetTag(SemanticConventions.AttributeUrlQuery, query); - } + var queryString = query.StartsWith("?", StringComparison.InvariantCulture) ? query.Substring(1) : query; + activity.SetTag(SemanticConventions.AttributeUrlQuery, this.options.DisableUrlQueryRedaction ? queryString : RedactionHelper.GetRedactedQueryString(queryString)); } var userAgent = request.UserAgent; diff --git a/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj b/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj index cd8d81f1fe..25b0a759d8 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj @@ -17,6 +17,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.Owin/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Owin/CHANGELOG.md index 4ffd5330c9..92f8480061 100644 --- a/src/OpenTelemetry.Instrumentation.Owin/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Owin/CHANGELOG.md @@ -1,6 +1,16 @@ # Changelog -## Unreleased +## 1.0.0-rc.5 + +Released 2024-Apr-17 + +* **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 `http.url` 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_OWIN_DISABLE_URL_QUERY_REDACTION` to `true`. + ([#1656](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1656)) * `ActivitySource.Version` and `Meter.Version` are set to NuGet package version. ([#1624](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1624)) diff --git a/src/OpenTelemetry.Instrumentation.Owin/Implementation/DiagnosticsMiddleware.cs b/src/OpenTelemetry.Instrumentation.Owin/Implementation/DiagnosticsMiddleware.cs index 6b435ef87b..1efc1ee464 100644 --- a/src/OpenTelemetry.Instrumentation.Owin/Implementation/DiagnosticsMiddleware.cs +++ b/src/OpenTelemetry.Instrumentation.Owin/Implementation/DiagnosticsMiddleware.cs @@ -9,6 +9,7 @@ using Microsoft.Owin; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Owin.Implementation; +using OpenTelemetry.Internal; using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.Owin; @@ -114,7 +115,7 @@ private static void BeginRequest(IOwinContext owinContext) activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method); activity.SetTag(SemanticConventions.AttributeHttpTarget, request.Uri.AbsolutePath); - activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUriTagValueFromRequestUri(request.Uri)); + activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUriTagValueFromRequestUri(request.Uri, OwinInstrumentationActivitySource.Options.DisableUrlQueryRedaction)); if (request.Headers.TryGetValue("User-Agent", out string[] userAgent) && userAgent.Length > 0) { @@ -228,13 +229,15 @@ private static void RequestEnd(IOwinContext owinContext, Exception? exception, l /// /// . /// Span uri value. - private static string GetUriTagValueFromRequestUri(Uri uri) + private static string GetUriTagValueFromRequestUri(Uri uri, bool disableQueryRedaction) { - if (string.IsNullOrEmpty(uri.UserInfo)) + if (string.IsNullOrEmpty(uri.UserInfo) && disableQueryRedaction) { - return uri.ToString(); + 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); } } diff --git a/src/OpenTelemetry.Instrumentation.Owin/Implementation/OwinInstrumentationEventSource.cs b/src/OpenTelemetry.Instrumentation.Owin/Implementation/OwinInstrumentationEventSource.cs index 1ac5514375..1f5f35ca9a 100644 --- a/src/OpenTelemetry.Instrumentation.Owin/Implementation/OwinInstrumentationEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.Owin/Implementation/OwinInstrumentationEventSource.cs @@ -45,16 +45,32 @@ public void EnrichmentException(Exception exception) } } + [NonEvent] + public void FailedToReadEnvironmentVariable(string envVarName, Exception ex) + { + if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) + { + this.FailedToReadEnvironmentVariable(envVarName, ex.ToInvariantString()); + } + } + [Event(EventIds.EnrichmentException, Message = "Enrichment threw exception. Exception {0}.", Level = EventLevel.Error)] public void EnrichmentException(string exception) { this.WriteEvent(EventIds.EnrichmentException, exception); } + [Event(EventIds.FailedToReadEnvironmentVariable, Message = "Failed to read environment variable='{0}': {1}", Level = EventLevel.Error)] + public void FailedToReadEnvironmentVariable(string envVarName, string exception) + { + this.WriteEvent(4, envVarName, exception); + } + private class EventIds { public const int RequestIsFilteredOut = 1; public const int RequestFilterException = 2; public const int EnrichmentException = 3; + public const int FailedToReadEnvironmentVariable = 4; } } diff --git a/src/OpenTelemetry.Instrumentation.Owin/OpenTelemetry.Instrumentation.Owin.csproj b/src/OpenTelemetry.Instrumentation.Owin/OpenTelemetry.Instrumentation.Owin.csproj index 94e9dd8358..9773878f10 100644 --- a/src/OpenTelemetry.Instrumentation.Owin/OpenTelemetry.Instrumentation.Owin.csproj +++ b/src/OpenTelemetry.Instrumentation.Owin/OpenTelemetry.Instrumentation.Owin.csproj @@ -10,6 +10,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.Owin/OwinInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Owin/OwinInstrumentationOptions.cs index 57f98e812d..e17a0c3bfd 100644 --- a/src/OpenTelemetry.Instrumentation.Owin/OwinInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.Owin/OwinInstrumentationOptions.cs @@ -12,6 +12,27 @@ namespace OpenTelemetry.Instrumentation.Owin; /// public class OwinInstrumentationOptions { + private const string DisableQueryRedactionEnvVar = "OTEL_DOTNET_EXPERIMENTAL_OWIN_DISABLE_URL_QUERY_REDACTION"; + + /// + /// Initializes a new instance of the class. + /// + public OwinInstrumentationOptions() + { + try + { + var disableQueryRedaction = Environment.GetEnvironmentVariable(DisableQueryRedactionEnvVar); + if (disableQueryRedaction != null && disableQueryRedaction.Equals("true", StringComparison.OrdinalIgnoreCase)) + { + this.DisableUrlQueryRedaction = true; + } + } + catch (Exception ex) + { + OwinInstrumentationEventSource.Log.FailedToReadEnvironmentVariable(DisableQueryRedactionEnvVar, ex); + } + } + /// /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis. /// The Filter gets the , and should return a boolean. @@ -32,4 +53,14 @@ public class OwinInstrumentationOptions /// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md. /// 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; } } diff --git a/src/Shared/RedactionHelper.cs b/src/Shared/RedactionHelper.cs new file mode 100644 index 0000000000..cc6e34c7ea --- /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 sealed class RedactionHelper +{ + private const 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.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 4b832aa6d6..4359a3b9b8 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -20,26 +20,29 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests; public class HttpInListenerTests { [Theory] - [InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] - [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] - [InlineData("https://localhost/", "https", "/", null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null, "HTTP")] - [InlineData("https://user:pass@localhost/", "https", "/", null, "localhost", 443, "GeT", "GET", "GeT", 0, null, "GET")] // Test URL sanitization - [InlineData("http://localhost:443/", "http", "/", null, "localhost", 443, "GET", "GET", null, 0, null, "GET")] // Test http over 443 - [InlineData("https://localhost:80/", "https", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test https over 80 - [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL - [InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL sanitization - [InlineData("http://localhost:80/Index", "http", "/Index", null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}", "GET {controller}/{action}/{id}")] - [InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, "localhost", 443, "HEAD", "HEAD", null, 2, "about_attr_route/{customerId}", "HEAD about_attr_route/{customerId}")] - [InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}", "GET api/{controller}/{id}")] - [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}", "GET subroute/{customerId}")] - [InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "/api/value")] // Request will be filtered - [InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "{ThrowException}")] // Filter user code will throw an exception - [InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, null, true, "System.InvalidOperationException")] // Test RecordException option + [InlineData("http://localhost/", "http", "/", null, true, "localhost", 80, "GET", "GET", null, 0, null, "GET")] + [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", true, "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] + [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=Redacted&baz=Redacted", false, "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] + [InlineData("https://localhost/", "https", "/", null, true, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null, "HTTP")] + [InlineData("https://user:pass@localhost/", "https", "/", null, true, "localhost", 443, "GeT", "GET", "GeT", 0, null, "GET")] // Test URL sanitization + [InlineData("http://localhost:443/", "http", "/", null, true, "localhost", 443, "GET", "GET", null, 0, null, "GET")] // Test http over 443 + [InlineData("https://localhost:80/", "https", "/", null, true, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test https over 80 + [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", true, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL + [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=Redacted&q2=Redacted", false, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL + [InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", true, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL sanitization + [InlineData("http://localhost:80/Index", "http", "/Index", null, true, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}", "GET {controller}/{action}/{id}")] + [InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, true, "localhost", 443, "HEAD", "HEAD", null, 2, "about_attr_route/{customerId}", "HEAD about_attr_route/{customerId}")] + [InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, true, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}", "GET api/{controller}/{id}")] + [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, true, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}", "GET subroute/{customerId}")] + [InlineData("http://localhost/api/value", "http", "/api/value", null, true, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "/api/value")] // Request will be filtered + [InlineData("http://localhost/api/value", "http", "/api/value", null, true, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "{ThrowException}")] // Filter user code will throw an exception + [InlineData("http://localhost/", "http", "/", null, true, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, null, true, "System.InvalidOperationException")] // Test RecordException option public void AspNetRequestsAreCollectedSuccessfully( string url, string expectedUrlScheme, string expectedUrlPath, string expectedUrlQuery, + bool disableQueryRedaction, string expectedHost, int expectedPort, string requestMethod, @@ -53,113 +56,125 @@ public void AspNetRequestsAreCollectedSuccessfully( bool recordException = false, string? expectedErrorType = null) { - HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate, requestMethod); + try + { + if (disableQueryRedaction) + { + Environment.SetEnvironmentVariable("OTEL_DOTNET_EXPERIMENTAL_ASPNET_DISABLE_URL_QUERY_REDACTION", "true"); + } - typeof(HttpRequest).GetField("_wr", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(HttpContext.Current.Request, new TestHttpWorkerRequest()); + HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate, requestMethod); - List exportedItems = new List(16); + typeof(HttpRequest).GetField("_wr", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(HttpContext.Current.Request, new TestHttpWorkerRequest()); - Sdk.SetDefaultTextMapPropagator(new TraceContextPropagator()); - using (Sdk.CreateTracerProviderBuilder() - .AddAspNetInstrumentation((options) => - { - options.Filter = httpContext => - { - Assert.True(Activity.Current!.IsAllDataRequested); - if (string.IsNullOrEmpty(filter)) - { - return true; - } + List exportedItems = new List(16); - if (filter == "{ThrowException}") + Sdk.SetDefaultTextMapPropagator(new TraceContextPropagator()); + using (Sdk.CreateTracerProviderBuilder() + .AddAspNetInstrumentation((options) => + { + options.Filter = httpContext => { - throw new InvalidOperationException(); - } + Assert.True(Activity.Current!.IsAllDataRequested); + if (string.IsNullOrEmpty(filter)) + { + return true; + } + + if (filter == "{ThrowException}") + { + throw new InvalidOperationException(); + } + + return httpContext.Request.Path != filter; + }; + + options.Enrich = GetEnrichmentAction(setStatusToErrorInEnrich ? ActivityStatusCode.Error : default); + + options.RecordException = recordException; + }) + .AddInMemoryExporter(exportedItems) + .Build()) + { + using var inMemoryEventListener = new InMemoryEventListener(AspNetInstrumentationEventSource.Log); - return httpContext.Request.Path != filter; - }; + var activity = ActivityHelper.StartAspNetActivity(Propagators.DefaultTextMapPropagator, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStartedCallback); - options.Enrich = GetEnrichmentAction(setStatusToErrorInEnrich ? ActivityStatusCode.Error : default); + if (filter == "{ThrowException}") + { + Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 2)); + } - options.RecordException = recordException; - }) - .AddInMemoryExporter(exportedItems) - .Build()) - { - using var inMemoryEventListener = new InMemoryEventListener(AspNetInstrumentationEventSource.Log); + Assert.Equal(TelemetryHttpModule.AspNetActivityName, Activity.Current!.OperationName); - var activity = ActivityHelper.StartAspNetActivity(Propagators.DefaultTextMapPropagator, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStartedCallback); + if (recordException) + { + ActivityHelper.WriteActivityException(activity, HttpContext.Current, new InvalidOperationException(), TelemetryHttpModule.Options.OnExceptionCallback); + } - if (filter == "{ThrowException}") - { - Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 2)); + ActivityHelper.StopAspNetActivity(Propagators.DefaultTextMapPropagator, activity, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStoppedCallback); } - Assert.Equal(TelemetryHttpModule.AspNetActivityName, Activity.Current!.OperationName); - - if (recordException) + if (HttpContext.Current.Request.Path == filter || filter == "{ThrowException}") { - ActivityHelper.WriteActivityException(activity, HttpContext.Current, new InvalidOperationException(), TelemetryHttpModule.Options.OnExceptionCallback); + Assert.Empty(exportedItems); + return; } - ActivityHelper.StopAspNetActivity(Propagators.DefaultTextMapPropagator, activity, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStoppedCallback); - } - - if (HttpContext.Current.Request.Path == filter || filter == "{ThrowException}") - { - Assert.Empty(exportedItems); - return; - } + Assert.Single(exportedItems); - Assert.Single(exportedItems); + Activity span = exportedItems[0]; - Activity span = exportedItems[0]; + Assert.Equal(TelemetryHttpModule.AspNetActivityName, span.OperationName); + Assert.NotEqual(TimeSpan.Zero, span.Duration); - Assert.Equal(TelemetryHttpModule.AspNetActivityName, span.OperationName); - Assert.NotEqual(TimeSpan.Zero, span.Duration); + Assert.Equal(expectedName, span.DisplayName); + Assert.Equal(ActivityKind.Server, span.Kind); + Assert.True(span.Duration != TimeSpan.Zero); - Assert.Equal(expectedName, span.DisplayName); - Assert.Equal(ActivityKind.Server, span.Kind); - Assert.True(span.Duration != TimeSpan.Zero); + Assert.Equal(200, span.GetTagValue("http.response.status_code")); - Assert.Equal(200, span.GetTagValue("http.response.status_code")); + Assert.Equal(expectedHost, span.GetTagValue("server.address")); + Assert.Equal(expectedPort, span.GetTagValue("server.port")); - Assert.Equal(expectedHost, span.GetTagValue("server.address")); - Assert.Equal(expectedPort, span.GetTagValue("server.port")); + Assert.Equal(expectedRequestMethod, span.GetTagValue("http.request.method")); + Assert.Equal(expectedOriginalRequestMethod, span.GetTagValue("http.request.method_original")); + Assert.Equal("FakeHTTP/123", span.GetTagValue("network.protocol.version")); - Assert.Equal(expectedRequestMethod, span.GetTagValue("http.request.method")); - Assert.Equal(expectedOriginalRequestMethod, span.GetTagValue("http.request.method_original")); - Assert.Equal("FakeHTTP/123", span.GetTagValue("network.protocol.version")); + Assert.Equal(expectedUrlPath, span.GetTagValue("url.path")); + Assert.Equal(expectedUrlQuery, span.GetTagValue("url.query")); + Assert.Equal(expectedUrlScheme, span.GetTagValue("url.scheme")); + Assert.Equal("Custom User Agent v1.2.3", span.GetTagValue("user_agent.original")); + Assert.Equal(expectedErrorType, span.GetTagValue("error.type")); - Assert.Equal(expectedUrlPath, span.GetTagValue("url.path")); - Assert.Equal(expectedUrlQuery, span.GetTagValue("url.query")); - Assert.Equal(expectedUrlScheme, span.GetTagValue("url.scheme")); - Assert.Equal("Custom User Agent v1.2.3", span.GetTagValue("user_agent.original")); - Assert.Equal(expectedErrorType, span.GetTagValue("error.type")); + if (recordException) + { + var status = span.Status; + Assert.Equal(ActivityStatusCode.Error, span.Status); + Assert.Equal("Operation is not valid due to the current state of the object.", span.StatusDescription); + } + else if (setStatusToErrorInEnrich) + { + // This validates that users can override the + // status in Enrich. + Assert.Equal(ActivityStatusCode.Error, span.Status); - if (recordException) - { - var status = span.Status; - Assert.Equal(ActivityStatusCode.Error, span.Status); - Assert.Equal("Operation is not valid due to the current state of the object.", span.StatusDescription); - } - else if (setStatusToErrorInEnrich) - { - // This validates that users can override the - // status in Enrich. - Assert.Equal(ActivityStatusCode.Error, span.Status); + // Instrumentation is not expected to set status description + // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode + Assert.True(string.IsNullOrEmpty(span.StatusDescription)); + } + else + { + Assert.Equal(ActivityStatusCode.Unset, span.Status); - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.True(string.IsNullOrEmpty(span.StatusDescription)); + // Instrumentation is not expected to set status description + // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode + Assert.True(string.IsNullOrEmpty(span.StatusDescription)); + } } - else + finally { - Assert.Equal(ActivityStatusCode.Unset, span.Status); - - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.True(string.IsNullOrEmpty(span.StatusDescription)); + Environment.SetEnvironmentVariable("OTEL_DOTNET_EXPERIMENTAL_ASPNET_DISABLE_URL_QUERY_REDACTION", null); } } diff --git a/test/OpenTelemetry.Instrumentation.Owin.Tests/DiagnosticsMiddlewareTests.cs b/test/OpenTelemetry.Instrumentation.Owin.Tests/DiagnosticsMiddlewareTests.cs index d2111ab468..71e57081a1 100644 --- a/test/OpenTelemetry.Instrumentation.Owin.Tests/DiagnosticsMiddlewareTests.cs +++ b/test/OpenTelemetry.Instrumentation.Owin.Tests/DiagnosticsMiddlewareTests.cs @@ -281,6 +281,63 @@ Owin has finished to inspect the activity status. */ } } + [Theory] + [InlineData("path?a=b&c=d", "path?a=Redacted&c=Redacted", false)] + [InlineData("path?a=b&c=d", "path?a=b&c=d", true)] + public async Task QueryParametersAreRedacted(string actualPath, string expectedPath, bool disableQueryRedaction) + { + try + { + if (disableQueryRedaction) + { + Environment.SetEnvironmentVariable("OTEL_DOTNET_EXPERIMENTAL_OWIN_DISABLE_URL_QUERY_REDACTION", "true"); + } + + List stoppedActivities = new List(); + + var builder = Sdk.CreateTracerProviderBuilder() + .AddInMemoryExporter(stoppedActivities) + .AddOwinInstrumentation() + .Build(); + + using HttpClient client = new HttpClient(); + + Uri requestUri = new Uri($"{this.serviceBaseUri}{actualPath}"); + Uri expectedRequestUri = new Uri($"{this.serviceBaseUri}{expectedPath}"); + + this.requestCompleteHandle.Reset(); + + try + { + using var response = await client.GetAsync(requestUri).ConfigureAwait(false); + } + catch + { + } + + /* Note: This code will continue executing as soon as the response + is available but Owin could still be working. We need to wait until + Owin has finished to inspect the activity status. */ + + Assert.True(this.requestCompleteHandle.WaitOne(3000)); + + Assert.NotEmpty(stoppedActivities); + Assert.Single(stoppedActivities); + + Activity activity = stoppedActivities[0]; + Assert.Equal(OwinInstrumentationActivitySource.IncomingRequestActivityName, activity.OperationName); + + Assert.Equal(requestUri.Host + ":" + requestUri.Port, activity.TagObjects.FirstOrDefault(t => t.Key == SemanticConventions.AttributeHttpHost).Value); + Assert.Equal("GET", activity.TagObjects.FirstOrDefault(t => t.Key == SemanticConventions.AttributeHttpMethod).Value); + Assert.Equal(requestUri.AbsolutePath, activity.TagObjects.FirstOrDefault(t => t.Key == SemanticConventions.AttributeHttpTarget).Value); + Assert.Equal(expectedRequestUri.ToString(), activity.TagObjects.FirstOrDefault(t => t.Key == SemanticConventions.AttributeHttpUrl).Value); + } + finally + { + Environment.SetEnvironmentVariable("OTEL_DOTNET_EXPERIMENTAL_OWIN_DISABLE_URL_QUERY_REDACTION", null); + } + } + private List GetMetricPoints(Metric metric) { List metricPoints = new();