Add/non slo redis alerts

[wejdross]

Enabling Redis System Metrics, because of:

• REDIS_EXPORTER_INCL_SYSTEM_METRICS | Whether to include system metrics like total_system_memory_bytes, defaults to false.
• all rules definitions around Internet use them, so I prefer to reuse them
• I made nice workaround to those metrics, but it was much longer with bunch of regexes and I dropped that afterwards

Redis Storage was simply copied from PostgreSQL storage

[Kidswiss]

all rules definitions around Internet use them, so I prefer to reuse them

I get why this could be a reason to go with the specific metrics but here's an idea:

If we use the kube-state-metrics like with PostgreSQL for memory and storage then we can put the generation of those rules into jsonnet functions and just parameterize them. Like what I did for the sloth rules.

Then we won't run into the risk of copy/paste errors, and we know all the metrics are from the same source. If we ever need to adjust them, we can do it in one place.

[wejdross]

Ok, we could abstract that, but we still need to enable additional metrics for both customers and us, especially if we hit clusters monitoring, I'm flexible with both solutions - @TheBigLee be the tiebreaker :D

Those are rules I was talking about, there are few pages aggregating Prometheus rules and they all reuse those additional metrics, this is why I think it's worth enabling them
https://samber.github.io/awesome-prometheus-alerts/rules.html#redis

[Kidswiss]

Sure, I'm not against enabling the additional metrics, they will be useful for additional alerts and dashboards.

I'm just saying we should use the kube-state-metrics for storage and memory, as they are service agnostic.

[Kidswiss]

The rendered composition looks a bit broken. Also, before merging this test it on the lab and also check if the existing dashboards on insights need adjustments.

[wejdross]

@Kidswiss @zugao
MR is ready to be roasted :D

it compiles and works on Lab

[Kidswiss]

Some things look a bit broken in the rendered yaml files.

Also please test it on the lab to see if something in the dashboards would also need fixing.

[zugao]

As Simon already pointed out there are changes in postgres files which should not be there. This PR should not change anything in Postgres, at best only the names.

[zugao]

Changes are requested

[TheBigLee]

As Simon already pointed out there are changes in postgres files which should not be there. This PR should not change anything in Postgres, at best only the names.

This has been fixed now

[TheBigLee]

Also please test it on the lab to see if something in the dashboards would also need fixing.

What do you mean? The dashboards are not affected by this, as the dashboards use the metrics and not the alertRules. And nothing changed with the metrics themselves.

[TheBigLee]

I've refactored the prometheus code a bit and put it into a separate file and also added the missing runbook for the memoryCritical alert.

[zugao]

LGTM, I haven't tested it.