korfuri / django-prometheus

Export Django monitoring metrics for Prometheus.io
Apache License 2.0
1.44k stars 244 forks source link

fix(exports): use try except to handle port in use by django gunicorn #407

Closed saeedAdelpour closed 7 months ago

saeedAdelpour commented 1 year ago

Hi,

Thank you guys for your very powerful library, You are GREAT!

When we run gunicorn command to serve django app, the command is

gunicorn APP.wsgi:application --name APP --workers 4 --bind=0.0.0.0:8000 --log-level=INFO --log-file=-

then 4 process runs this part of code, one of them allocates metrics on the PROMETHEUS_METRICS_EXPORT_PORT and if one of them allocates, this is enough for the rest, because all other processes write their metrics in PROMETHEUS_MULTIPROC_DIR

i had this workaround in my django to solve this issue


if config("PROMETHEUS_ENABLE", cast=bool, default=False):
    # there was a bug here. when we import libs and then assign env variables, the metrics didnt store

    PROMETHEUS_MULTIPROC_DIR = config("PROMETHEUS_MULTIPROC_DIR")
    if not os.environ.get("PROMETHEUS_MULTIPROC_DIR"):
        os.environ["PROMETHEUS_MULTIPROC_DIR"] = config("PROMETHEUS_MULTIPROC_DIR")
    if not os.path.exists(PROMETHEUS_MULTIPROC_DIR):
        os.makedirs(PROMETHEUS_MULTIPROC_DIR)

    from prometheus_client.multiprocess import MultiProcessCollector
    from prometheus_client import CollectorRegistry, start_http_server

    INSTALLED_APPS += ["django_prometheus"]
    MIDDLEWARE.insert(0, "django_prometheus.middleware.PrometheusBeforeMiddleware")
    MIDDLEWARE.append("django_prometheus.middleware.PrometheusAfterMiddleware")

    registry = CollectorRegistry()
    MultiProcessCollector(registry)
    try:
        start_http_server(8001, registry=registry)
    except OSError:
        """
        first process serves metrics on port 8001, other processes raise error: port already in use
        one processes collect metrics from PROMETHEUS_MULTIPROC_DIR
        """

this workaround dosen't work on celery processes, we need to run these export PROMETHEUS_MULTIPROC_DIR=PATH/TO/ celery ....

when we set this env variable on OS layer, we can use this code in our backend

if config("PROMETHEUS_ENABLE", cast=bool, default=False):
    PROMETHEUS_METRICS_EXPORT_PORT = 8001
    PROMETHEUS_METRICS_EXPORT_ADDRESS = "0.0.0.0"
    INSTALLED_APPS += ["django_prometheus"]
    MIDDLEWARE.insert(0, "django_prometheus.middleware.PrometheusBeforeMiddleware")
    MIDDLEWARE.append("django_prometheus.middleware.PrometheusAfterMiddleware")

I still checking python client to solve the issue,

saeedAdelpour commented 7 months ago

Hi @asherf I wanted to kindly ask if you could spare a moment to review my recent pull request? Thank you so much

asherf commented 7 months ago

hi, thanks for the PR. I think this is not a good change in the context of this package. it complicates the code. the app can run this type of code instead of calling this package. I think this type of code in the package (setting up an endpoint) is mostly for reference. ideally, the app should define its own endpoint function and implement it in a way that fits the app specific reqs.