Add metrics for Graphite Web #2240
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2240 +/- ##
==========================================
- Coverage 79.6% 79.58% -0.03%
==========================================
Files 85 87 +2
Lines 8857 8889 +32
Branches 1897 1900 +3
==========================================
+ Hits 7051 7074 +23
- Misses 1545 1551 +6
- Partials 261 264 +3
Continue to review full report at Codecov.
|
webapp/graphite/urls.py
Outdated
|
|
||
| try: | ||
| graphite_urls.append( | ||
| url('^prometheus/', include('django_prometheus.urls')), |
There was a problem hiding this comment.
should it be something like prometeus-metrics/ ? Thinking about possible (logical) clash with #2195
There was a problem hiding this comment.
prometheus-metrics/metrics isn't nice. Maybe having prometheus/metrics, prometheus/write, prometheus/read isn't really an issue ? I could just create a prometheus apps that will later be extended to add /read and redirect its /metrics to django-prometheus
|
I like the idea a lot, but I'm much less keen on coupling ever more tightly with django, and I would like to see instrumentation of at least the basic steps in the find, render and processing pipeline so we can get some of the same stats that right now are just in the logs. |
Per view/method/service stats really need to be done at the framework level. If it ever moves to another framework the metrics will change, but that's fine. Using django-prometheus gives us most of the metrics we want for free
Sure, I'll add something that doesn't depend directly on the view. I'll probably instrument FYI, here is how it currently looks like: metrics.txt |
|
I also not really like more Django, but it's better to have some metrics then no metrics, right? |
|
The django related metrics already have Django in their names. Would not
make much sense for process_fds for example :)
Le 22 févr. 2018 14:24, "Denis Zhdanov" <[email protected]> a écrit :
… I also not really like more Django, but it's better to have some metrics
then no metrics, right?
Maybe default prefiux should be switched to graphite.django. ?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2240 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA_DA0zHf1radSf91jie9-L7S4Y_GRe1ks5tXWjGgaJpZM4SO49s>
.
|
Ironically, graphite-web exposes no metrics at all, which makes it hard to monitor things (people can implement things in their webservers but this won't have the same level of details) This takes advantage of Prometheus's client library and django integration to add django-related metrics and allow for additional metrics to be added later. Bonus point, we get Prometheus support too. Also adds three basic metrics: - storage_find_latency_seconds (per finder + global) - storage_fetch_latency_seconds (per finder + global) - prefetched_series (number of prefetched series per expression)
|
Here it is with a few basic metrics on top of what django provides for http endpoints |
|
Yes, but now we have hard dependency on |
|
Yeah, those metric names are horrible also. I think that it makes more sense for us to build this ourselves or to use a more graphite-friendly library. |
|
Maybe.. I'll experiment with django-prometheus insternally then, it's doable to integrate it using local_settings.py only |
4 participants
Ironically, graphite-web exposes no metrics at all, which makes it hard to
monitor things (people can implement things in their webservers but this
won't have the same level of details)
This takes advantage of Prometheus's client library and django integration
to add django-related metrics and allow for additional metrics to be added
later. Bonus point, we get Prometheus support too.