From ac4fe159fe56e189ec81eab1383dac2fe058ec77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Mon, 25 Mar 2024 18:32:00 +0100 Subject: [PATCH] [Instrumentation.AspNet] Spans - semantic convention v1.24.0 (#1607) --- .../CHANGELOG.md | 17 +++- .../Implementation/HttpInListener.cs | 62 +++++++----- .../Implementation/HttpInMetricsListener.cs | 18 +--- ...stMethodHelper.cs => RequestDataHelper.cs} | 34 ++++++- src/Shared/SemanticConventions.cs | 5 + .../HttpInListenerTests.cs | 94 +++++++++---------- .../HttpInMetricsListenerTests.cs | 2 +- ...lperTests.cs => RequestDataHelperTests.cs} | 17 +++- .../RouteTestHelper.cs | 8 +- .../TestHttpWorkerRequest.cs | 18 +++- 10 files changed, 170 insertions(+), 105 deletions(-) rename src/OpenTelemetry.Instrumentation.AspNet/Implementation/{RequestMethodHelper.cs => RequestDataHelper.cs} (62%) rename test/OpenTelemetry.Instrumentation.AspNet.Tests/{RequestMethodHelperTests.cs => RequestDataHelperTests.cs} (77%) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index f92eda8790..a5c732e472 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -8,6 +8,21 @@ * **Breaking Change**: `server.address` and `server.port` no longer added for `http.server.request.duration` metric. ([#1606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1606)) +* **Breaking change** Spans names and attributes + will be set as per [HTTP semantic convention v1.24.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md): + * span names follows: `{HTTP method} [route name if available]` pattern + * `error.type` added when exception occurred while processing request, + * `http.request.method` replaces `http.method`, + * `http.request.method_original` added when `http.request.method` is not in + canonical form, + * `http.response.status_code` replaces `http.status_code`, + * `network.protocol.version` added with HTTP version value, + * `server.address` and `server.port` replace `http.host`, + * `url.path` replaces `http.target`, + * `url.query` added when query url part is not empty, + * `url.scheme` added with `http` or `https` value, + * `user_agent.original` replaces `http.user_agent`. + ([#1607](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1607)) ## 1.7.0-beta.2 @@ -96,7 +111,7 @@ Released 2022-Nov-28 Released 2022-Sep-28 -* Migrate to native Activity `Status` and `StatusDesciption`. +* Migrate to native Activity `Status` and `StatusDescription`. ([#651](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/651)) ## 1.0.0-rc9.5 (source code moved to contrib repo) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index ca9dd387a9..0337c56eaa 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -14,6 +14,7 @@ internal sealed class HttpInListener : IDisposable { private readonly HttpRequestRouteHelper routeHelper = new(); private readonly AspNetTraceInstrumentationOptions options; + private readonly RequestDataHelper requestDataHelper = new(); public HttpInListener(AspNetTraceInstrumentationOptions options) { @@ -35,16 +36,6 @@ public void Dispose() TelemetryHttpModule.Options.OnExceptionCallback -= this.OnException; } - /// - /// Gets the OpenTelemetry standard uri tag value for a span based on its request . - /// - /// . - /// Span uri value. - private static string GetUriTagValueFromRequestUri(Uri uri) - { - return string.IsNullOrEmpty(uri.UserInfo) ? uri.ToString() : string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.PathAndQuery, uri.Fragment); - } - private void OnStartActivity(Activity activity, HttpContext context) { if (activity.IsAllDataRequested) @@ -76,23 +67,45 @@ private void OnStartActivity(Activity activity, HttpContext context) var request = context.Request; var requestValues = request.Unvalidated; - // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md - var path = requestValues.Path; - activity.DisplayName = path; + // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md + var originalHttpMethod = request.HttpMethod; + var normalizedHttpMethod = this.requestDataHelper.GetNormalizedHttpMethod(originalHttpMethod); + activity.DisplayName = normalizedHttpMethod == "_OTHER" ? "HTTP" : normalizedHttpMethod; + + var url = request.Url; + activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host); + activity.SetTag(SemanticConventions.AttributeServerPort, url.Port); + activity.SetTag(SemanticConventions.AttributeUrlScheme, url.Scheme); - if (request.Url.Port == 80 || request.Url.Port == 443) + this.requestDataHelper.SetHttpMethodTag(activity, originalHttpMethod); + + var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request); + if (!string.IsNullOrEmpty(protocolVersion)) { - activity.SetTag(SemanticConventions.AttributeHttpHost, request.Url.Host); + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion); } - else + + activity.SetTag(SemanticConventions.AttributeUrlPath, requestValues.Path); + + // TODO url.query should be sanitized + var query = url.Query; + if (!string.IsNullOrEmpty(query)) { - activity.SetTag(SemanticConventions.AttributeHttpHost, request.Url.Host + ":" + request.Url.Port); + if (query.StartsWith("?", StringComparison.InvariantCulture)) + { + activity.SetTag(SemanticConventions.AttributeUrlQuery, query.Substring(1)); + } + else + { + activity.SetTag(SemanticConventions.AttributeUrlQuery, query); + } } - activity.SetTag(SemanticConventions.AttributeHttpMethod, request.HttpMethod); - activity.SetTag(SemanticConventions.AttributeHttpTarget, path); - activity.SetTag(SemanticConventions.AttributeHttpUserAgent, request.UserAgent); - activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUriTagValueFromRequestUri(request.Url)); + var userAgent = request.UserAgent; + if (!string.IsNullOrEmpty(userAgent)) + { + activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent); + } try { @@ -111,7 +124,7 @@ private void OnStopActivity(Activity activity, HttpContext context) { var response = context.Response; - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode); + activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, response.StatusCode); if (activity.Status == ActivityStatusCode.Unset) { @@ -122,8 +135,8 @@ private void OnStopActivity(Activity activity, HttpContext context) if (!string.IsNullOrEmpty(template)) { - // Override the name that was previously set to the path part of URL. - activity.DisplayName = template!; + // Override the name that was previously set to the normalized HTTP method/HTTP + activity.DisplayName = $"{activity.DisplayName} {template!}"; activity.SetTag(SemanticConventions.AttributeHttpRoute, template); } @@ -148,6 +161,7 @@ private void OnException(Activity activity, HttpContext context, Exception excep } activity.SetStatus(ActivityStatusCode.Error, exception.Message); + activity.SetTag(SemanticConventions.AttributeErrorType, exception.GetType().FullName); try { diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs index 3607f4a465..d1687d9d66 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInMetricsListener.cs @@ -12,7 +12,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation; internal sealed class HttpInMetricsListener : IDisposable { private readonly HttpRequestRouteHelper routeHelper = new(); - private readonly RequestMethodHelper requestMethodHelper = new(); + private readonly RequestDataHelper requestDataHelper = new(); private readonly Histogram httpServerDuration; private readonly AspNetMetricsInstrumentationOptions options; @@ -31,18 +31,6 @@ public void Dispose() TelemetryHttpModule.Options.OnRequestStoppedCallback -= this.OnStopActivity; } - private static string GetHttpProtocolVersion(HttpRequest request) - { - var protocol = request.ServerVariables["SERVER_PROTOCOL"]; - return protocol switch - { - "HTTP/1.1" => "1.1", - "HTTP/2" => "2", - "HTTP/3" => "3", - _ => protocol, - }; - } - private void OnStopActivity(Activity activity, HttpContext context) { var request = context.Request; @@ -59,10 +47,10 @@ private void OnStopActivity(Activity activity, HttpContext context) tags.Add(SemanticConventions.AttributeServerPort, url.Port); } - var normalizedMethod = this.requestMethodHelper.GetNormalizedHttpMethod(request.HttpMethod); + var normalizedMethod = this.requestDataHelper.GetNormalizedHttpMethod(request.HttpMethod); tags.Add(SemanticConventions.AttributeHttpRequestMethod, normalizedMethod); - var protocolVersion = GetHttpProtocolVersion(request); + var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request); if (!string.IsNullOrEmpty(protocolVersion)) { tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion); diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestMethodHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestDataHelper.cs similarity index 62% rename from src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestMethodHelper.cs rename to src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestDataHelper.cs index 3d7c5d4183..ef5a346f98 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestMethodHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/RequestDataHelper.cs @@ -3,11 +3,14 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; +using System.Web; +using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.AspNet.Implementation; -internal sealed class RequestMethodHelper +internal sealed class RequestDataHelper { private const string KnownHttpMethodsEnvironmentVariable = "OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS"; @@ -20,7 +23,7 @@ internal sealed class RequestMethodHelper // List of known HTTP methods as per spec. private readonly Dictionary knownHttpMethods; - public RequestMethodHelper() + public RequestDataHelper() { var suppliedKnownMethods = Environment.GetEnvironmentVariable(KnownHttpMethodsEnvironmentVariable) ?.Split(SplitChars, StringSplitOptions.RemoveEmptyEntries); @@ -40,10 +43,37 @@ public RequestMethodHelper() }; } + public static string GetHttpProtocolVersion(HttpRequest request) + { + return GetHttpProtocolVersion(request.ServerVariables["SERVER_PROTOCOL"]); + } + + public void SetHttpMethodTag(Activity activity, string originalHttpMethod) + { + var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod); + + if (originalHttpMethod != normalizedHttpMethod) + { + activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod); + } + } + public string GetNormalizedHttpMethod(string method) { return this.knownHttpMethods.TryGetValue(method, out var normalizedMethod) ? normalizedMethod : OtherHttpMethod; } + + internal static string GetHttpProtocolVersion(string protocol) + { + return protocol switch + { + "HTTP/1.1" => "1.1", + "HTTP/2" => "2", + "HTTP/3" => "3", + _ => protocol, + }; + } } diff --git a/src/Shared/SemanticConventions.cs b/src/Shared/SemanticConventions.cs index 202d4b4eef..0d9fe00f87 100644 --- a/src/Shared/SemanticConventions.cs +++ b/src/Shared/SemanticConventions.cs @@ -95,17 +95,22 @@ internal static class SemanticConventions public const string AttributeExceptionType = "exception.type"; public const string AttributeExceptionMessage = "exception.message"; public const string AttributeExceptionStacktrace = "exception.stacktrace"; + public const string AttributeErrorType = "error.type"; // v1.21.0 // https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md#http-server public const string AttributeHttpRequestMethod = "http.request.method"; // replaces: "http.method" (AttributeHttpMethod) + public const string AttributeHttpRequestMethodOriginal = "http.request.method_original"; public const string AttributeHttpResponseStatusCode = "http.response.status_code"; // replaces: "http.status_code" (AttributeHttpStatusCode) public const string AttributeUrlScheme = "url.scheme"; // replaces: "http.scheme" (AttributeHttpScheme) + public const string AttributeUrlPath = "url.path"; // replaces: "http.target" (AttributeHttpTarget) + public const string AttributeUrlQuery = "url.query"; // replaces: "http.target" (AttributeHttpTarget) // v1.23.0 // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#http-server public const string AttributeNetworkProtocolVersion = "network.protocol.version"; // replaces: "http.flavor" (AttributeHttpFlavor) public const string AttributeServerAddress = "server.address"; // replaces: "net.host.name" (AttributeNetHostName) public const string AttributeServerPort = "server.port"; // replaces: "net.host.port" (AttributeNetHostPort) + public const string AttributeUserAgentOriginal = "user_agent.original"; // replaces: http.user_agent (AttributeHttpUserAgent) #pragma warning restore CS1591 // Missing XML comment for publicly visible type or member } diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 9959d372ab..4b832aa6d6 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -20,31 +20,40 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests; public class HttpInListenerTests { [Theory] - [InlineData("http://localhost/", "http://localhost/", 0, null)] - [InlineData("http://localhost/", "http://localhost/", 0, null, true)] - [InlineData("https://localhost/", "https://localhost/", 0, null)] - [InlineData("https://localhost/", "https://user:pass@localhost/", 0, null)] // Test URL sanitization - [InlineData("http://localhost:443/", "http://localhost:443/", 0, null)] // Test http over 443 - [InlineData("https://localhost:80/", "https://localhost:80/", 0, null)] // Test https over 80 - [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", 0, null)] // Test complex URL - [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", 0, null)] // Test complex URL sanitization - [InlineData("http://localhost:80/Index", "http://localhost:80/Index", 1, "{controller}/{action}/{id}")] - [InlineData("https://localhost:443/about_attr_route/10", "https://localhost:443/about_attr_route/10", 2, "about_attr_route/{customerId}")] - [InlineData("http://localhost:1880/api/weatherforecast", "http://localhost:1880/api/weatherforecast", 3, "api/{controller}/{id}")] - [InlineData("https://localhost:1843/subroute/10", "https://localhost:1843/subroute/10", 4, "subroute/{customerId}")] - [InlineData("http://localhost/api/value", "http://localhost/api/value", 0, null, false, "/api/value")] // Request will be filtered - [InlineData("http://localhost/api/value", "http://localhost/api/value", 0, null, false, "{ThrowException}")] // Filter user code will throw an exception - [InlineData("http://localhost/", "http://localhost/", 0, null, false, null, true)] // Test RecordException option + [InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] + [InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", "localhost", 80, "POST", "POST", null, 0, null, "POST", true)] + [InlineData("https://localhost/", "https", "/", null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null, "HTTP")] + [InlineData("https://user:pass@localhost/", "https", "/", null, "localhost", 443, "GeT", "GET", "GeT", 0, null, "GET")] // Test URL sanitization + [InlineData("http://localhost:443/", "http", "/", null, "localhost", 443, "GET", "GET", null, 0, null, "GET")] // Test http over 443 + [InlineData("https://localhost:80/", "https", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test https over 80 + [InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL + [InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL sanitization + [InlineData("http://localhost:80/Index", "http", "/Index", null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}", "GET {controller}/{action}/{id}")] + [InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, "localhost", 443, "HEAD", "HEAD", null, 2, "about_attr_route/{customerId}", "HEAD about_attr_route/{customerId}")] + [InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}", "GET api/{controller}/{id}")] + [InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}", "GET subroute/{customerId}")] + [InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "/api/value")] // Request will be filtered + [InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "{ThrowException}")] // Filter user code will throw an exception + [InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, null, true, "System.InvalidOperationException")] // Test RecordException option public void AspNetRequestsAreCollectedSuccessfully( - string expectedUrl, string url, + string expectedUrlScheme, + string expectedUrlPath, + string expectedUrlQuery, + string expectedHost, + int expectedPort, + string requestMethod, + string expectedRequestMethod, + string? expectedOriginalRequestMethod, int routeType, - string routeTemplate, + string? routeTemplate, + string expectedName, bool setStatusToErrorInEnrich = false, string? filter = null, - bool recordException = false) + bool recordException = false, + string? expectedErrorType = null) { - HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate); + HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate, requestMethod); typeof(HttpRequest).GetField("_wr", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(HttpContext.Current.Request, new TestHttpWorkerRequest()); @@ -109,44 +118,24 @@ public void AspNetRequestsAreCollectedSuccessfully( Assert.Equal(TelemetryHttpModule.AspNetActivityName, span.OperationName); Assert.NotEqual(TimeSpan.Zero, span.Duration); - Assert.Equal(routeTemplate ?? HttpContext.Current.Request.Path, span.DisplayName); + Assert.Equal(expectedName, span.DisplayName); Assert.Equal(ActivityKind.Server, span.Kind); Assert.True(span.Duration != TimeSpan.Zero); - Assert.Equal(200, span.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); + Assert.Equal(200, span.GetTagValue("http.response.status_code")); - var expectedUri = new Uri(expectedUrl); - var actualUrl = span.GetTagValue(SemanticConventions.AttributeHttpUrl); + Assert.Equal(expectedHost, span.GetTagValue("server.address")); + Assert.Equal(expectedPort, span.GetTagValue("server.port")); - Assert.Equal(expectedUri.ToString(), actualUrl); + Assert.Equal(expectedRequestMethod, span.GetTagValue("http.request.method")); + Assert.Equal(expectedOriginalRequestMethod, span.GetTagValue("http.request.method_original")); + Assert.Equal("FakeHTTP/123", span.GetTagValue("network.protocol.version")); - // Url strips 80 or 443 if the scheme matches. - if ((expectedUri.Port == 80 && expectedUri.Scheme == "http") || (expectedUri.Port == 443 && expectedUri.Scheme == "https")) - { - Assert.DoesNotContain($":{expectedUri.Port}", actualUrl as string); - } - else - { - Assert.Contains($":{expectedUri.Port}", actualUrl as string); - } - - // Host includes port if it isn't 80 or 443. - if (expectedUri.Port is 80 or 443) - { - Assert.Equal( - expectedUri.Host, - span.GetTagValue(SemanticConventions.AttributeHttpHost) as string); - } - else - { - Assert.Equal( - $"{expectedUri.Host}:{expectedUri.Port}", - span.GetTagValue(SemanticConventions.AttributeHttpHost) as string); - } - - Assert.Equal(HttpContext.Current.Request.HttpMethod, span.GetTagValue(SemanticConventions.AttributeHttpMethod) as string); - Assert.Equal(HttpContext.Current.Request.Path, span.GetTagValue(SemanticConventions.AttributeHttpTarget) as string); - Assert.Equal(HttpContext.Current.Request.UserAgent, span.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string); + Assert.Equal(expectedUrlPath, span.GetTagValue("url.path")); + Assert.Equal(expectedUrlQuery, span.GetTagValue("url.query")); + Assert.Equal(expectedUrlScheme, span.GetTagValue("url.scheme")); + Assert.Equal("Custom User Agent v1.2.3", span.GetTagValue("user_agent.original")); + Assert.Equal(expectedErrorType, span.GetTagValue("error.type")); if (recordException) { @@ -271,7 +260,8 @@ void EnrichAction(Activity activity, string method, object obj) break; - default: + case "OnException": + Assert.True(obj is Exception); break; } } diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs index b5c6ddf904..8c39fb7db4 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInMetricsListenerTests.cs @@ -44,7 +44,7 @@ public void AspNetMetricTagsAreCollectedSuccessfully( bool enableServerAttributesForRequestDuration = true) { double duration = 0; - HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate); + HttpContext.Current = RouteTestHelper.BuildHttpContext(url, routeType, routeTemplate, "GET"); HttpContext.Current.Response.StatusCode = expectedStatus; // This is to enable activity creation diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestMethodHelperTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs similarity index 77% rename from test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestMethodHelperTests.cs rename to test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs index fd0312f187..e1beb3a377 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestMethodHelperTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RequestDataHelperTests.cs @@ -7,7 +7,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests; -public class RequestMethodHelperTests : IDisposable +public class RequestDataHelperTests : IDisposable { [Theory] [InlineData("GET", "GET")] @@ -23,7 +23,7 @@ public class RequestMethodHelperTests : IDisposable [InlineData("invalid", "_OTHER")] public void MethodMappingWorksForKnownMethods(string method, string expected) { - var requestHelper = new RequestMethodHelper(); + var requestHelper = new RequestDataHelper(); var actual = requestHelper.GetNormalizedHttpMethod(method); Assert.Equal(expected, actual); } @@ -44,11 +44,22 @@ 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 RequestMethodHelper(); + var requestHelper = new RequestDataHelper(); var actual = requestHelper.GetNormalizedHttpMethod(method); Assert.Equal(expected, actual); } + [Theory] + [InlineData("HTTP/1.1", "1.1")] + [InlineData("HTTP/2", "2")] + [InlineData("HTTP/3", "3")] + [InlineData("Unknown", "Unknown")] + public void MappingProtocolToVersion(string protocolVersion, string expected) + { + var actual = RequestDataHelper.GetHttpProtocolVersion(protocolVersion); + Assert.Equal(expected, actual); + } + public void Dispose() { // Clean up after tests that set environment variables. diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs index 318d815572..fcebc17d5b 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs @@ -10,7 +10,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests; internal static class RouteTestHelper { - public static HttpContext BuildHttpContext(string url, int routeType, string routeTemplate) + public static HttpContext BuildHttpContext(string url, int routeType, string? routeTemplate, string requestMethod) { RouteData routeData; switch (routeType) @@ -21,7 +21,7 @@ public static HttpContext BuildHttpContext(string url, int routeType, string rou case 1: // Traditional MVC. case 2: // Attribute routing MVC. case 3: // Traditional WebAPI. - routeData = new RouteData() + routeData = new RouteData { Route = new Route(routeTemplate, null), }; @@ -48,12 +48,14 @@ public static HttpContext BuildHttpContext(string url, int routeType, string rou var request = new HttpRequest(string.Empty, url, string.Empty) { - RequestContext = new RequestContext() + RequestContext = new RequestContext { RouteData = routeData, }, }; + typeof(HttpRequest).GetField("_httpMethod", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).SetValue(request, requestMethod); + return new HttpContext(request, new HttpResponse(new StringWriter())); } } diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/TestHttpWorkerRequest.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/TestHttpWorkerRequest.cs index c9b1bb5058..7230af6a91 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/TestHttpWorkerRequest.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/TestHttpWorkerRequest.cs @@ -8,6 +8,16 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests; internal class TestHttpWorkerRequest : HttpWorkerRequest { + public override string GetKnownRequestHeader(int index) + { + if (index == 39) + { + return "Custom User Agent v1.2.3"; + } + + return base.GetKnownRequestHeader(index); + } + public override void EndOfRequest() { throw new NotImplementedException(); @@ -25,17 +35,17 @@ public override string GetHttpVerbName() public override string GetHttpVersion() { - throw new NotImplementedException(); + return "FakeHTTP/123"; } public override string GetLocalAddress() { - throw new NotImplementedException(); + return "fake-local-address"; // avoid throwing exception } public override int GetLocalPort() { - throw new NotImplementedException(); + return 1234; // avoid throwing exception } public override string GetQueryString() @@ -50,7 +60,7 @@ public override string GetRawUrl() public override string GetRemoteAddress() { - throw new NotImplementedException(); + return "fake-remote-address"; // avoid throwing exception } public override int GetRemotePort()