diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 5b608371d46..dc8929d3546 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -6,6 +6,9 @@ `server.address` when it has default values (`80` for `HTTP` and `443` for `HTTPS` protocol). ([#5419](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5419)) +* Fixed an issue for spans when `http.request.method_original` attribute was not + set for non canonical form of HTTP methods, e.g. for `Options`. + ([#5471](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5471)) ## 1.7.1 diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 9b7fa69f2f8..32ca4c7d0e0 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -6,6 +6,11 @@ `server.address` when it has default values (`80` for `HTTP` and `443` for `HTTPS` protocol). ([#5419](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5419)) +* Fixed an issue for spans when `http.request.method_original` attribute was not + set for non canonical form of HTTP methods, e.g. for `Options`. The attribute + is not set for non canonical form of `GET`, `HEAD`, `PUT`, and `POST`. + HTTP Client is converting these values to canonical form. + ([#5471](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5471)) ## 1.7.1 diff --git a/src/Shared/RequestMethodHelper.cs b/src/Shared/RequestMethodHelper.cs index 05a3f022511..667837061a1 100644 --- a/src/Shared/RequestMethodHelper.cs +++ b/src/Shared/RequestMethodHelper.cs @@ -51,16 +51,14 @@ public static string GetNormalizedHttpMethod(string method) : OtherHttpMethod; } - public static void SetHttpMethodTag(Activity activity, string method) + public static void SetHttpMethodTag(Activity activity, string originalHttpMethod) { - if (KnownMethods.TryGetValue(method, out var normalizedMethod)) - { - activity?.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedMethod); - } - else + var normalizedHttpMethod = GetNormalizedHttpMethod(originalHttpMethod); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod); + + if (originalHttpMethod != normalizedHttpMethod) { - activity?.SetTag(SemanticConventions.AttributeHttpRequestMethod, OtherHttpMethod); - activity?.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, method); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod); } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 80033de3711..44145005b72 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -629,18 +629,18 @@ void ConfigureTestServices(IServiceCollection services) } [Theory] - [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 HttpRequestMethodIsSetAsPerSpec(string originalMethod, string expectedMethod) + [InlineData("CONNECT", "CONNECT", null)] + [InlineData("DELETE", "DELETE", null)] + [InlineData("GET", "GET", null)] + [InlineData("PUT", "PUT", null)] + [InlineData("HEAD", "HEAD", null)] + [InlineData("OPTIONS", "OPTIONS", null)] + [InlineData("PATCH", "PATCH", null)] + [InlineData("Get", "GET", "Get")] + [InlineData("POST", "POST", null)] + [InlineData("TRACE", "TRACE", null)] + [InlineData("CUSTOM", "_OTHER", "CUSTOM")] + public async Task HttpRequestMethodIsSetAsPerSpec(string originalMethod, string expectedMethod, string expectedOriginalMethod) { var exportedItems = new List(); @@ -681,18 +681,8 @@ void ConfigureTestServices(IServiceCollection services) var activity = exportedItems[0]; - Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethod); - - if (originalMethod.Equals(expectedMethod, StringComparison.OrdinalIgnoreCase)) - { - Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal); - } - else - { - Assert.Equal(originalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); - } - - Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); + Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal)); } [Fact] diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index ba758f25103..847af5b37a8 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -356,18 +356,26 @@ public async Task ExportsSpansCreatedForRetries() } [Theory] - [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) + [InlineData("CONNECT", "CONNECT", null)] + [InlineData("DELETE", "DELETE", null)] + [InlineData("GET", "GET", null)] + [InlineData("PUT", "PUT", null)] + [InlineData("HEAD", "HEAD", null)] + [InlineData("OPTIONS", "OPTIONS", null)] + [InlineData("PATCH", "PATCH", null)] + [InlineData("POST", "POST", null)] + [InlineData("TRACE", "TRACE", null)] + [InlineData("Connect", "CONNECT", null)] + [InlineData("Delete", "DELETE", "Delete")] + [InlineData("Get", "GET", null)] // HTTP Client converts Get to its canonical form (GET). Expected method is null. + [InlineData("Put", "PUT", null)] // HTTP Client converts Put to its canonical form (PUT). Expected method is null. + [InlineData("Head", "HEAD", null)] // HTTP Client converts Head to its canonical form (HEAD). Expected method is null. + [InlineData("Options", "OPTIONS", "Options")] + [InlineData("Patch", "PATCH", "Patch")] + [InlineData("Post", "POST", null)] // HTTP Client converts Post to its canonical form (POST). Expected method is null. + [InlineData("Trace", "TRACE", "Trace")] + [InlineData("CUSTOM", "_OTHER", "CUSTOM")] + public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMethod, string expectedMethod, string expectedOriginalMethod) { var exportedItems = new List(); using var request = new HttpRequestMessage @@ -396,20 +404,17 @@ public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMetho var activity = exportedItems[0]; - Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethod); - if (originalMethod.Equals(expectedMethod, StringComparison.OrdinalIgnoreCase)) { Assert.Equal(expectedMethod, activity.DisplayName); - Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal); } else { Assert.Equal("HTTP", activity.DisplayName); - Assert.Equal(originalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string); } - Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); + Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); + Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal)); } [Theory] @@ -423,6 +428,7 @@ public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMetho [InlineData("Get", "GET")] [InlineData("POST", "POST")] [InlineData("TRACE", "TRACE")] + [InlineData("Trace", "TRACE")] [InlineData("CUSTOM", "_OTHER")] public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string originalMethod, string expectedMethod) {