Expose "Reason" field to prometheus metric

[or-shachar]

(original discussion in slack)
Hey! We implemented alerts over gotk_reconcile_condition{type="Ready", status!="True"} for > 10m.
We noticed that when HelmRelease is pending on DependencyNotReady (because we explicitly configured dependsOn ) - the Status is False as well.

Status:
  Conditions:
    Last Transition Time:          2022-04-25T10:48:30Z
    Message:                       dependency 'default/calico' is not ready
    Reason:                        DependencyNotReady
    Status:                        False
    Type:                          Ready

If my dependencies take > 10m to finish - the alert would be fired.

Looking at the metric - I see that I have no way to filter out releases that are waiting for dependencies.

Consulting with @kingdonb - we thought it can be a good idea to add this attribute to the prometheus metric. I understand that there are finite number of reasons so it shouldn't increase cardinality by too much.

FYI: @seh (As I remember seeing you commenting about prometheus metrics ideas in the past)

[kingdonb]

I thought it would make sense to at least have this information exposed in the Prometheus metric, so you can have a different threshold (maybe 45m) for resources that wait a long time, for example because of long dependency chains in monorepos, which is a common pattern we have seen causing this issue.

Normally I want to see things reconciling within 10m so I would use the threshold that we have documented in the monitoring guide, same one you have mentioned. But if I know that I have these long dependency chains and as long as the condition reason is DependencyNotReady then I might prefer to have two different alerts, one for:

gotk_reconcile_condition{type="Ready", status!="True", reason!="DependencyNotReady"}

that alerts on > 10m

and another one

gotk_reconcile_condition{type="Ready", status!="True", reason="DependencyNotReady"}

that alerts on > 30m or something longer. But we cannot make the alerting varied in this way unless Reason is exposed in the metric, like status and type fields currently are exposed.

Are there any obstacles that would prevent us from implementing this change? @or-shachar suggested he might be able to implement this, I don't know enough to guide you through the whole PR, but if you're able to make some progress, it's good to have this issue posted first. Maybe we can convert it to an issue in the Kustomize Controller if the discussion around it seems in favor of this idea?

[kingdonb]

Hey, I remember that we had this discussion before, there hasn't been any progress on it.

Last time we discussed this, I remember that we considered the cardinality of Reason and said that it was probably of sufficiently low cardinality to be used in a metric. I don't think the discussion ever progressed any further than that.

Have you still been affected by this issue and do you have any interest in contributing a PR to expose the Reason field in the metric?

[kingdonb]

Please refer to:

• Support for flux custom metrics #4128

This has been solved, now you can create any metrics that you wish if the information is in the CRD spec or status, you can create a metric to report on it. This includes conditions and the ready field.

See also:

https://fluxcd.io/flux/monitoring/custom-metrics/