Skip to content

Commit

Permalink
Merge pull request #30568 from Channyboy/30567-4xx
Browse files Browse the repository at this point in the history
Prevent http route resolution for requests made directly to server with 4xx response codes.
  • Loading branch information
Channyboy authored Jan 22, 2025
2 parents 148181b + 8905021 commit 60c9c79
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -165,8 +170,16 @@ public void atSendResponse(@This Object probedHttpDispatcherLinkObj, @Args Objec
try {

HttpRequest httpRequest = httpDispatcherLink.getRequest();
if (!is4xxCode) {
builder.withHttpRoute(httpRequest.getURI());
} else {
/*
* For 4xx response codes,
* set HTTP route to "/"
*/
builder.withHttpRoute("/");
}

builder.withHttpRoute(httpRequest.getURI());
builder.withRequestMethod(httpRequest.getMethod());
builder.withScheme(httpRequest.getScheme());
resolveNetworkProtocolInfo(httpRequest.getVersion(), builder);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) &&
Expand Down Expand Up @@ -325,15 +325,15 @@ 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\"\\} ";

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\"\\} ";
Expand All @@ -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\"\\} ";
Expand All @@ -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\"\\} ";
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -89,4 +89,30 @@ 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 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), 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), expectedRoute, responseStatus,
requestMethod, null,
">1", null));

}

}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -84,4 +84,25 @@ 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 expectedRoute = "/";

String res = requestHttpServlet(route, server, requestMethod);

assertTrue(validateMpMetricsHttp(getVendorMetrics(server), expectedRoute, responseStatus, requestMethod));

String route2 = "/anotherMadeThisUp";
String res2 = requestHttpServlet(route, server, requestMethod);

assertTrue(validateMpMetricsHttp(getVendorMetrics(server), expectedRoute, responseStatus, requestMethod, null, ">1", null));

}

}

0 comments on commit 60c9c79

Please sign in to comment.