Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

saeedAdelpour
Copy link

@saeedAdelpour saeedAdelpour commented May 27, 2023

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 saeedAdelpour force-pushed the fix-use-multi-process-for-load-metrics branch from f2bd661 to e6493bc Compare May 27, 2023 17:46
@saeedAdelpour
Copy link
Author

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

@asherf
Copy link
Collaborator

asherf commented Feb 9, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants