Prevent exception in HttpServer exporter when using custom root metrics path

[Donnerbart]

The behavior of HttpServer.createContext() was changed between Java 11 and 21. In the current JDKs the server doesn't allow to register multiple contexts with the same path. So if you configure

HTTPServer.builder()
            .port(0)
            .registry(registry)
            .metricsHandlerPath("/")
            .buildAndStart()

you get the following exception:

java.lang.IllegalArgumentException: cannot add context to list
	at jdk.httpserver/sun.net.httpserver.ContextList.add(ContextList.java:37)
	at jdk.httpserver/sun.net.httpserver.ServerImpl.createContext(ServerImpl.java:306)
	at jdk.httpserver/sun.net.httpserver.HttpServerImpl.createContext(HttpServerImpl.java:69)
	at jdk.httpserver/sun.net.httpserver.HttpServerImpl.createContext(HttpServerImpl.java:34)
	at io.prometheus.metrics.exporter.httpserver.HTTPServer.registerHandler(HTTPServer.java:111)

This PR fixes the issue by skipping the default handler registration in case the metrics are configured for the root path.

I also improved the unit test so we can properly assert expectedStatusCode and expectedBody. With this improvement we can be sure that an endpoint actually returns the expected handler content, not just a matching status code.

[zeitlinger]

it looks as though this is not backwards compatible - maybe add a setting to opt in?

[Donnerbart]

What exactly do you think breaks the backwards compatibility? With Java 11 it should already behave exactly like this (replacing the previously registered default handler). And with a newer Java runtime the current implementation doesn't work at all.

[zeitlinger]

What exactly do you think breaks the backwards compatibility?

That registration would not work the first time

What about this change?

Index: prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java b/prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java
--- a/prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java	(revision 7f93e015dc64160204b51d13aebf75c83044a364)
+++ b/prometheus-metrics-exporter-httpserver/src/main/java/io/prometheus/metrics/exporter/httpserver/HTTPServer.java	(date 1768917816428)
@@ -9,6 +9,9 @@
 import com.sun.net.httpserver.HttpsServer;
 import io.prometheus.metrics.config.PrometheusProperties;
 import io.prometheus.metrics.model.registry.PrometheusRegistry;
+
+import javax.annotation.Nullable;
+import javax.security.auth.Subject;
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
@@ -21,8 +24,6 @@
 import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
-import javax.annotation.Nullable;
-import javax.security.auth.Subject;
 
 /**
  * Expose Prometheus metrics using a plain Java HttpServer.
@@ -66,11 +67,21 @@
     this.server = httpServer;
     this.executorService = executorService;
     String metricsPath = getMetricsPath(metricsHandlerPath);
+    try {
+      server.removeContext("/");
+    } catch (IllegalArgumentException e) {
+      // context "/" not registered yet, ignore
+    }
     registerHandler(
         "/",
         defaultHandler == null ? new DefaultHandler(metricsPath) : defaultHandler,
         authenticator,
         authenticatedSubjectAttributeName);
+    try {
+      server.removeContext(metricsPath);
+    } catch (IllegalArgumentException e) {
+      // context metricsPath not registered yet, ignore
+    }
     registerHandler(
         metricsPath,
         new MetricsHandler(config, registry),

[Donnerbart]

To my knowledge contexts/handlers are registered per HttpServer instance, not globally. So how could / be registered already? Same with the metricsPath, unless the metrics path is / (then we would have already registered the default handler on this path).

At least I'm not able to come up with a test that already has a context registered on /, so the first removeContext() call actually does something. And the double registration of the metrics handler on / is fixed with less complexity by my if equals check.

I think both approaches will effectively do the same, so I pushed your suggestion.

[zeitlinger]

thanks @Donnerbart - I'll merge once #1781 is merged

[zeitlinger]

@Donnerbart now it is failing for the Java 17 test

[Donnerbart]

Error:  /home/runner/work/client_java/client_java/prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HTTPServerTest.java:[217,27] incompatible types: try-with-resources not applicable to variable type
Error:      (java.net.http.HttpClient cannot be converted to java.lang.AutoCloseable)

Ah, right, I think AutoCloseable was added in Java 21 to that class. I've removed the try-with-resources from the test, should work now.

EDIT: Sigh, the close() and shutdown() methods were introduced in Java 21 as well. If this test needs to run with Java 17, I can't really close the client at all (beside using reflection). I'll remove the close() call for now and add a comment why the client is not closed.

[zeitlinger]

thanks a lot @Donnerbart