From e311af5b1cfd7db75cee33c19c67556ca61a18ae Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Fri, 27 Oct 2023 15:24:44 -0700 Subject: [PATCH 01/10] [HttpClient & HttpWebRequest]update request.method --- .../HttpHandlerDiagnosticListener.cs | 11 +- .../HttpHandlerMetricsDiagnosticListener.cs | 3 +- .../HttpWebRequestActivitySource.netfx.cs | 14 +- .../Implementation/TelemetryHelper.cs | 45 ++++++ .../HttpClientTests.Basic.cs | 138 ++++++++++++++++++ .../HttpWebRequestTests.Basic.cs | 58 ++++++++ 6 files changed, 265 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 40bf9a3fab6..088519fb835 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -185,7 +185,16 @@ 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 (TelemetryHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod)) + { + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); + } + else + { + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); + activity.SetTag("http.request.method_original", request.Method.Method); + } + activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); if (!request.RequestUri.IsDefaultPort) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 89266ce9139..1ba60e84d3b 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -97,7 +97,8 @@ public override void OnEventWritten(string name, object payload) { TagList tags = default; - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method))); + TelemetryHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod); + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); tags.Add(new KeyValuePair(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 index 2cb52a8bbd6..33e43fc665c 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -153,7 +153,16 @@ 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 (TelemetryHelper.TryResolveHttpMethod(request.Method, out var httpMethod)) + { + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); + } + else + { + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); + activity.SetTag("http.request.method_original", request.Method); + } + activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); if (!request.RequestUri.IsDefaultPort) { @@ -495,7 +504,8 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC { TagList tags = default; - tags.Add(SemanticConventions.AttributeHttpRequestMethod, request.Method); + TelemetryHelper.TryResolveHttpMethod(request.Method, out var httpMethod); + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); 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/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs index 9b53a5bc5f6..6c6d50beae4 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -22,6 +22,36 @@ internal static class TelemetryHelper { public static readonly object[] BoxedStatusCodes; +#if NET8_0_OR_GREATER + internal static readonly FrozenDictionary 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 + internal static readonly Dictionary KnownMethods = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "GET", "GET" }, + { "PUT", "PUT" }, + { "POST", "POST" }, + { "DELETE", "DELETE" }, + { "HEAD", "HEAD" }, + { "OPTIONS", "OPTIONS" }, + { "TRACE", "TRACE" }, + { "PATCH", "PATCH" }, + { "CONNECT", "CONNECT" }, + }; +#endif + static TelemetryHelper() { BoxedStatusCodes = new object[500]; @@ -41,4 +71,19 @@ public static object GetBoxedStatusCode(HttpStatusCode statusCode) return intStatusCode; } + + public static bool TryResolveHttpMethod(string method, out string resolvedMethod) + { + if (KnownMethods.TryGetValue(method, out var result)) + { + // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case. + resolvedMethod = result; + 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; + } } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index e039f25ac12..0222174e281 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -15,6 +15,7 @@ // using System.Diagnostics; +using Microsoft.Extensions.Configuration; #if NETFRAMEWORK using System.Net; using System.Net.Http; @@ -23,11 +24,14 @@ using Moq; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Http.Implementation; +using OpenTelemetry.Metrics; using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; using Xunit.Abstractions; +using static OpenTelemetry.Internal.HttpSemanticConventionHelper; + namespace OpenTelemetry.Instrumentation.Http.Tests; public partial class HttpClientTests : IDisposable @@ -368,6 +372,140 @@ public async Task ExportsSpansCreatedForRetries() Assert.NotEqual(spanid2, spanid3); } + [Theory] + [InlineData("CONNECT")] + [InlineData("DELETE")] + [InlineData("GET")] + [InlineData("PUT")] + [InlineData("HEAD")] + [InlineData("OPTIONS")] + [InlineData("PATCH")] + [InlineData("Get")] + [InlineData("POST")] + [InlineData("TRACE")] + [InlineData("CUSTOM")] + public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string method) + { + var exportedItems = new List(); + using var request = new HttpRequestMessage + { + RequestUri = new Uri(this.url), + Method = new HttpMethod(method), + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) + .Build(); + + using var traceprovider = Sdk.CreateTracerProviderBuilder() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddHttpClientInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + using var httpClient = new HttpClient(); + + try + { + await httpClient.SendAsync(request).ConfigureAwait(false); + } + catch + { + // ignore error. + } + + Assert.Single(exportedItems); + + var activity = exportedItems[0]; + + if (TelemetryHelper.KnownMethods.TryGetValue(method, out var val)) + { + Assert.Equal(val, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.DoesNotContain(activity.TagObjects, t => t.Key == "http.request.method_original"); + } + else + { + Assert.Equal("_OTHER", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(method, activity.GetTagValue("http.request.method_original") as string); + } + } + + [Theory] + [InlineData("CONNECT")] + [InlineData("DELETE")] + [InlineData("GET")] + [InlineData("PUT")] + [InlineData("HEAD")] + [InlineData("OPTIONS")] + [InlineData("PATCH")] + [InlineData("Get")] + [InlineData("POST")] + [InlineData("TRACE")] + [InlineData("CUSTOM")] + public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string method) + { + var metricItems = new List(); + using var request = new HttpRequestMessage + { + RequestUri = new Uri(this.url), + Method = new HttpMethod(method), + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) + .Build(); + + using var meterprovider = Sdk.CreateMeterProviderBuilder() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddHttpClientInstrumentation() + .AddInMemoryExporter(metricItems) + .Build(); + + using var httpClient = new HttpClient(); + + try + { + await httpClient.SendAsync(request).ConfigureAwait(false); + } + catch + { + // ignore error. + } + + meterprovider.Dispose(); + + var metric = metricItems.FirstOrDefault(m => m.Name == "http.client.request.duration"); + + Assert.NotNull(metric); + + var metricPoints = new List(); + foreach (var p in metric.GetMetricPoints()) + { + metricPoints.Add(p); + } + + Assert.Single(metricPoints); + var mp = metricPoints[0]; + + // Inspect Metric Attributes + var attributes = new Dictionary(); + foreach (var tag in mp.Tags) + { + attributes[tag.Key] = tag.Value; + } + + if (TelemetryHelper.KnownMethods.TryGetValue(method, out var val)) + { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == method.ToUpper()); + } + else + { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == "_OTHER"); + } + + Assert.DoesNotContain(attributes, t => t.Key == "http.request.method_original"); + } + [Fact] public async Task RedirectTest() { diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs index 85ab9988335..ae142e304eb 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs @@ -16,6 +16,7 @@ using System.Diagnostics; using System.Net; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Moq; @@ -25,6 +26,8 @@ using OpenTelemetry.Trace; using Xunit; +using static OpenTelemetry.Internal.HttpSemanticConventionHelper; + #pragma warning disable SYSLIB0014 // Type or member is obsolete namespace OpenTelemetry.Instrumentation.Http.Tests; @@ -357,6 +360,61 @@ public async Task ReportsExceptionEventForNetworkFailures() Assert.Single(exportedItems[0].Events.Where(evt => evt.Name.Equals("exception"))); } + [Theory] + [InlineData("CONNECT")] + [InlineData("DELETE")] + [InlineData("GET")] + [InlineData("PUT")] + [InlineData("HEAD")] + [InlineData("OPTIONS")] + [InlineData("PATCH")] + [InlineData("Get")] + [InlineData("POST")] + [InlineData("TRACE")] + [InlineData("CUSTOM")] + public async Task HttpRequestMethodIsSetAsPerSpec(string method) + { + var exportedItems = new List(); + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) + .Build(); + + using var traceprovider = Sdk.CreateTracerProviderBuilder() + .ConfigureServices(services => services.AddSingleton(configuration)) + .AddHttpClientInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + try + { + var request = (HttpWebRequest)WebRequest.Create(this.url); + + request.Method = method; + + using var response = await request.GetResponseAsync().ConfigureAwait(false); + } + catch + { + // ignore error. + } + + Assert.Single(exportedItems); + + var activity = exportedItems[0]; + + if (TelemetryHelper.KnownMethods.TryGetValue(method, out var val)) + { + Assert.Equal(val, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.DoesNotContain(activity.TagObjects, t => t.Key == "http.request.method_original"); + } + else + { + Assert.Equal("_OTHER", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(method, activity.GetTagValue("http.request.method_original") as string); + } + } + [Fact] public async Task ReportsExceptionEventOnErrorResponse() { From 83c0242eecec9f9abbc9158a0690b3ef28b7058c Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Fri, 27 Oct 2023 15:33:37 -0700 Subject: [PATCH 02/10] rmv dup test --- .../HttpWebRequestTests.Basic.cs | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs index ae142e304eb..714a5643883 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs @@ -360,61 +360,6 @@ public async Task ReportsExceptionEventForNetworkFailures() Assert.Single(exportedItems[0].Events.Where(evt => evt.Name.Equals("exception"))); } - [Theory] - [InlineData("CONNECT")] - [InlineData("DELETE")] - [InlineData("GET")] - [InlineData("PUT")] - [InlineData("HEAD")] - [InlineData("OPTIONS")] - [InlineData("PATCH")] - [InlineData("Get")] - [InlineData("POST")] - [InlineData("TRACE")] - [InlineData("CUSTOM")] - public async Task HttpRequestMethodIsSetAsPerSpec(string method) - { - var exportedItems = new List(); - - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) - .Build(); - - using var traceprovider = Sdk.CreateTracerProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddHttpClientInstrumentation() - .AddInMemoryExporter(exportedItems) - .Build(); - - try - { - var request = (HttpWebRequest)WebRequest.Create(this.url); - - request.Method = method; - - using var response = await request.GetResponseAsync().ConfigureAwait(false); - } - catch - { - // ignore error. - } - - Assert.Single(exportedItems); - - var activity = exportedItems[0]; - - if (TelemetryHelper.KnownMethods.TryGetValue(method, out var val)) - { - Assert.Equal(val, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); - Assert.DoesNotContain(activity.TagObjects, t => t.Key == "http.request.method_original"); - } - else - { - Assert.Equal("_OTHER", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); - Assert.Equal(method, activity.GetTagValue("http.request.method_original") as string); - } - } - [Fact] public async Task ReportsExceptionEventOnErrorResponse() { From b62e6313c72bd9a0eed23af07701c6404434f4b6 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Fri, 27 Oct 2023 15:45:22 -0700 Subject: [PATCH 03/10] fix build --- .../Implementation/TelemetryHelper.cs | 3 +++ .../HttpWebRequestTests.Basic.cs | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs index 6c6d50beae4..d08bf5174f3 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -15,6 +15,9 @@ // using System.Net; +#if NET8_0_OR_GREATER +using System.Collections.Frozen; +#endif namespace OpenTelemetry.Instrumentation.Http.Implementation; diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs index 714a5643883..85ab9988335 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.cs @@ -16,7 +16,6 @@ using System.Diagnostics; using System.Net; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Moq; @@ -26,8 +25,6 @@ using OpenTelemetry.Trace; using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - #pragma warning disable SYSLIB0014 // Type or member is obsolete namespace OpenTelemetry.Instrumentation.Http.Tests; From 795d547feba6289c8436b3b72520ff864ee4fa1d Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 30 Oct 2023 09:22:36 -0700 Subject: [PATCH 04/10] add changelog --- .../CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 8229a7becd2..36ed1c380a0 100644 --- 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, request method will be set on an additional tag + `http.request.method.original` will be added 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) + and `http.request.method` will be set to `_OTHER`. + * For metrics, `http.request.method` on `http.client.request.duration` metric + will be set to `_OTHER` 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) + + `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 From a529ffb03d870bc5b33323ad0c609b2a34ab0689 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 31 Oct 2023 13:28:38 -0700 Subject: [PATCH 05/10] refactor --- .../CHANGELOG.md | 14 ++-- .../HttpHandlerDiagnosticListener.cs | 5 +- .../HttpHandlerMetricsDiagnosticListener.cs | 3 +- .../HttpWebRequestActivitySource.netfx.cs | 7 +- .../Implementation/TelemetryHelper.cs | 48 ------------ .../MeterProviderBuilderExtensions.cs | 3 +- .../OpenTelemetry.Instrumentation.Http.csproj | 1 + .../TracerProviderBuilderExtensions.cs | 3 +- .../HttpClientTests.Basic.cs | 75 +++++++++---------- 9 files changed, 56 insertions(+), 103 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 36ed1c380a0..4b7bbb8cd3e 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -3,15 +3,15 @@ ## Unreleased * Updated `http.request.method` to match specification guidelines. - * For activity, request method will be set on an additional tag - `http.request.method.original` will be added if the method does not belong - to one of the [known + * 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) - and `http.request.method` will be set to `_OTHER`. - * For metrics, `http.request.method` on `http.client.request.duration` metric - will be set to `_OTHER` if the original method does not belong to one of the - [known + 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 diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 088519fb835..07e5218e527 100644 --- 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,14 +186,14 @@ 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) { - if (TelemetryHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod)) + if (RequestMethodHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod)) { activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); } else { activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); - activity.SetTag("http.request.method_original", request.Method.Method); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method.Method); } activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 1ba60e84d3b..4dfb1d473f6 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/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,7 @@ public override void OnEventWritten(string name, object payload) { TagList tags = default; - TelemetryHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod); + RequestMethodHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 33e43fc665c..86e9e344e14 100644 --- 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,14 +154,14 @@ 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) { - if (TelemetryHelper.TryResolveHttpMethod(request.Method, out var httpMethod)) + if (RequestMethodHelper.TryResolveHttpMethod(request.Method, out var httpMethod)) { activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); } else { activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); - activity.SetTag("http.request.method_original", request.Method); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method); } activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); @@ -504,7 +505,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC { TagList tags = default; - TelemetryHelper.TryResolveHttpMethod(request.Method, out var httpMethod); + RequestMethodHelper.TryResolveHttpMethod(request.Method, out var httpMethod); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs index d08bf5174f3..9b53a5bc5f6 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -15,9 +15,6 @@ // using System.Net; -#if NET8_0_OR_GREATER -using System.Collections.Frozen; -#endif namespace OpenTelemetry.Instrumentation.Http.Implementation; @@ -25,36 +22,6 @@ internal static class TelemetryHelper { public static readonly object[] BoxedStatusCodes; -#if NET8_0_OR_GREATER - internal static readonly FrozenDictionary 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 - internal static readonly Dictionary KnownMethods = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "GET", "GET" }, - { "PUT", "PUT" }, - { "POST", "POST" }, - { "DELETE", "DELETE" }, - { "HEAD", "HEAD" }, - { "OPTIONS", "OPTIONS" }, - { "TRACE", "TRACE" }, - { "PATCH", "PATCH" }, - { "CONNECT", "CONNECT" }, - }; -#endif - static TelemetryHelper() { BoxedStatusCodes = new object[500]; @@ -74,19 +41,4 @@ public static object GetBoxedStatusCode(HttpStatusCode statusCode) return intStatusCode; } - - public static bool TryResolveHttpMethod(string method, out string resolvedMethod) - { - if (KnownMethods.TryGetValue(method, out var result)) - { - // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case. - resolvedMethod = result; - 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; - } } diff --git a/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs index 0890a662c8b..73b7ca44c0a 100644 --- 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 index c252a0b5e26..1e18cd01855 100644 --- a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj +++ b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj @@ -16,6 +16,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs index 9429819b8a2..f8bb13a615c 100644 --- 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/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index 0222174e281..ea55780d955 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -373,24 +373,24 @@ public async Task ExportsSpansCreatedForRetries() } [Theory] - [InlineData("CONNECT")] - [InlineData("DELETE")] - [InlineData("GET")] - [InlineData("PUT")] - [InlineData("HEAD")] - [InlineData("OPTIONS")] - [InlineData("PATCH")] - [InlineData("Get")] - [InlineData("POST")] - [InlineData("TRACE")] - [InlineData("CUSTOM")] - public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string method) + [InlineData("CONNECT", "CONNECT")] + [InlineData("DELETE", "DELETE")] + [InlineData("GET", "GET")] + [InlineData("PUT", "PUT")] + [InlineData("HEAD", "HEAD")] + [InlineData("OPTIONS", "OPTIONS")] + [InlineData("PATCH", "PATCH")] + [InlineData("Get", "GET")] + [InlineData("POST", "POST")] + [InlineData("TRACE", "TRACE")] + [InlineData("CUSTOM", "_OTHER")] + public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMethod, string expectedMethod) { var exportedItems = new List(); using var request = new HttpRequestMessage { RequestUri = new Uri(this.url), - Method = new HttpMethod(method), + Method = new HttpMethod(originalMethod), }; var configuration = new ConfigurationBuilder() @@ -418,37 +418,39 @@ public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string method) var activity = exportedItems[0]; - if (TelemetryHelper.KnownMethods.TryGetValue(method, out var val)) + Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethod); + + if (originalMethod.Equals(expectedMethod, StringComparison.OrdinalIgnoreCase)) { - Assert.Equal(val, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); - Assert.DoesNotContain(activity.TagObjects, t => t.Key == "http.request.method_original"); + Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal); } else { - Assert.Equal("_OTHER", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); - Assert.Equal(method, activity.GetTagValue("http.request.method_original") as string); + Assert.Equal(originalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); } + + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); } [Theory] - [InlineData("CONNECT")] - [InlineData("DELETE")] - [InlineData("GET")] - [InlineData("PUT")] - [InlineData("HEAD")] - [InlineData("OPTIONS")] - [InlineData("PATCH")] - [InlineData("Get")] - [InlineData("POST")] - [InlineData("TRACE")] - [InlineData("CUSTOM")] - public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string method) + [InlineData("CONNECT", "CONNECT")] + [InlineData("DELETE", "DELETE")] + [InlineData("GET", "GET")] + [InlineData("PUT", "PUT")] + [InlineData("HEAD", "HEAD")] + [InlineData("OPTIONS", "OPTIONS")] + [InlineData("PATCH", "PATCH")] + [InlineData("Get", "GET")] + [InlineData("POST", "POST")] + [InlineData("TRACE", "TRACE")] + [InlineData("CUSTOM", "_OTHER")] + public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string originalMethod, string expectedMethod) { var metricItems = new List(); using var request = new HttpRequestMessage { RequestUri = new Uri(this.url), - Method = new HttpMethod(method), + Method = new HttpMethod(originalMethod), }; var configuration = new ConfigurationBuilder() @@ -494,16 +496,9 @@ public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string attributes[tag.Key] = tag.Value; } - if (TelemetryHelper.KnownMethods.TryGetValue(method, out var val)) - { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == method.ToUpper()); - } - else - { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == "_OTHER"); - } + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == expectedMethod); - Assert.DoesNotContain(attributes, t => t.Key == "http.request.method_original"); + Assert.DoesNotContain(attributes, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal); } [Fact] From 8d1e6fc0ad7ab1df28ed8c5a3e3af610f31fee2a Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 31 Oct 2023 15:03:15 -0700 Subject: [PATCH 06/10] refactor --- .../Implementation/HttpInListener.cs | 6 ++++-- .../Implementation/HttpInMetricsListener.cs | 14 ++++++++++---- .../HttpHandlerDiagnosticListener.cs | 6 ++++-- .../HttpHandlerMetricsDiagnosticListener.cs | 13 ++++++++++++- .../HttpWebRequestActivitySource.netfx.cs | 19 ++++++++++++++++--- src/Shared/RequestMethodHelper.cs | 15 +-------------- 6 files changed, 47 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index b074fb5c254..83ba919385b 100644 --- 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 index 987fac68e57..d78628f3b5d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -14,7 +14,6 @@ // limitations under the License. // -#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(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol))); tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, context.Request.Scheme)); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); - RequestMethodHelper.TryResolveHttpMethod(context.Request.Method, out var httpMethod); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); + if (RequestMethodHelper.KnownMethods.TryGetValue(context.Request.Method, out var httpMethod)) + { + tags.Add(new KeyValuePair(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(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/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 07e5218e527..b5d4c85fc5e 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -186,13 +186,15 @@ 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) { - if (RequestMethodHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod)) + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.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.Method); } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 4dfb1d473f6..cd8e585f8f0 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Diagnostics; #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; @@ -98,7 +99,17 @@ public override void OnEventWritten(string name, object payload) { TagList tags = default; - RequestMethodHelper.TryResolveHttpMethod(request.Method.Method, out var httpMethod); + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod)) + { + tags.Add(new KeyValuePair(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(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); + } + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 86e9e344e14..9a4e1743f75 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -22,6 +22,7 @@ using System.Reflection; using System.Reflection.Emit; using System.Runtime.CompilerServices; +using System.Runtime.Remoting.Contexts; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Internal; using OpenTelemetry.Trace; @@ -154,13 +155,15 @@ 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) { - 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); } @@ -505,7 +508,17 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC { TagList tags = default; - RequestMethodHelper.TryResolveHttpMethod(request.Method, out var httpMethod); + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod)) + { + tags.Add(new KeyValuePair(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(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); + } + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); diff --git a/src/Shared/RequestMethodHelper.cs b/src/Shared/RequestMethodHelper.cs index b4404bde770..27487ae7bd1 100644 --- a/src/Shared/RequestMethodHelper.cs +++ b/src/Shared/RequestMethodHelper.cs @@ -30,6 +30,7 @@ internal static class RequestMethodHelper static RequestMethodHelper() { + // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case. #if NET8_0_OR_GREATER KnownMethods = FrozenDictionary.ToFrozenDictionary( new[] @@ -60,18 +61,4 @@ static RequestMethodHelper() }; #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; - } } From d08a700376d9ac3732731d0b34e859553f83ea9b Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 31 Oct 2023 15:11:09 -0700 Subject: [PATCH 07/10] fix --- .../Implementation/HttpHandlerMetricsDiagnosticListener.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index cd8e585f8f0..c2d52ef8d19 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -110,7 +110,6 @@ public override void OnEventWritten(string name, object payload) tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); } - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme)); From bc0d0ae159c8de670959d7ee8901f2e9b3ab3f06 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 31 Oct 2023 15:13:06 -0700 Subject: [PATCH 08/10] fix --- .../Implementation/HttpWebRequestActivitySource.netfx.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 9a4e1743f75..33a9c14201f 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -519,7 +519,6 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); } - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion)); From 2c4890e46d141b01308a2ad8e34ad32363e2a36e Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 31 Oct 2023 15:40:36 -0700 Subject: [PATCH 09/10] fix using --- .../Implementation/HttpHandlerMetricsDiagnosticListener.cs | 1 - .../Implementation/HttpWebRequestActivitySource.netfx.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index c2d52ef8d19..eab4c6d4dc4 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -14,7 +14,6 @@ // limitations under the License. // -using System; using System.Diagnostics; #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 33a9c14201f..d2c23f6c6bf 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -22,7 +22,6 @@ using System.Reflection; using System.Reflection.Emit; using System.Runtime.CompilerServices; -using System.Runtime.Remoting.Contexts; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Internal; using OpenTelemetry.Trace; From dfb92ead3b6142d3401dd63ee6742500b5fe70ea Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 31 Oct 2023 16:27:20 -0700 Subject: [PATCH 10/10] refactor --- src/Shared/RequestMethodHelper.cs | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/Shared/RequestMethodHelper.cs b/src/Shared/RequestMethodHelper.cs index 27487ae7bd1..8ae14f89d0b 100644 --- a/src/Shared/RequestMethodHelper.cs +++ b/src/Shared/RequestMethodHelper.cs @@ -30,24 +30,7 @@ internal static class RequestMethodHelper static RequestMethodHelper() { - // KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case. -#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(StringComparer.OrdinalIgnoreCase) + var knownMethodSet = new Dictionary(StringComparer.OrdinalIgnoreCase) { { "GET", "GET" }, { "PUT", "PUT" }, @@ -59,6 +42,12 @@ static RequestMethodHelper() { "PATCH", "PATCH" }, { "CONNECT", "CONNECT" }, }; + + // 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 } }