From 137e43b8e241bdfc3001fd626ad6253a501df64d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 15 May 2024 18:28:37 +0200 Subject: [PATCH] [Instrumentation.AspNetCore, Instrumentation.Http] Revert support for OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS (#1754) --- .../Implementation/HttpInListener.cs | 2 +- .../Implementation/HttpInMetricsListener.cs | 2 +- .../Implementation/TelemetryHelper.cs | 2 +- .../Implementation/HttpTagHelper.cs | 2 +- src/Shared/RequestDataHelper.cs | 6 +++--- .../RequestDataHelperTests.cs | 13 +++++++++++-- 6 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index af6780f665..e31b10327d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -14,7 +14,7 @@ internal sealed class HttpInListener : IDisposable { private readonly HttpRequestRouteHelper routeHelper = new(); private readonly AspNetTraceInstrumentationOptions options; - private readonly RequestDataHelper requestDataHelper = new(); + private readonly RequestDataHelper requestDataHelper = new(configureByHttpKnownMethodsEnvironmentalVariable: true); public HttpInListener(AspNetTraceInstrumentationOptions options) { diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs index b50ce09959..533cae7f62 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs @@ -13,7 +13,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation; internal sealed class HttpInMetricsListener : IDisposable { private readonly HttpRequestRouteHelper routeHelper = new(); - private readonly RequestDataHelper requestDataHelper = new(); + private readonly RequestDataHelper requestDataHelper = new(configureByHttpKnownMethodsEnvironmentalVariable: true); private readonly Histogram httpServerDuration; private readonly AspNetMetricsInstrumentationOptions options; diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/TelemetryHelper.cs index 4e8cd55524..ad51848e85 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/TelemetryHelper.cs @@ -8,7 +8,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation; internal static class TelemetryHelper { public static readonly object[] BoxedStatusCodes = InitializeBoxedStatusCodes(); - internal static readonly RequestDataHelper RequestDataHelper = new(); + internal static readonly RequestDataHelper RequestDataHelper = new(configureByHttpKnownMethodsEnvironmentalVariable: false); public static object GetBoxedStatusCode(int statusCode) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs index 79815cf576..81dcec230a 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs @@ -10,7 +10,7 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation; /// internal static class HttpTagHelper { - internal static readonly RequestDataHelper RequestDataHelper = new(); + internal static readonly RequestDataHelper RequestDataHelper = new(configureByHttpKnownMethodsEnvironmentalVariable: false); /// /// Gets the OpenTelemetry standard uri tag value for a span based on its request . diff --git a/src/Shared/RequestDataHelper.cs b/src/Shared/RequestDataHelper.cs index 2325165a70..06c0c40c32 100644 --- a/src/Shared/RequestDataHelper.cs +++ b/src/Shared/RequestDataHelper.cs @@ -33,10 +33,10 @@ internal sealed class RequestDataHelper private readonly Dictionary knownHttpMethods; #endif - public RequestDataHelper() + public RequestDataHelper(bool configureByHttpKnownMethodsEnvironmentalVariable) { - var suppliedKnownMethods = Environment.GetEnvironmentVariable(KnownHttpMethodsEnvironmentVariable) - ?.Split(SplitChars, StringSplitOptions.RemoveEmptyEntries); + var suppliedKnownMethods = configureByHttpKnownMethodsEnvironmentalVariable ? Environment.GetEnvironmentVariable(KnownHttpMethodsEnvironmentVariable) + ?.Split(SplitChars, StringSplitOptions.RemoveEmptyEntries) : null; var knownMethodSet = suppliedKnownMethods?.Length > 0 ? suppliedKnownMethods.ToDictionary(x => x, x => x, StringComparer.OrdinalIgnoreCase) : new(StringComparer.OrdinalIgnoreCase) diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs index 5d4c7cbf89..b4e4c3c699 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs @@ -34,7 +34,7 @@ public class RequestDataHelperTests : IDisposable [InlineData("invalid", "_OTHER")] public void MethodMappingWorksForKnownMethods(string method, string expected) { - var requestHelper = new RequestDataHelper(); + var requestHelper = new RequestDataHelper(configureByHttpKnownMethodsEnvironmentalVariable: true); var actual = requestHelper.GetNormalizedHttpMethod(method); Assert.Equal(expected, actual); } @@ -55,11 +55,20 @@ public void MethodMappingWorksForKnownMethods(string method, string expected) public void MethodMappingWorksForEnvironmentVariables(string method, string expected) { Environment.SetEnvironmentVariable("OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS", "GET,POST"); - var requestHelper = new RequestDataHelper(); + var requestHelper = new RequestDataHelper(configureByHttpKnownMethodsEnvironmentalVariable: true); var actual = requestHelper.GetNormalizedHttpMethod(method); Assert.Equal(expected, actual); } + [Fact] + public void MethodMappingWorksIfEnvironmentalVariableConfigurationIsDisabled() + { + Environment.SetEnvironmentVariable("OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS", "GET,POST"); + var requestHelper = new RequestDataHelper(configureByHttpKnownMethodsEnvironmentalVariable: false); + var actual = requestHelper.GetNormalizedHttpMethod("CONNECT"); + Assert.Equal("CONNECT", actual); + } + [Theory] [InlineData("HTTP/1.1", "1.1")] [InlineData("HTTP/2", "2")]