[release/9.0] Fix duplicate error.type on kestrel.connection.duration (…

…#57581)

* Fix duplicate error.type on kestrel.connection.duration

* Comment

* Clean up

---------

Co-authored-by: James Newton-King <[email protected]>

src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs

@@ -120,6 +120,15 @@ private void ConnectionStopCore(ConnectionMetricsContext metricsContext, Excepti
 
         if (metricsContext.ConnectionDurationEnabled)
         {
+            // Add custom tags for duration.
+            if (customTags != null)
+            {
+                for (var i = 0; i < customTags.Count; i++)
+                {
+                    tags.Add(customTags[i]);
+                }
+            }
+
             // Check if there is an end reason on the context. For example, the connection could have been aborted by shutdown.
             if (metricsContext.ConnectionEndReason is { } reason && TryGetErrorType(reason, out var errorValue))
             {
@@ -130,15 +139,6 @@ private void ConnectionStopCore(ConnectionMetricsContext metricsContext, Excepti
                 tags.TryAddTag(ErrorTypeAttributeName, exception.GetType().FullName);
             }
 
-            // Add custom tags for duration.
-            if (customTags != null)
-            {
-                for (var i = 0; i < customTags.Count; i++)
-                {
-                    tags.Add(customTags[i]);
-                }
-            }
-
             var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp);
             _connectionDuration.Record(duration.TotalSeconds, tags);
         }

src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs

@@ -5,15 +5,15 @@
 using System.Net;
 using System.Net.Http;
 using System.Net.Http.Headers;
+using System.Net.Sockets;
 using System.Security.Authentication;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Internal;
+using Microsoft.AspNetCore.InternalTesting;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
-using Microsoft.AspNetCore.InternalTesting;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Diagnostics.Metrics;
 using Microsoft.Extensions.Diagnostics.Metrics.Testing;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
@@ -23,6 +23,72 @@ namespace Interop.FunctionalTests.Http2;
 [Collection(nameof(NoParallelCollection))]
 public class Http2RequestTests : LoggedTest
 {
+    [Fact]
+    public async Task InvalidHandshake_MetricsHasErrorType()
+    {
+        // Arrange
+        var builder = CreateHostBuilder(
+            c =>
+            {
+                return Task.CompletedTask;
+            },
+            protocol: HttpProtocols.Http2,
+            plaintext: true);
+
+        using (var host = builder.Build())
+        {
+            var meterFactory = host.Services.GetRequiredService<IMeterFactory>();
+
+            // Use MeterListener for this test because we want to check that a single error.type tag is added.
+            // MetricCollector can't be used for this because it stores tags in a dictionary and overwrites values.
+            var measurementTcs = new TaskCompletionSource<Measurement<double>>();
+            var meterListener = new MeterListener();
+            meterListener.InstrumentPublished = (instrument, meterListener) =>
+            {
+                if (instrument.Meter.Scope == meterFactory &&
+                    instrument.Meter.Name == "Microsoft.AspNetCore.Server.Kestrel" &&
+                    instrument.Name == "kestrel.connection.duration")
+                {
+                    meterListener.EnableMeasurementEvents(instrument);
+                    meterListener.SetMeasurementEventCallback<double>((Instrument instrument, double measurement, ReadOnlySpan<KeyValuePair<string, object>> tags, object state) =>
+                    {
+                        measurementTcs.SetResult(new Measurement<double>(measurement, tags));
+                    });
+                }
+            };
+            meterListener.Start();
+
+            await host.StartAsync();
+            var client = HttpHelpers.CreateClient(maxResponseHeadersLength: 1024);
+
+            // Act
+            using var socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
+            socket.LingerState = new LingerOption(false, 0);
+
+            socket.Connect(IPAddress.Loopback, host.GetPort());
+            socket.Send(new byte[1024 * 16]);
+
+            // Wait for measurement to be available.
+            var measurement = await measurementTcs.Task.DefaultTimeout();
+
+            // Assert
+            Assert.True(measurement.Value > 0);
+
+            var tags = measurement.Tags.ToArray();
+            Assert.Equal("http", (string)tags.Single(t => t.Key == "network.protocol.name").Value);
+            Assert.Equal("2", (string)tags.Single(t => t.Key == "network.protocol.version").Value);
+            Assert.Equal("tcp", (string)tags.Single(t => t.Key == "network.transport").Value);
+            Assert.Equal("ipv4", (string)tags.Single(t => t.Key == "network.type").Value);
+            Assert.Equal("127.0.0.1", (string)tags.Single(t => t.Key == "server.address").Value);
+            Assert.Equal(host.GetPort(), (int)tags.Single(t => t.Key == "server.port").Value);
+            Assert.Equal("invalid_handshake", (string)tags.Single(t => t.Key == "error.type").Value);
+
+            socket.Close();
+
+            await host.StopAsync();
+        }
+    }
+
     [Fact]
     public async Task GET_Metrics_HttpProtocolAndTlsSet()
     {