[Instrumentation.Http][Instrumentation.AspNetCore] Fix url.full and…

… `url.query` attribute values (#5532)

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

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;
+ }
  }
 
  /// <summary>
@@ -94,4 +102,14 @@ internal AspNetCoreTraceInstrumentationOptions(IConfiguration configuration)
  /// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md.
  /// </remarks>
  internal bool EnableGrpcAspNetCoreSupport { get; set; }
+
+ /// <summary>
+ /// Gets or sets a value indicating whether the url query value should be redacted or not.
+ /// </summary>
+ /// <remarks>
+ /// 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 <see langword="true" />.
+ /// </remarks>
+ internal bool DisableUrlQueryRedaction { get; set; }
 }

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
 

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);

src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj

@@ -17,6 +17,7 @@
  <Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\GrpcTagHelper.cs" Link="Includes\GrpcTagHelper.cs" />
  <Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\StatusCanonicalCode.cs" Link="Includes\StatusCanonicalCode.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
+ <Compile Include="$(RepoRoot)\src\Shared\RedactionHelper.cs" Link="Includes\RedactionHelper.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\RequestMethodHelper.cs" Link="Includes\RequestMethodHelper.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />

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

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
 

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

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;
 /// </summary>
 public class HttpClientTraceInstrumentationOptions
 {
+ /// <summary>
+ /// Initializes a new instance of the <see cref="HttpClientTraceInstrumentationOptions"/> class.
+ /// </summary>
+ 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;
+ }
+ }
+
  /// <summary>
  /// 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
  /// </remarks>
  public bool RecordException { get; set; }
 
+ /// <summary>
+ /// Gets or sets a value indicating whether the url query value should be redacted or not.
+ /// </summary>
+ /// <remarks>
+ /// 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 <see langword="true" />.
+ /// </remarks>
+ internal bool DisableUrlQueryRedaction { get; set; }
+
  [MethodImpl(MethodImplOptions.AggressiveInlining)]
  internal bool EventFilterHttpRequestMessage(string activityName, object arg1)
  {

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
  {

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.
 /// </summary>
 [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);
+ }
 }

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;
 
 /// <summary>
@@ -12,15 +14,18 @@ internal static class HttpTagHelper
  /// Gets the OpenTelemetry standard uri tag value for a span based on its request <see cref="Uri"/>.
  /// </summary>
  /// <param name="uri"><see cref="Uri"/>.</param>
+ /// <param name="disableQueryRedaction">Indicates whether query parameter should be redacted or not.</param>
  /// <returns>Span uri value.</returns>
- 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

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
  {

src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj

@@ -12,8 +12,12 @@
  </PropertyGroup>
 
  <ItemGroup>
+ <Compile Include="$(RepoRoot)\src\Shared\Configuration\*.cs" Link="Includes\Configuration\%(Filename).cs" />
+ <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
+ <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
+ <Compile Include="$(RepoRoot)\src\Shared\RedactionHelper.cs" Link="Includes\RedactionHelper.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\RequestMethodHelper.cs" Link="Includes\RequestMethodHelper.cs" />
  </ItemGroup>
 
@@ -29,6 +33,7 @@
  <ItemGroup>
  <Reference Include="System.Net.Http" Condition="'$(TargetFramework)' == '$(NetFrameworkMinimumSupportedVersion)'" />
  <PackageReference Include="Microsoft.Extensions.Options" />
+ <PackageReference Include="Microsoft.Extensions.Configuration" />
  </ItemGroup>
 
 </Project>

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.

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();
+ }
+}