Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Instrumentation.AspNet][Instrumentation.Owin] Redact query parameters #1656

Merged
merged 9 commits into from
Apr 17, 2024
Merged
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion opentelemetry-dotnet-contrib.sln
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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;
/// </summary>
public class AspNetTraceInstrumentationOptions
{
private const string DisableQueryRedactionEnvVar = "OTEL_DOTNET_EXPERIMENTAL_ASPNET_DISABLE_URL_QUERY_REDACTION";

/// <summary>
/// Initializes a new instance of the <see cref="AspNetTraceInstrumentationOptions"/> class.
/// </summary>
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);
}
}

/// <summary>
/// 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: <see href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md"/>.
/// </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; }
}
10 changes: 9 additions & 1 deletion src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
# Changelog

## Unreleased
## 1.8.0-beta.2

* **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

Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@
<Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\PropertyFetcher.AOT.cs" Link="Includes\PropertyFetcher.AOT.cs" />
<Compile Include="$(RepoRoot)\src\Shared\RedactionHelper.cs" Link="Includes\RedactionHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\SpanHelper.cs" Link="Includes\SpanHelper.cs" />
</ItemGroup>
10 changes: 9 additions & 1 deletion src/OpenTelemetry.Instrumentation.Owin/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
# Changelog

## Unreleased
## 1.0.0-rc.5

* **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))
Original file line number Diff line number Diff line change
@@ -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
/// </summary>
/// <param name="uri"><see cref="Uri"/>.</param>
/// <returns>Span uri value.</returns>
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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@
<Compile Include="$(RepoRoot)\src\Shared\AssemblyVersionExtensions.cs" Link="Includes\AssemblyVersionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.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\SemanticConventions.cs" Link="Includes\SemanticConventions.cs"/>
<Compile Include="$(RepoRoot)\src\Shared\SpanHelper.cs" Link="Includes\SpanHelper.cs" />
</ItemGroup>
Original file line number Diff line number Diff line change
@@ -12,6 +12,27 @@ namespace OpenTelemetry.Instrumentation.Owin;
/// </summary>
public class OwinInstrumentationOptions
{
private const string DisableQueryRedactionEnvVar = "OTEL_DOTNET_EXPERIMENTAL_OWIN_DISABLE_URL_QUERY_REDACTION";

/// <summary>
/// Initializes a new instance of the <see cref="OwinInstrumentationOptions"/> class.
/// </summary>
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);
}
}

/// <summary>
/// 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 <see cref="IOwinContext"/>, 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.
/// </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; }
}
59 changes: 59 additions & 0 deletions src/Shared/RedactionHelper.cs
Original file line number Diff line number Diff line change
@@ -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
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
{
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();
}
}
Loading