Update mixin to latest changes from grafana/postgres_exporter #1179
Conversation
This rounds to .001, otherwise cache hit axis was showing multiple points with 100% since values were rounded at axis, but not raw values, which changes from 0.9900001 were shown 0.9900002 on graphs, causing visual noise) Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
Use psql specific metric to avoid return of postgres unrelated instances Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
Move QPS to left and remove transparency Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
These metrics are gauges, so applying rate is not required Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
Add missing refId Signed-off-by: Vitaly Zhuravlev <zhuravlev.vitaly@gmail.com>
* lint fixes * Add .lint exclusion file - for mixtool * Update .lint exclusion file - for mixtool
* lint fixes * Add .lint exclusion file - for mixtool * Update .lint exclusion file - for mixtool * reverting overview dashboard changes * delete postgres-overview.libsonnet * bringing back Vitaly's changes * updating instance template definition, adding job filter to datname template
There was a problem hiding this comment.
Seems correct to me, will let @v-zhuravlev take a peek as he is much fresher with our latest mixin standards.
| @@ -1,22 +1,22 @@ | |||
| { | |||
| prometheusAlerts+:: { | |||
| prometheusAlerts+: { | |||
There was a problem hiding this comment.
| prometheusAlerts+: { | |
| prometheusAlerts+:: { |
This should be +:: by monitoring-mixin convention otherwise it woudn't load in some cases (like grizzly).
There was a problem hiding this comment.
I'm happy to apply the change, note though that this is exactly what we use here: https://github.com/grafana/postgres_exporter/blob/master/postgres_mixin/alerts/postgres.libsonnet#L2 (and then imported in cloud-onboarding)
There was a problem hiding this comment.
It works both way in most cases, this one is considered a standard though (hidden fields). And grizzly tool only works with hidden fields.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| grafanaDashboards+:: { | |||
| 'postgres-overview.json': (import 'postgres-overview.json'), | |||
| 'postgresql-overview.json': (import 'postgresql-overview.json'), | |||
There was a problem hiding this comment.
I think this might cause some issues to existing users, best if we keep it as is if possible.
There was a problem hiding this comment.
Similar to the above, I can keep the dashboard name unchanged but consider that this is what we use in cloud-onboarding: it means that when we import the mixin from this repo, we'll need to change the dashboard name there.
|
Looks like the DCO will need to be cleaned up. What is the plan with the Grafana fork? Is it intended to be a long term fork or are there other Grafana specific reasons for it? This PR shows intent to upstream some changes so I'm just curious. We don't necessarily have mixin standards in this project so I'm open to suggestions, however I did start to refactor the mixin in #911 |
Hi Joe! I will squash commits (if @gaantunes and @v-zhuravlev are fine with that) and fix the DCO once the changes are in a good shape, if that's ok. We'd ideally avoid keeping the fork around, but we need some changes to embed it into Alloy. My intention is to start submitting some of the changes here so we can narrow the gap and at least make the fork easier to maintain. What is your plan with #911? I'm wondering if any improvement there can be applied on top of the changes we have here? |
|
I'm not set on #911 but I do want to make the mixin easier to maintain. I think that splitting queries, dashboards, alerts, and other types into separate files is helpful in that regard. |
Would you still be ok with getting this change in, then building on top of it? |
Porting a bunch of PRs accumulated over time in grafana/postgres_exporter:
grafana#11 (@v-zhuravlev)
grafana#12 (@gaantunes)
grafana#13 (@gaantunes)
grafana#14 (@gaantunes)
grafana#15 (@gaantunes)
grafana#16 (@gaantunes)
grafana#17 (@gaantunes)
grafana#20 (@gaantunes)
grafana#21 (@mshahzeb)
grafana#22 (@mshahzeb)