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

Add multiprocess support #16

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

VladVolchkov
Copy link

@VladVolchkov VladVolchkov commented Feb 14, 2024

relates to #15

@deepminimal
Copy link

deepminimal commented Mar 22, 2024

@savar @mbarouski Hello! Can you check it please?

@savar
Copy link
Collaborator

savar commented Apr 6, 2024

@savar @mbarouski Hello! Cat you check it please?

Hey, I am not experienced in this multiprocess part of the prometheus client.
I am trying to read the doc but I am not sure that this Collector stuff can be done only once in the initializer as the example in https://prometheus.github.io/client_python/multiprocess/ is doing this every time the scrape is happening.

Also as far as I can tell the multiprocess would require a properly set PROMETHEUS_MULTIPROC_DIR environment variable to work. Maybe we need to test for this envrionment variable to be set and only if, then we start the multiprocess part, otherwise we stick to the existing setup?

Do you have a working example deployment where this is being used, so that we could have a better look onto it?

@savar
Copy link
Collaborator

savar commented Apr 6, 2024

It looks like it could work but we will lose the python_* and process_* as well as the *_created metrics. Also I would like to not use this when there is only a single process being used, specifically because we do not execute the CollectorRegistry+MultiProcessCollector on each scrape request but only once initially. The documentation states it has a risk, but I cannot say right now if this applies to us here.

I just cannot figure out how we could detect the multiprocess situation. Thumbor is not providing this information in the config it passes into the initializer and relying on the environment variable would only mean that people do not realize that they didn't configure it correctly. 🤔

@sboisson
Copy link

sboisson commented Jul 5, 2024

Maybe only use CollectorRegistry and MultiProcessCollector when PROMETHEUS_MULTIPROC_DIR env var is set ?

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.

5 participants