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

respect PROMETHEUS_MULTIPROC_DIR in example metrics view #39

Merged
merged 1 commit into from Feb 5, 2024

Conversation

celloni
Copy link

@celloni celloni commented Apr 23, 2022

Hey

Since prometheus_client:0.10.x deprecated prometheus_multiproc_dir in favor of PROMETHEUS_MULTIPROC_DIR.
So I updated the example metrics view to also respect PROMETHEUS_MULTIPROC_DIR - wdyt?

severo added a commit to huggingface/dataset-viewer that referenced this pull request Aug 3, 2022
Note that it's not fixed in starlette-prometheus. See
perdy/starlette-prometheus#39.
severo added a commit to huggingface/dataset-viewer that referenced this pull request Aug 3, 2022
* feat: 🎸 produce the admin service metrics in multiprocess mode

See
https://github.com/prometheus/client_python#multiprocess-mode-eg-gunicorn
and
#250 (comment).

It works as expected:
- https://datasets-server.huggingface.co/admin/metrics gives the metrics
- the /prometheus directory on the admin pod is filled with the shared
files:

```
I have no name!@datasets-server-prod-admin-596d8d5688-nlz72:/src/services/admin$ ls /prometheus
counter_10.db  counter_12.db  counter_14.db  counter_16.db  counter_18.db    gauge_all_11.db  gauge_all_13.db  gauge_all_15.db  gauge_all_17.db  histogram_10.db  histogram_12.db  histogram_14.db  histogram_16.db  histogram_18.db
counter_11.db  counter_13.db  counter_15.db  counter_17.db  gauge_all_10.db  gauge_all_12.db  gauge_all_14.db  gauge_all_16.db  gauge_all_18.db  histogram_11.db  histogram_13.db  histogram_15.db  histogram_17.db
```

* feat: 🎸 use the multiprocess mode for api metrics

* fix: 🐛 use a different directory for each pod

* fix: 🐛 fix the name of the env var!

Note that it's not fixed in starlette-prometheus. See
perdy/starlette-prometheus#39.

* fix: 🐛 fix the name of the env var

* feat: 🎸 update the docker images and use /tmp for metrics
mattstern31 added a commit to mattstern31/datasets-server-storage-admin that referenced this pull request Nov 11, 2023
* feat: 🎸 produce the admin service metrics in multiprocess mode

See
https://github.com/prometheus/client_python#multiprocess-mode-eg-gunicorn
and
huggingface/dataset-viewer#250 (comment).

It works as expected:
- https://datasets-server.huggingface.co/admin/metrics gives the metrics
- the /prometheus directory on the admin pod is filled with the shared
files:

```
I have no name!@datasets-server-prod-admin-596d8d5688-nlz72:/src/services/admin$ ls /prometheus
counter_10.db  counter_12.db  counter_14.db  counter_16.db  counter_18.db    gauge_all_11.db  gauge_all_13.db  gauge_all_15.db  gauge_all_17.db  histogram_10.db  histogram_12.db  histogram_14.db  histogram_16.db  histogram_18.db
counter_11.db  counter_13.db  counter_15.db  counter_17.db  gauge_all_10.db  gauge_all_12.db  gauge_all_14.db  gauge_all_16.db  gauge_all_18.db  histogram_11.db  histogram_13.db  histogram_15.db  histogram_17.db
```

* feat: 🎸 use the multiprocess mode for api metrics

* fix: 🐛 use a different directory for each pod

* fix: 🐛 fix the name of the env var!

Note that it's not fixed in starlette-prometheus. See
perdy/starlette-prometheus#39.

* fix: 🐛 fix the name of the env var

* feat: 🎸 update the docker images and use /tmp for metrics
@perdy perdy merged commit 10ffd5e into perdy:master Feb 5, 2024
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.

None yet

2 participants