Using start_http_server results into undefined behaviour

[sbrandtb]

When starting a Flask app in debug mode (werkzeug server) on port, let's say, 5000 and using PrometheusMetrics.start_http_server on another port, let's say 9000, Prometheus endpoint is served properly on port 9000, but also on port 5000, resulting in responses that come randomly from the original or the prometheus Flask app.

I'm not sure if that's a limitation of werkzeug - however, when running the original Flask app with another WSGI container, this behaviour disappears.

So, why not just use start_http_server of prometheus_client under the hood? Is there a particular reason to serve metrics via Flask?

[rycus86]

Hi!

Thanks for trying this library! :)
I'll have a look at this in a little while and will try to fix it.
Does it only happen in debug mode?

The reason for exposing the metrics endpoint with Flask is that, I thought, we already have an HTTP server to use, why start a different one.
Having said that, I think you can avoid having it auto-registered on Flask, and start it through the prometheus_client, but I'll double-check this for you.

Thanks!

[rycus86]

Hi @sbrandtb ,

I've done some changes to try detect if the app is running in debug mode.
With this, you shouldn't get the metrics endpoint's responses on the main Flask app.

There is one caveat though, the actual metrics endpoint won't see the changes that are loaded via the reloader.

Let me know if this makes things better for you or not!
The change will be in version 0.2.1

Thanks!

[sbrandtb]

Hi, thanks for responding so quickly!

Thanks for trying this library! :)
Sure, it's nice. Btw, you inspired me to write the same for tornado (very early alpha): https://github.com/FRI-DAY/tornado-prometheus-exporter

The reason for exposing the metrics endpoint with Flask is that, I thought, we already have an HTTP server to use, why start a different one.

That's true if you choose the same port. But if you choose a different port, like we and AFAIK lots of others do because it makes it easier to not expose metrics to the outside world, then it makes not that much sense anymore.

How I solved it:

def init_prometheus(port):
    log.info('Starting prometheus export on port %s', port)
    start_http_server(port)

app = your_global_instance

def init_app(...):
    # run all kind of initialization to the app depending on parameters to main

def main(...):
    ...
    if not flag_that_tells_if_will_run_in_reload or is_running_from_reloader():
        init_prometheus(prometheus_port)
        init_app(...)
    app.run(...)

The important point here is that if we are running with reload, the first invocation of main will happen in the parent process that will never serve the application. Instead, werkzeug will spawn subprocesses with the same environment (plus the variable that is detected in is_running_from_reloader) and command line parameters when calling app.run() before it actually starts its server/socket.

So, if we would create the server/socket for prometheus in the first invocation, the next invocation would cause an IOError because the port is used already. However, if we would create the prometheus only for the parent process and not subprocesses, it would never show the metrics of the subprocesses.

I haven't tested your changes, but I think it would suffer from the same issue, wouldn't it? is_running_from_reloader would return false on the first invocation and start the prometheus server, but for the actual children, we would not have the prometheus exporter at all.

While one could argue that having metrics in development is not required, it's still confusing and makes it harder to test, doesn't it?

[rycus86]

Thanks for looking into this some more!
I've had another quick look, and I'm not sure at this point how easy it would be to support this use case.
The solution you posted looks promising, just not sure how I'd integrate it into the library.
You're absolutely correct, the changes I've done do suffer from the same issue you described.

I get that it might be confusing to have the metrics endpoint not reflecting updates in debug mode, but I think for the time being, I'm going to just add this as a note to the readme.

Thanks again for this very valuable input, and for mentioning this library in your project. :)

[skbly7]

+1 for having metrics in development via configurable parameter just in case you reconsider in future.

[rycus86]

I'd accept PRs if you'd have time to investigate a solution for it? :)

[rycus86]

@skbly7 I've added some very basic support for metrics in debug=True mode in version 0.5.1. To get it, make sure you have the DEBUG_METRICS environment variable set to something non-empty.
Let me know if this makes things better for you? :)