Start Prometheus HTTP server on the provided executor

[mberndt123]

The metrics HTTP server should run on a daemon JVM
thread so as to not keep the JVM running even
though all relevant threads have already exited.
Unfortunately, the OpenJDK builtin HTTP server
likes to spawn its own HTTP Dispatcher thread on
startup.
https://github.com/openjdk/jdk/blob/02c95a6d7eb77ed17ae64d0f585197e87a67cc4a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java#L190
This new thread inherits the daemon flag from the
thread that started it, therefore we need to call
the start method from the executor service,
because those threads are daemon threads.

[mberndt123]

OK, here's the thing. This change is working exactly as intended and prevents Prometheus from spawning non-Daemon threads. In the tests however, this Prometheus thread is the only non-Daemon thread other than the main thread, and the main thread exits right after starting the Prometheus HTTP server, leading to the termination of the program before the tests can complete.

There are probably programs out there that run everything in daemon threads, the only thing keeping their JVM alive being the Prometheus HTTP server, and so this change would break their program.
I would argue that this is a bug in their program, and that it's more important to prevent an application from entering “Zombie mode” where the only thing that's left running is the metrics HTTP server, than it is to support such users. But that's a call that the library maintainers need to make.

[dhoard]

There are probably programs out there that run everything in daemon threads, the only thing keeping their JVM alive being the Prometheus HTTP server, and so this change would break their program. I would argue that this is a bug in their program, and that it's more important to prevent an application from entering “Zombie mode” where the only thing that's left running is the metrics HTTP server, than it is to support such users. But that's a call that the library maintainers need to make.

I feel this is a good change - the application should be responsible for preventing the application from exiting and shouldn't rely on HTTPServer threads.

If this PR causes test issues/failures in client_java due to the behavior change, then the client_java tests should be fixed as part of the PR.

---

We could add a join method to HTTPServer for convenience...

public void join() {
    try {
        Thread.currentThread().join();
    } catch (InterruptedException e) {
        // DO NOTHING
    }
}

[mberndt123]

Thanks you for your encouraging remarks @dhoard.

Regarding the joinmethod you've proposed, I'm not quite sure I understand. When a thread waits for itself to terminate, that's just a deadlock, right? The tests already work thanks to the sleep statement that I added – 10 seconds should easily be enough for the test to complete, and it doesn't slow down the tests because afaics the test doesn't wait for the test server to terminate.

[dhoard]

Regarding the joinmethod you've proposed, I'm not quite sure I understand. When a thread waits for itself to terminate, that's just a deadlock, right?

The thought was if someone wanted the HTTPServer to block the application from exiting, we could provide an HTTPServer join method for convenience.

class SomeApplication {
    
    public static void main(String[] args) {
        // code that creates and runs daemon threads
        
        // create the HTTPServer
        HTTPServer httpServer = ...
        
        // cause the main thread to sleep until interrupted,
        // blocking the application from exiting
        httpServer.join();
    }
}

[mberndt123]

Oh, I see what you mean now. I think such functionality is beyond the scope of the library and it doesn't really save much code anyway.

[dhoard]

Minor changes.

[dhoard]

@mberndt123 Thanks for the PR!