rycus86 / prometheus_flask_exporter

Prometheus exporter for Flask applications
https://pypi.python.org/pypi/prometheus-flask-exporter
MIT License
642 stars 162 forks source link

Make ConnexionPrometheusMetrics._create_response_converter Public #93

Open dtaniwaki opened 3 years ago

dtaniwaki commented 3 years ago

I have a Connexion app which runs by flask run directly in the dev env, and with gunicorn in the production env, so I can't directly use ConnexionPrometheusMetrics. I made the following code to make this package work with Connexion in both env.

For direct run,

PrometheusMetrics(
    app,
    response_converter=ConnexionPrometheusMetrics._create_response_converter("application/json"),
)

For gunicorn,

GunicornPrometheusMetrics(
    app,
    response_converter=ConnexionPrometheusMetrics._create_response_converter("application/json"),
)

It looks fine, but the method to create a response converter for ConnexionPrometheusMetrics is _-prefixed, which should indicate that using it directly is not supported. Could you consider making it no _ prefixed and officially support using it from anywhere?

rycus86 commented 3 years ago

Thanks for reporting this @dtaniwaki , I hadn't considered these differences when running locally vs in prod. We can make it non-underscored (though should leave the old version there as well for backwards compatibility). Do you think just exposing it as is makes sense, or is there a better pattern that may be more useful here? Thinking of maybe something like ConnexionPrometheusMetrics.create_json_response_converter() ?

dtaniwaki commented 3 years ago

I think we're not sure if the default content type is JSON for all the users.

So, I think the following 2 methods satisfies any user's usecases.

ConnexionPrometheusMetrics.context_type(content_type) ConnexionPrometheusMetrics.create_response_converter(default_mimetype)