From 7d552b34080a7baa8679e5dd30990fc39143064b Mon Sep 17 00:00:00 2001 From: unknown <3676547+alanwest@users.noreply.github.com> Date: Tue, 24 Oct 2023 17:48:40 -0700 Subject: [PATCH] Fix tests --- .../RouteTests/README.md | 80 ++++--------------- .../RouteTests/RouteTestData.cs | 7 ++ .../RouteTests/RoutingTestFixture.cs | 6 +- .../RouteTests/RoutingTests.cs | 34 ++++---- .../RouteTests/TestApplicationFactory.cs | 12 ++- .../RouteTests/testcases.json | 6 +- 6 files changed, 59 insertions(+), 86 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.md b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.md index 334b030faec..60aedb7ae05 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.md +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.md @@ -18,8 +18,6 @@ | :broken_heart: | [16](#16) | Index | /Index | RazorPages | GET /Index | Index | | :broken_heart: | [17](#17) | PageThatThrowsException | /PageThatThrowsException | RazorPages | GET /PageThatThrowsException | PageThatThrowsException | | :broken_heart: | [18](#18) | /js/site.js | | RazorPages | GET /js/site.js | /js/site.js | -| :broken_heart: | [19](#19) | /MinimalApi | TBD | MinimalApi | GET /MinimalApi | /MinimalApi | -| :broken_heart: | [20](#20) | /MinimalApi/123 | TBD | MinimalApi | GET /MinimalApi/123 | /MinimalApi/123 | #### 1 @@ -32,7 +30,7 @@ "HttpRouteByActionDescriptor": "ConventionalRoute/Default/{id?}", "DebugInfo": { "RawText": "{controller=ConventionalRoute}/{action=Default}/{id?}", - "RouteDiagnosticMetadata": "{controller=ConventionalRoute}/{action=Default}/{id?}", + "RouteDiagnosticMetadata": null, "RouteData": { "controller": "ConventionalRoute", "action": "Default" @@ -58,7 +56,7 @@ "HttpRouteByActionDescriptor": "ConventionalRoute/ActionWithStringParameter/{id?}", "DebugInfo": { "RawText": "{controller=ConventionalRoute}/{action=Default}/{id?}", - "RouteDiagnosticMetadata": "{controller=ConventionalRoute}/{action=Default}/{id?}", + "RouteDiagnosticMetadata": null, "RouteData": { "controller": "ConventionalRoute", "action": "ActionWithStringParameter", @@ -88,7 +86,7 @@ "HttpRouteByActionDescriptor": "ConventionalRoute/ActionWithStringParameter/{id?}", "DebugInfo": { "RawText": "{controller=ConventionalRoute}/{action=Default}/{id?}", - "RouteDiagnosticMetadata": "{controller=ConventionalRoute}/{action=Default}/{id?}", + "RouteDiagnosticMetadata": null, "RouteData": { "controller": "ConventionalRoute", "action": "ActionWithStringParameter" @@ -140,7 +138,7 @@ "HttpRouteByActionDescriptor": "SomePath/{id}/{num:int}", "DebugInfo": { "RawText": "SomePath/{id}/{num:int}", - "RouteDiagnosticMetadata": "SomePath/{id}/{num:int}", + "RouteDiagnosticMetadata": null, "RouteData": { "controller": "ConventionalRoute", "action": "ActionWithStringParameter", @@ -194,7 +192,7 @@ "HttpRouteByActionDescriptor": "{area:exists}/ControllerForMyArea/Default/{id?}", "DebugInfo": { "RawText": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}", - "RouteDiagnosticMetadata": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}", + "RouteDiagnosticMetadata": null, "RouteData": { "controller": "ControllerForMyArea", "action": "Default", @@ -221,7 +219,7 @@ "HttpRouteByActionDescriptor": "{area:exists}/ControllerForMyArea/NonDefault/{id?}", "DebugInfo": { "RawText": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}", - "RouteDiagnosticMetadata": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}", + "RouteDiagnosticMetadata": null, "RouteData": { "controller": "ControllerForMyArea", "area": "MyArea", @@ -248,7 +246,7 @@ "HttpRouteByActionDescriptor": "SomePrefix/AnotherArea/Index/{id?}", "DebugInfo": { "RawText": "SomePrefix/{controller=AnotherArea}/{action=Index}/{id?}", - "RouteDiagnosticMetadata": "SomePrefix/{controller=AnotherArea}/{action=Index}/{id?}", + "RouteDiagnosticMetadata": null, "RouteData": { "area": "AnotherArea", "controller": "AnotherArea", @@ -275,7 +273,7 @@ "HttpRouteByActionDescriptor": "AttributeRoute", "DebugInfo": { "RawText": "AttributeRoute", - "RouteDiagnosticMetadata": "AttributeRoute", + "RouteDiagnosticMetadata": null, "RouteData": { "action": "Get", "controller": "AttributeRoute" @@ -301,7 +299,7 @@ "HttpRouteByActionDescriptor": "AttributeRoute/Get", "DebugInfo": { "RawText": "AttributeRoute/Get", - "RouteDiagnosticMetadata": "AttributeRoute/Get", + "RouteDiagnosticMetadata": null, "RouteData": { "action": "Get", "controller": "AttributeRoute" @@ -327,7 +325,7 @@ "HttpRouteByActionDescriptor": "AttributeRoute/Get/{id}", "DebugInfo": { "RawText": "AttributeRoute/Get/{id}", - "RouteDiagnosticMetadata": "AttributeRoute/Get/{id}", + "RouteDiagnosticMetadata": null, "RouteData": { "action": "Get", "controller": "AttributeRoute", @@ -356,7 +354,7 @@ "HttpRouteByActionDescriptor": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate", "DebugInfo": { "RawText": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate", - "RouteDiagnosticMetadata": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate", + "RouteDiagnosticMetadata": null, "RouteData": { "action": "GetWithActionNameInDifferentSpotInTemplate", "controller": "AttributeRoute", @@ -385,7 +383,7 @@ "HttpRouteByActionDescriptor": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate", "DebugInfo": { "RawText": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate", - "RouteDiagnosticMetadata": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate", + "RouteDiagnosticMetadata": null, "RouteData": { "action": "GetWithActionNameInDifferentSpotInTemplate", "controller": "AttributeRoute", @@ -414,7 +412,7 @@ "HttpRouteByActionDescriptor": "/Index", "DebugInfo": { "RawText": "", - "RouteDiagnosticMetadata": "", + "RouteDiagnosticMetadata": null, "RouteData": { "page": "/Index" }, @@ -439,7 +437,7 @@ "HttpRouteByActionDescriptor": "/Index", "DebugInfo": { "RawText": "Index", - "RouteDiagnosticMetadata": "Index", + "RouteDiagnosticMetadata": null, "RouteData": { "page": "/Index" }, @@ -464,7 +462,7 @@ "HttpRouteByActionDescriptor": "/PageThatThrowsException", "DebugInfo": { "RawText": "PageThatThrowsException", - "RouteDiagnosticMetadata": "PageThatThrowsException", + "RouteDiagnosticMetadata": null, "RouteData": { "page": "/PageThatThrowsException" }, @@ -500,51 +498,3 @@ } } ``` - -#### 19 - -```json -{ - "HttpMethod": "GET", - "Path": "/MinimalApi", - "HttpRouteByRawText": "/MinimalApi/", - "HttpRouteByControllerActionAndParameters": "", - "HttpRouteByActionDescriptor": "", - "DebugInfo": { - "RawText": "/MinimalApi/", - "RouteDiagnosticMetadata": "/MinimalApi/", - "RouteData": {}, - "AttributeRouteInfo": null, - "ActionParameters": null, - "PageActionDescriptorRelativePath": null, - "PageActionDescriptorViewEnginePath": null, - "ControllerActionDescriptorControllerName": null, - "ControllerActionDescriptorActionName": null - } -} -``` - -#### 20 - -```json -{ - "HttpMethod": "GET", - "Path": "/MinimalApi/123", - "HttpRouteByRawText": "/MinimalApi/{id}", - "HttpRouteByControllerActionAndParameters": "", - "HttpRouteByActionDescriptor": "", - "DebugInfo": { - "RawText": "/MinimalApi/{id}", - "RouteDiagnosticMetadata": "/MinimalApi/{id}", - "RouteData": { - "id": "123" - }, - "AttributeRouteInfo": null, - "ActionParameters": null, - "PageActionDescriptorRelativePath": null, - "PageActionDescriptorViewEnginePath": null, - "ControllerActionDescriptorControllerName": null, - "ControllerActionDescriptorActionName": null - } -} -``` diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RouteTestData.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RouteTestData.cs index fc62fb12299..304da57fafd 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RouteTestData.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RouteTestData.cs @@ -55,6 +55,11 @@ private static IEnumerable GetArgumentsFromTestCaseObject(IEnumerable< { foreach (var testCase in input) { + if (testCase.MinimumDotnetVersion.HasValue && Environment.Version.Major < testCase.MinimumDotnetVersion.Value) + { + continue; + } + result.Add(new object[] { testCase, @@ -67,6 +72,8 @@ private static IEnumerable GetArgumentsFromTestCaseObject(IEnumerable< public class RouteTestCase { + public int? MinimumDotnetVersion { get; set; } + public bool Debug { get; set; } public TestApplicationScenario TestApplicationScenario { get; set; } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestFixture.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestFixture.cs index fd2b794bf15..3e4e28fb21a 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestFixture.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestFixture.cs @@ -33,7 +33,11 @@ public RoutingTestFixture() { foreach (var scenario in Enum.GetValues()) { - this.apps.Add(scenario, TestApplicationFactory.CreateApplication(scenario)); + var app = TestApplicationFactory.CreateApplication(scenario); + if (app != null) + { + this.apps.Add(scenario, app); + } } foreach (var app in this.apps) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs index b38b7cc0079..053c61ff1b6 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs @@ -26,9 +26,10 @@ namespace RouteTests; public class RoutingTests : IClassFixture, IDisposable { - // TODO: This test currently uses the old conventions. Update it to use the new conventions. - private const string HttpStatusCode = "http.status_code"; - private const string HttpMethod = "http.method"; + private const string OldHttpStatusCode = "http.status_code"; + private const string OldHttpMethod = "http.method"; + private const string HttpStatusCode = "http.response.status_code"; + private const string HttpMethod = "http.request.method"; private const string HttpRoute = "http.route"; private readonly RoutingTestFixture fixture; @@ -56,7 +57,7 @@ public RoutingTests(RoutingTestFixture fixture) [Theory] [MemberData(nameof(TestData))] - public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsserts = true) + public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsserts = true, bool useLegacyConventions = true) { await this.fixture.MakeRequest(testCase.TestApplicationScenario, testCase.Path); @@ -73,10 +74,10 @@ public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsse this.meterProvider.ForceFlush(); Assert.Single(this.exportedActivities); - Assert.Single(this.exportedMetrics); + var durationMetric = this.exportedMetrics.Single(x => x.Name == "http.server.request.duration" || x.Name == "http.server.duration"); var metricPoints = new List(); - foreach (var mp in this.exportedMetrics[0].GetMetricPoints()) + foreach (var mp in durationMetric.GetMetricPoints()) { metricPoints.Add(mp); } @@ -86,8 +87,8 @@ public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsse var activity = this.exportedActivities[0]; var metricPoint = metricPoints.First(); - this.GetTagsFromActivity(activity, out var activityHttpStatusCode, out var activityHttpMethod, out var activityHttpRoute); - this.GetTagsFromMetricPoint(metricPoint, out var metricHttpStatusCode, out var metricHttpMethod, out var metricHttpRoute); + this.GetTagsFromActivity(useLegacyConventions, activity, out var activityHttpStatusCode, out var activityHttpMethod, out var activityHttpRoute); + this.GetTagsFromMetricPoint(useLegacyConventions && Environment.Version.Major < 8, metricPoint, out var metricHttpStatusCode, out var metricHttpMethod, out var metricHttpRoute); Assert.Equal(testCase.ExpectedStatusCode, activityHttpStatusCode); Assert.Equal(testCase.ExpectedStatusCode, metricHttpStatusCode); @@ -124,26 +125,31 @@ public void Dispose() this.meterProvider.Dispose(); } - private void GetTagsFromActivity(Activity activity, out int httpStatusCode, out string httpMethod, out string? httpRoute) + private void GetTagsFromActivity(bool useLegacyConventions, Activity activity, out int httpStatusCode, out string httpMethod, out string? httpRoute) { - httpStatusCode = Convert.ToInt32(activity.GetTagItem(HttpStatusCode)); - httpMethod = (activity.GetTagItem(HttpMethod) as string)!; + var expectedStatusCodeKey = useLegacyConventions ? OldHttpStatusCode : HttpStatusCode; + var expectedHttpMethodKey = useLegacyConventions ? OldHttpMethod : HttpMethod; + httpStatusCode = Convert.ToInt32(activity.GetTagItem(expectedStatusCodeKey)); + httpMethod = (activity.GetTagItem(expectedHttpMethodKey) as string)!; httpRoute = activity.GetTagItem(HttpRoute) as string ?? string.Empty; } - private void GetTagsFromMetricPoint(MetricPoint metricPoint, out int httpStatusCode, out string httpMethod, out string? httpRoute) + private void GetTagsFromMetricPoint(bool useLegacyConventions, MetricPoint metricPoint, out int httpStatusCode, out string httpMethod, out string? httpRoute) { + var expectedStatusCodeKey = useLegacyConventions ? OldHttpStatusCode : HttpStatusCode; + var expectedHttpMethodKey = useLegacyConventions ? OldHttpMethod : HttpMethod; + httpStatusCode = 0; httpMethod = string.Empty; httpRoute = string.Empty; foreach (var tag in metricPoint.Tags) { - if (tag.Key.Equals(HttpStatusCode)) + if (tag.Key.Equals(expectedStatusCodeKey)) { httpStatusCode = Convert.ToInt32(tag.Value); } - else if (tag.Key.Equals(HttpMethod)) + else if (tag.Key.Equals(expectedHttpMethodKey)) { httpMethod = (tag.Value as string)!; } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplicationFactory.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplicationFactory.cs index 7760906a615..5cf1d87dc5d 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplicationFactory.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplicationFactory.cs @@ -14,7 +14,7 @@ // limitations under the License. // -#nullable disable +#nullable enable using System.Diagnostics; using Microsoft.AspNetCore.Builder; @@ -56,7 +56,7 @@ internal class TestApplicationFactory ? AspNetCoreTestsPath : Path.Combine(AspNetCoreTestsPath, "RouteTests"); - public static WebApplication CreateApplication(TestApplicationScenario config) + public static WebApplication? CreateApplication(TestApplicationScenario config) { Debug.Assert(Directory.Exists(ContentRootPath), $"Cannot find ContentRootPath: {ContentRootPath}"); switch (config) @@ -66,7 +66,11 @@ public static WebApplication CreateApplication(TestApplicationScenario config) case TestApplicationScenario.AttributeRouting: return CreateAttributeRoutingApplication(); case TestApplicationScenario.MinimalApi: +#if NET7_0_OR_GREATER return CreateMinimalApiApplication(); +#else + return null; +#endif case TestApplicationScenario.RazorPages: return CreateRazorPagesApplication(); default: @@ -125,6 +129,7 @@ private static WebApplication CreateAttributeRoutingApplication() return app; } +#if NET7_0_OR_GREATER private static WebApplication CreateMinimalApiApplication() { var builder = WebApplication.CreateBuilder(); // WebApplication.CreateSlimBuilder(); @@ -134,14 +139,13 @@ private static WebApplication CreateMinimalApiApplication() app.Urls.Add("http://[::1]:0"); app.UseMiddleware(); -#if NET7_0_OR_GREATER var api = app.MapGroup("/MinimalApi"); api.MapGet("/", () => Results.Ok()); api.MapGet("/{id}", (int id) => Results.Ok()); -#endif return app; } +#endif private static WebApplication CreateRazorPagesApplication() { diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/testcases.json b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/testcases.json index 9057f8b2e2b..c807b4e9e90 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/testcases.json +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/testcases.json @@ -130,13 +130,15 @@ "httpMethod": "GET", "path": "/MinimalApi", "expectedStatusCode": 200, - "expectedHttpRoute": "TBD" + "expectedHttpRoute": "TBD", + "minimumDotnetVersion": 7 }, { "testApplicationScenario": "MinimalApi", "httpMethod": "GET", "path": "/MinimalApi/123", "expectedStatusCode": 200, - "expectedHttpRoute": "TBD" + "expectedHttpRoute": "TBD", + "minimumDotnetVersion": 7 } ]