Make sure we are sending the same dimensions to each metric

Also - cleaned up metrics update - Record 404s as NotFound
hummingbird-project · Oct 14, 2024 · d0f893a · d0f893a
1 parent a561c5f
commit d0f893a
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 21 deletions.
diff --git a/Sources/Hummingbird/Middleware/MetricsMiddleware.swift b/Sources/Hummingbird/Middleware/MetricsMiddleware.swift
@@ -58,24 +58,25 @@ public struct MetricsMiddleware<Context: RequestContext>: RouterMiddleware {
             } else {
                 errorType = "500"
             }
-            // need to create dimensions once request has been responded to ensure
-            // we have the correct endpoint path
-            let dimensions: [(String, String)]
-            // Don't record uri in 404 errors, to avoid spamming of metrics
-            if let endpointPath = context.endpointPath {
-                dimensions = [
+            let endpointPath = context.endpointPath ?? "NotFound"
+            // increment requests
+            Counter(
+                label: "hb.requests",
+                dimensions: [
                     ("http.route", endpointPath),
                     ("http.request.method", request.method.rawValue),
-                    ("error.type", errorType),
+                    ("http.response.status_code", errorType),
                 ]
-                Counter(label: "hb.requests", dimensions: dimensions).increment()
-            } else {
-                dimensions = [
+            ).increment()
+            // increment errors
+            Counter(
+                label: "hb.request.errors",
+                dimensions: [
+                    ("http.route", endpointPath),
                     ("http.request.method", request.method.rawValue),
                     ("error.type", errorType),
                 ]
-            }
-            Counter(label: "hb.request.errors", dimensions: dimensions).increment()
+            ).increment()
             activeRequestMeter.decrement()
             throw error
         }

diff --git a/Tests/HummingbirdTests/MetricsTests.swift b/Tests/HummingbirdTests/MetricsTests.swift
@@ -219,13 +219,21 @@ final class MetricsTests: XCTestCase {
             try await client.execute(uri: "/hello", method: .get) { _ in }
         }
 
-        let counter = try XCTUnwrap(Self.testMetrics.counters.withLockedValue { $0 }["hb.request.errors"] as? TestCounter)
-        XCTAssertEqual(counter.values.withLockedValue { $0 }.count, 1)
+        let counter = try XCTUnwrap(Self.testMetrics.counters.withLockedValue { $0 }["hb.requests"] as? TestCounter)
         XCTAssertEqual(counter.values.withLockedValue { $0 }[0].1, 1)
         XCTAssertEqual(counter.dimensions[0].0, "http.route")
         XCTAssertEqual(counter.dimensions[0].1, "/hello")
         XCTAssertEqual(counter.dimensions[1].0, "http.request.method")
         XCTAssertEqual(counter.dimensions[1].1, "GET")
+        XCTAssertEqual(counter.dimensions[2].0, "http.response.status_code")
+        XCTAssertEqual(counter.dimensions[2].1, "400")
+        let errorCounter = try XCTUnwrap(Self.testMetrics.counters.withLockedValue { $0 }["hb.request.errors"] as? TestCounter)
+        XCTAssertEqual(errorCounter.values.withLockedValue { $0 }.count, 1)
+        XCTAssertEqual(errorCounter.values.withLockedValue { $0 }[0].1, 1)
+        XCTAssertEqual(errorCounter.dimensions[0].0, "http.route")
+        XCTAssertEqual(errorCounter.dimensions[0].1, "/hello")
+        XCTAssertEqual(errorCounter.dimensions[1].0, "http.request.method")
+        XCTAssertEqual(errorCounter.dimensions[1].1, "GET")
     }
 
     func testNotFoundError() async throws {
@@ -239,14 +247,24 @@ final class MetricsTests: XCTestCase {
             try await client.execute(uri: "/hello2", method: .get) { _ in }
         }
 
-        let counter = try XCTUnwrap(Self.testMetrics.counters.withLockedValue { $0 }["hb.request.errors"] as? TestCounter)
-        XCTAssertEqual(counter.values.withLockedValue { $0 }.count, 1)
+        let counter = try XCTUnwrap(Self.testMetrics.counters.withLockedValue { $0 }["hb.requests"] as? TestCounter)
         XCTAssertEqual(counter.values.withLockedValue { $0 }[0].1, 1)
-        XCTAssertEqual(counter.dimensions.count, 2)
-        XCTAssertEqual(counter.dimensions[0].0, "http.request.method")
-        XCTAssertEqual(counter.dimensions[0].1, "GET")
-        XCTAssertEqual(counter.dimensions[1].0, "error.type")
-        XCTAssertEqual(counter.dimensions[1].1, "404")
+        XCTAssertEqual(counter.dimensions[0].0, "http.route")
+        XCTAssertEqual(counter.dimensions[0].1, "NotFound")
+        XCTAssertEqual(counter.dimensions[1].0, "http.request.method")
+        XCTAssertEqual(counter.dimensions[1].1, "GET")
+        XCTAssertEqual(counter.dimensions[2].0, "http.response.status_code")
+        XCTAssertEqual(counter.dimensions[2].1, "404")
+        let errorCounter = try XCTUnwrap(Self.testMetrics.counters.withLockedValue { $0 }["hb.request.errors"] as? TestCounter)
+        XCTAssertEqual(errorCounter.values.withLockedValue { $0 }.count, 1)
+        XCTAssertEqual(errorCounter.values.withLockedValue { $0 }[0].1, 1)
+        XCTAssertEqual(errorCounter.dimensions.count, 3)
+        XCTAssertEqual(errorCounter.dimensions[0].0, "http.route")
+        XCTAssertEqual(errorCounter.dimensions[0].1, "NotFound")
+        XCTAssertEqual(errorCounter.dimensions[1].0, "http.request.method")
+        XCTAssertEqual(errorCounter.dimensions[1].1, "GET")
+        XCTAssertEqual(errorCounter.dimensions[2].0, "error.type")
+        XCTAssertEqual(errorCounter.dimensions[2].1, "404")
     }
 
     func testParameterEndpoint() async throws {