From 2198e6e918d63acf5d0dbc2fbb093eadb09e3ff9 Mon Sep 17 00:00:00 2001 From: David Chan Date: Wed, 15 Jan 2025 17:54:17 -0500 Subject: [PATCH 1/2] When the server is handling requests made to the server itself (i.e., not through a deployed application) and a 4xx response code is encountered, there should not be an HTTP route resolved for the HTTP stat --- .../http/monitor/HttpServerStatsMonitor.java | 11 ++++++-- .../http/monitor/fat/BaseTestClass.java | 14 +++++------ .../http/monitor/fat/ContainerNoAppTest.java | 25 ++++++++++++++++++- .../http/monitor/fat/NoAppTest.java | 22 +++++++++++++++- 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java b/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java index e72e33d5e0b4..f6b6fc28af27 100644 --- a/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java +++ b/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2024 IBM Corporation and others. + * Copyright (c) 2024, 2025 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -137,13 +137,18 @@ public void atSendResponse(@This Object probedHttpDispatcherLinkObj, @Args Objec tl_startNanos.set(System.nanoTime()); HttpStatAttributes.Builder builder = HttpStatAttributes.builder(); + + boolean is4xxCode = false; + /* * Get Status Code (and Exception if it exists) */ if (myargs.length > 0) { if (myargs[0] != null && myargs[0] instanceof StatusCodes) { StatusCodes statCode = (StatusCodes) myargs[0]; + builder.withResponseStatus(statCode.getIntCode()); + is4xxCode = (statCode.getIntCode() % 400 <= 99 ) ? true : false; } if (myargs[2] != null && myargs[2] instanceof Exception) { @@ -165,8 +170,10 @@ public void atSendResponse(@This Object probedHttpDispatcherLinkObj, @Args Objec try { HttpRequest httpRequest = httpDispatcherLink.getRequest(); + if (!is4xxCode) { + builder.withHttpRoute(httpRequest.getURI()); + } - builder.withHttpRoute(httpRequest.getURI()); builder.withRequestMethod(httpRequest.getMethod()); builder.withScheme(httpRequest.getScheme()); resolveNetworkProtocolInfo(httpRequest.getVersion(), builder); diff --git a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/BaseTestClass.java b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/BaseTestClass.java index 148c952144f9..a523124d93e2 100644 --- a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/BaseTestClass.java +++ b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/BaseTestClass.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2024 IBM Corporation and others. + * Copyright (c) 2024, 2025 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -259,14 +259,14 @@ protected boolean validateMpMetricsHttp(String vendorMetricsOutput, String route + "\",http_request_method=\"" + requestMethod + "\",http_response_status_code=\"" + responseStatus - + "\",http_route=\"" + route + + ((route != null) ? ("\",http_route=\"" + route) : "\",http_route=\"") + "\",mp_scope=\"vendor\",network_protocol_version=\"1\\.[01]\",server_address=\"localhost\",server_port=\"[0-9]+\",url_scheme=\"http\",\\} "; String sumMatchString = "http_server_request_duration_seconds_sum\\{error_type=\"" + errorType + "\",http_request_method=\"" + requestMethod + "\",http_response_status_code=\"" + responseStatus - + "\",http_route=\"" + route + + ((route != null) ? ("\",http_route=\"" + route) : "\",http_route=\"") + "\",mp_scope=\"vendor\",network_protocol_version=\"1\\.[01]\",server_address=\"localhost\",server_port=\"[0-9]+\",url_scheme=\"http\",\\} "; return validatePrometheusHTTPMetricCount(vendorMetricsOutput, route, responseStatus, requestMethod, errorType, expectedCount, countMatchString) && @@ -325,7 +325,7 @@ protected boolean validateMpTelemetryHttp(String appName, String vendorMetricsOu countMatchString = "http_server_request_duration_seconds_count\\{http_request_method=\"" + requestMethod + "\",http_response_status_code=\"" + responseStatus - + "\",http_route=\"" + route + + ((route != null) ? ("\",http_route=\"" + route) : "") + "\",instance=\"[a-zA-Z0-9-]*\"" + ",job=\"" + appName + "\",network_protocol_version=\"1\\.[01]\",server_address=\"localhost\",server_port=\"[0-9]+\",url_scheme=\"http\"\\} "; @@ -333,7 +333,7 @@ protected boolean validateMpTelemetryHttp(String appName, String vendorMetricsOu sumMatchString = "http_server_request_duration_seconds_sum\\{http_request_method=\"" + requestMethod + "\",http_response_status_code=\"" + responseStatus - + "\",http_route=\"" + route + + ((route != null) ? ("\",http_route=\"" + route) : "") + "\",instance=\"[a-zA-Z0-9-]*\"" + ",job=\"" + appName + "\",network_protocol_version=\"1\\.[01]\",server_address=\"localhost\",server_port=\"[0-9]+\",url_scheme=\"http\"\\} "; @@ -342,7 +342,7 @@ protected boolean validateMpTelemetryHttp(String appName, String vendorMetricsOu + "\",http_request_method=\"" + requestMethod + "\",http_response_status_code=\"" + responseStatus - + "\",http_route=\"" + route + + ((route != null) ? ("\",http_route=\"" + route) : "") + "\",instance=\"[a-zA-Z0-9-]*\"" + ",job=\"" + appName + "\",network_protocol_version=\"1\\.[01]\",server_address=\"localhost\",server_port=\"[0-9]+\",url_scheme=\"http\"\\} "; @@ -351,7 +351,7 @@ protected boolean validateMpTelemetryHttp(String appName, String vendorMetricsOu + "\",http_request_method=\"" + requestMethod + "\",http_response_status_code=\"" + responseStatus - + "\",http_route=\"" + route + + ((route != null) ? ("\",http_route=\"" + route) : "") + "\",instance=\"[a-zA-Z0-9-]*\"" + ",job=\"" + appName + "\",network_protocol_version=\"1\\.[01]\",server_address=\"localhost\",server_port=\"[0-9]+\",url_scheme=\"http\"\\} "; diff --git a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java index 68073c59fc96..2c3aa3e74e53 100644 --- a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java +++ b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2024 IBM Corporation and others. + * Copyright (c) 2024, 2025 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -89,4 +89,27 @@ public void c_noApp_splashPage() throws Exception { } + @Test + public void c_noApp_nonExistent() throws Exception { + + assertTrue(server.isStarted()); + + String route = "/madeThisUp"; + String requestMethod = HttpMethod.GET; + String responseStatus = "404"; + + String res = requestHttpServlet(route, server, requestMethod); + + //Allow time for the collector to receive and expose metrics + assertTrueRetryWithTimeout(() -> validateMpTelemetryHttp(Constants.OTEL_SERVICE_NOT_SET, getContainerCollectorMetrics(container), null, responseStatus, requestMethod)); + + String route2 = "/anotherMadeThisUp"; + String res2 = requestHttpServlet(route, server, requestMethod); + + //Allow time for the collector to receive and expose metrics + assertTrueRetryWithTimeout(() -> validateMpTelemetryHttp(Constants.OTEL_SERVICE_NOT_SET, getContainerCollectorMetrics(container), null, responseStatus, requestMethod, null, + ">1", null)); + + } + } diff --git a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java index e32d29569be1..3f4ce495b419 100644 --- a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java +++ b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2024 IBM Corporation and others. + * Copyright (c) 2024, 2025 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -84,4 +84,24 @@ public void noApp_splashPage() throws Exception { } + @Test + public void noApp_nonExistent() throws Exception { + + assertTrue(server.isStarted()); + + String route = "/madeThisUp"; + String requestMethod = HttpMethod.GET; + String responseStatus = "404"; + + String res = requestHttpServlet(route, server, requestMethod); + + assertTrue(validateMpMetricsHttp(getVendorMetrics(server), null, responseStatus, requestMethod)); + + String route2 = "/anotherMadeThisUp"; + String res2 = requestHttpServlet(route, server, requestMethod); + + assertTrue(validateMpMetricsHttp(getVendorMetrics(server), null, responseStatus, requestMethod, null, ">1", null)); + + } + } From 8905021d726ce689eb0bbc4f9aa932fa3ed3f6ee Mon Sep 17 00:00:00 2001 From: David Chan Date: Tue, 21 Jan 2025 16:05:22 -0500 Subject: [PATCH 2/2] For 4xx response codes, set the http route to slash (/) --- .../openliberty/http/monitor/HttpServerStatsMonitor.java | 6 ++++++ .../openliberty/http/monitor/fat/ContainerNoAppTest.java | 7 +++++-- .../fat/src/io/openliberty/http/monitor/fat/NoAppTest.java | 5 +++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java b/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java index f6b6fc28af27..2682781f6ad5 100644 --- a/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java +++ b/dev/io.openliberty.http.monitor/src/io/openliberty/http/monitor/HttpServerStatsMonitor.java @@ -172,6 +172,12 @@ public void atSendResponse(@This Object probedHttpDispatcherLinkObj, @Args Objec HttpRequest httpRequest = httpDispatcherLink.getRequest(); if (!is4xxCode) { builder.withHttpRoute(httpRequest.getURI()); + } else { + /* + * For 4xx response codes, + * set HTTP route to "/" + */ + builder.withHttpRoute("/"); } builder.withRequestMethod(httpRequest.getMethod()); diff --git a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java index 2c3aa3e74e53..f198dc4bf5d0 100644 --- a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java +++ b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/ContainerNoAppTest.java @@ -97,17 +97,20 @@ public void c_noApp_nonExistent() throws Exception { String route = "/madeThisUp"; String requestMethod = HttpMethod.GET; String responseStatus = "404"; + String expectedRoute = "/"; String res = requestHttpServlet(route, server, requestMethod); //Allow time for the collector to receive and expose metrics - assertTrueRetryWithTimeout(() -> validateMpTelemetryHttp(Constants.OTEL_SERVICE_NOT_SET, getContainerCollectorMetrics(container), null, responseStatus, requestMethod)); + assertTrueRetryWithTimeout(() -> validateMpTelemetryHttp(Constants.OTEL_SERVICE_NOT_SET, getContainerCollectorMetrics(container), expectedRoute, responseStatus, + requestMethod)); String route2 = "/anotherMadeThisUp"; String res2 = requestHttpServlet(route, server, requestMethod); //Allow time for the collector to receive and expose metrics - assertTrueRetryWithTimeout(() -> validateMpTelemetryHttp(Constants.OTEL_SERVICE_NOT_SET, getContainerCollectorMetrics(container), null, responseStatus, requestMethod, null, + assertTrueRetryWithTimeout(() -> validateMpTelemetryHttp(Constants.OTEL_SERVICE_NOT_SET, getContainerCollectorMetrics(container), expectedRoute, responseStatus, + requestMethod, null, ">1", null)); } diff --git a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java index 3f4ce495b419..dcf41f1e7dfb 100644 --- a/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java +++ b/dev/io.openliberty.http.monitor_fat/fat/src/io/openliberty/http/monitor/fat/NoAppTest.java @@ -92,15 +92,16 @@ public void noApp_nonExistent() throws Exception { String route = "/madeThisUp"; String requestMethod = HttpMethod.GET; String responseStatus = "404"; + String expectedRoute = "/"; String res = requestHttpServlet(route, server, requestMethod); - assertTrue(validateMpMetricsHttp(getVendorMetrics(server), null, responseStatus, requestMethod)); + assertTrue(validateMpMetricsHttp(getVendorMetrics(server), expectedRoute, responseStatus, requestMethod)); String route2 = "/anotherMadeThisUp"; String res2 = requestHttpServlet(route, server, requestMethod); - assertTrue(validateMpMetricsHttp(getVendorMetrics(server), null, responseStatus, requestMethod, null, ">1", null)); + assertTrue(validateMpMetricsHttp(getVendorMetrics(server), expectedRoute, responseStatus, requestMethod, null, ">1", null)); }