[WIP/experiment] Add prometheus metrics support #518
Conversation
|
Example content of /metrics https://gist.github.com/alikins/39435297bad1637c4d3bc2edcb96f6b9 Grafana screenshot showing some basic metrics: https://imgur.com/a/xbn7uOK |
|
dynaconf will let you override the installed apps like any attribute in the settings module. Still though, I prefer the way this PR is positioning it as an optional dependency that "just works" if its installed. |
pulpcore/app/urls.py
Outdated
| @@ -115,6 +115,7 @@ def __repr__(self): | |||
| url(r'^{api_root}status/'.format(api_root=API_ROOT), StatusView.as_view()), | |||
| url(r'^{api_root}orphans/'.format(api_root=API_ROOT), OrphansView.as_view()), | |||
| url(r'^auth/', include('rest_framework.urls')), | |||
| url('', include('django_prometheus.urls')), | |||
There was a problem hiding this comment.
Because django_prometheus is an optional dependency, urls must be configured conditionally too.
|
WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue. |
|
Note: a1ac736 (making django_prometheus imports optional in urls.py) is untested at the moment |
This enables pulpcore to export pulp metrics to be scraped by prometheus. It uses the django module 'django_prometheus' django app/middleware to do this. (https://github.com/korfuri/django-prometheus) Setup the app. Register the middleware. Add /metrics url. re: #6462 https://pulp.plan.io/issues/6462
[noissue]
There was a problem hiding this comment.
Thank you for adding this support. Our users will appreciate it. There are two things I want to ask about:
The first is docs; it needs user facing docs explaining the url, what users need to install. Also maybe with a screenshot or two? I looked at the docs site and the place I think it makes the most sense at is as a new section titled Prometheus Integration at the bottom of the components page.
Also I think it needs a test. I believe this can be easily done if the CI folks add django_prometheus to the single-container which is the environment that tests this. Then a new test module named test_prometheus.py could be added here perhaps.
Would you be able to add ^?
Also I'll ask the installer team how they want to position an installer feature to install the optional package too.
|
I filed the installer change here: https://pulp.plan.io/issues/6808 |
|
And is there an issue already written up for this support? Good to see this moving forward. |
|
@bmbouter I can update the examples from #518 (comment) and add some docs into docs rst somewhere. |
|
@bmbouter One thing I was wondering about is if it would be useful to add a settings.py variable to explicitly disable prometheus even if the required django-prometheus modules are importable. |
That was my first approach, but didn't have much luck getting that to work. Could be revisited. |
|
We're closing this until the requested changes can be made. Please reopen. |
|
Chiming in here just to say that a user may want to see prometheus metrics around tasks - for example, their duration, total count, and error counts - in addition to the httpd request metrics already implemented here. |
Note: This is mostly just an example of how prometheus metrics support could be added to pulpcore (and presumably, all the pulp plugins).
I'm looking for a more general way to set up this configuration, if that is possible.
For example, I'm not sure this is something that could be done with dynaconf
(it seems like dynaconf's django support doesn't allow changing INSTALLED_APPS or MIDDLEWARE.)
So looking for suggestions on the best way to approach this.