Merge branch 'alanwest/aspnetcore-routing' of github.com:alanwest/ope…

…ntelemetry-dotnet into alanwest/aspnetcore-routing

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 | /MinimalApi/ | MinimalApi | GET /MinimalApi | /MinimalApi |
-| :broken_heart: | [20](#20) | /MinimalApi/123 | /MinimalApi/{id} | MinimalApi | GET /MinimalApi/123 | /MinimalApi/123 |
 
 #### 1
 
@@ -500,51 +498,3 @@
   }
 }
 ```
-
-#### 19
-
-```json
-{
-  "HttpMethod": "GET",
-  "Path": "/MinimalApi",
-  "HttpRouteByRawText": "/MinimalApi/",
-  "HttpRouteByControllerActionAndParameters": "",
-  "HttpRouteByActionDescriptor": "",
-  "DebugInfo": {
-    "RawText": "/MinimalApi/",
-    "RouteDiagnosticMetadata": null,
-    "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": null,
-    "RouteData": {
-      "id": "123"
-    },
-    "AttributeRouteInfo": null,
-    "ActionParameters": null,
-    "PageActionDescriptorRelativePath": null,
-    "PageActionDescriptorViewEnginePath": null,
-    "ControllerActionDescriptorControllerName": null,
-    "ControllerActionDescriptorActionName": null
-  }
-}
-```

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RouteTestData.cs

@@ -55,6 +55,11 @@ private static IEnumerable<object[]> 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<object[]> GetArgumentsFromTestCaseObject(IEnumerable<
 
     public class RouteTestCase
     {
+        public int? MinimumDotnetVersion { get; set; }
+
         public bool Debug { get; set; }
 
         public TestApplicationScenario TestApplicationScenario { get; set; }

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestFixture.cs

@@ -33,7 +33,11 @@ public RoutingTestFixture()
     {
         foreach (var scenario in Enum.GetValues<TestApplicationScenario>())
         {
-            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)

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs

@@ -26,9 +26,10 @@ namespace RouteTests;
 
 public class RoutingTests : IClassFixture<RoutingTestFixture>, 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<MetricPoint>();
-        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)!;
             }

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplicationFactory.cs

@@ -14,7 +14,7 @@
 // limitations under the License.
 // </copyright>
 
-#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<RouteInfoMiddleware>();
 
-#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()
     {

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/testcases.json

@@ -130,13 +130,15 @@
         "httpMethod": "GET",
         "path": "/MinimalApi",
         "expectedStatusCode": 200,
-        "expectedHttpRoute": "/MinimalApi/"
+        "expectedHttpRoute": "/MinimalApi/",
+        "minimumDotnetVersion": 7
     },
     {
         "testApplicationScenario": "MinimalApi",
         "httpMethod": "GET",
         "path": "/MinimalApi/123",
         "expectedStatusCode": 200,
-        "expectedHttpRoute": "/MinimalApi/{id}"
+        "expectedHttpRoute": "/MinimalApi/{id}",
+        "minimumDotnetVersion": 7
     }
 ]