[HttpClient & HttpWebRequest] Set http.request.method as per spec (#…

…5003)
open-telemetry · Nov 1, 2023 · d1d6d4b · d1d6d4b
1 parent 5cb7a3f
commit d1d6d4b
Show file tree

Hide file tree

Showing 11 changed files with 224 additions and 43 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
@@ -252,13 +252,15 @@ public void OnStartActivity(Activity activity, object payload)
                     activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value);
                 }
 
-                if (RequestMethodHelper.TryResolveHttpMethod(request.Method, out var httpMethod))
+                if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
                 {
                     activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
                 }
                 else
                 {
-                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
+                    // Set to default "_OTHER" as per spec.
+                    // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
                     activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method);
                 }
 

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs
@@ -14,7 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-#if !NET8_0_OR_GREATER
 using System.Diagnostics;
 using System.Diagnostics.Metrics;
 using Microsoft.AspNetCore.Http;
@@ -153,8 +152,16 @@ public void OnEventWritten_New(string name, object payload)
         tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
         tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme));
         tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
-        RequestMethodHelper.TryResolveHttpMethod(context.Request.Method, out var httpMethod);
-        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
+        if (RequestMethodHelper.KnownMethods.TryGetValue(context.Request.Method, out var httpMethod))
+        {
+            tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
+        }
+        else
+        {
+            // Set to default "_OTHER" as per spec.
+            // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
+            tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
+        }
 
 #if NET6_0_OR_GREATER
         var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
@@ -170,4 +177,3 @@ public void OnEventWritten_New(string name, object payload)
         this.httpServerRequestDuration.Record(Activity.Current.Duration.TotalSeconds, tags);
     }
 }
-#endif
diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
@@ -2,6 +2,22 @@
 
 ## Unreleased
 
+* Updated `http.request.method` to match specification guidelines.
+  * For activity, if the method does not belong to one of the [known
+    values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values)
+    then the request method will be set on an additional tag
+    `http.request.method.original` and `http.request.method` will be set to
+    `_OTHER`.
+  * For metrics, if the original method does not belong to one of the [known
+    values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values)
+    then `http.request.method` on `http.client.request.duration` metric will be
+    set to `_OTHER`
+
+  `http.request.method` is set on `http.client.request.duration` metric or
+  activity when `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable is set to
+  `http` or `http/dup`.
+  ([#5003](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5003))
+
 ## 1.6.0-beta.2
 
 Released 2023-Oct-26

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
@@ -23,6 +23,7 @@
 #endif
 using System.Reflection;
 using OpenTelemetry.Context.Propagation;
+using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
 using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
 
@@ -185,7 +186,18 @@ public void OnStartActivity(Activity activity, object payload)
             // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
             if (this.emitNewAttributes)
             {
-                activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method));
+                if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod))
+                {
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
+                }
+                else
+                {
+                    // Set to default "_OTHER" as per spec.
+                    // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method.Method);
+                }
+
                 activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
                 if (!request.RequestUri.IsDefaultPort)
                 {

diff --git a/...OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/...OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
@@ -23,6 +23,7 @@
 using System.Net.Http;
 #endif
 using System.Reflection;
+using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
 using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
 
@@ -97,7 +98,17 @@ public override void OnEventWritten(string name, object payload)
                 {
                     TagList tags = default;
 
-                    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)));
+                    if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod))
+                    {
+                        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
+                    }
+                    else
+                    {
+                        // Set to default "_OTHER" as per spec.
+                        // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
+                        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
+                    }
+
                     tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version)));
                     tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerAddress, request.RequestUri.Host));
                     tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme));

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
@@ -23,6 +23,7 @@
 using System.Reflection.Emit;
 using System.Runtime.CompilerServices;
 using OpenTelemetry.Context.Propagation;
+using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
 using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
 
@@ -153,7 +154,18 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A
             // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
             if (tracingEmitNewAttributes)
             {
-                activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, request.Method);
+                if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
+                {
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
+                }
+                else
+                {
+                    // Set to default "_OTHER" as per spec.
+                    // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
+                    activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method);
+                }
+
                 activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
                 if (!request.RequestUri.IsDefaultPort)
                 {
@@ -495,7 +507,17 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
             {
                 TagList tags = default;
 
-                tags.Add(SemanticConventions.AttributeHttpRequestMethod, request.Method);
+                if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
+                {
+                    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
+                }
+                else
+                {
+                    // Set to default "_OTHER" as per spec.
+                    // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
+                    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
+                }
+
                 tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
                 tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme);
                 tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));

diff --git a/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs
@@ -45,8 +45,9 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
             .AddMeter("System.Net.Http")
             .AddMeter("System.Net.NameResolution");
 #else
-        // Note: Warm-up the status code mapping.
+        // Note: Warm-up the status code and method mapping.
         _ = TelemetryHelper.BoxedStatusCodes;
+        _ = RequestMethodHelper.KnownMethods;
 
         builder.ConfigureServices(services =>
         {

diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj
@@ -16,6 +16,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\RequestMethodHelper.cs" Link="Includes\RequestMethodHelper.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(RunningDotNetPack)' != 'true'">

diff --git a/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs
@@ -60,8 +60,9 @@ public static TracerProviderBuilder AddHttpClientInstrumentation(
     {
         Guard.ThrowIfNull(builder);
 
-        // Note: Warm-up the status code mapping.
+        // Note: Warm-up the status code and method mapping.
         _ = TelemetryHelper.BoxedStatusCodes;
+        _ = RequestMethodHelper.KnownMethods;
 
         name ??= Options.DefaultName;
 

diff --git a/src/Shared/RequestMethodHelper.cs b/src/Shared/RequestMethodHelper.cs
@@ -30,23 +30,7 @@ internal static class RequestMethodHelper
 
     static RequestMethodHelper()
     {
-#if NET8_0_OR_GREATER
-        KnownMethods = FrozenDictionary.ToFrozenDictionary(
-            new[]
-            {
-                KeyValuePair.Create("GET", "GET"),
-                KeyValuePair.Create("PUT", "PUT"),
-                KeyValuePair.Create("POST", "POST"),
-                KeyValuePair.Create("DELETE", "DELETE"),
-                KeyValuePair.Create("HEAD", "HEAD"),
-                KeyValuePair.Create("OPTIONS", "OPTIONS"),
-                KeyValuePair.Create("TRACE", "TRACE"),
-                KeyValuePair.Create("PATCH", "PATCH"),
-                KeyValuePair.Create("CONNECT", "CONNECT"),
-            },
-            StringComparer.OrdinalIgnoreCase);
-#else
-        KnownMethods = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+        var knownMethodSet = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
         {
             { "GET", "GET" },
             { "PUT", "PUT" },
@@ -58,20 +42,12 @@ static RequestMethodHelper()
             { "PATCH", "PATCH" },
             { "CONNECT", "CONNECT" },
         };
-#endif
-    }
-
-    public static bool TryResolveHttpMethod(string method, out string resolvedMethod)
-    {
-        if (KnownMethods.TryGetValue(method, out resolvedMethod))
-        {
-            // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case.
-            return true;
-        }
 
-        // Set to default "_OTHER" as per spec.
-        // https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
-        resolvedMethod = "_OTHER";
-        return false;
+        // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case.
+#if NET8_0_OR_GREATER
+        KnownMethods = FrozenDictionary.ToFrozenDictionary(knownMethodSet, StringComparer.OrdinalIgnoreCase);
+#else
+        KnownMethods = knownMethodSet;
+#endif
     }
 }