Add metric to monitor common related objects

[dhaiducek]

This adds a common_related_objects metric, which is a Gauge Vector sliced by the related object and the policy. Each policy and related object pair has a gauge set to the total number of instances of that related object. For example, if two policies point to a related object, there would be two gauges for each policy/related object pair, each set to 2. Gauges set to 1 are ignored/cleaned up.

ref:

• https://github.com/stolostron/backlog/issues/25357

[dhaiducek]

I'm curious about the performance hit for this implementation and wondering whether there should be a flag to shut off this metric (or maybe all metrics...).

[dhaiducek]

Sorry for the churn here. CI is passing and this PR is ready for review.

[dhaiducek]

I also wound up adding in a flag, enable-metrics to disable this metric if desired.

[gparvin]

Only had a minor comment about a comment. Looks good.

[gparvin]

/hold

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhaiducek, gparvin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [dhaiducek,gparvin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[JustinKuli]

I'm slightly worried about the performance impact of all our metrics, not just this one. So I'm glad this is configurable.

From https://prometheus.io/docs/practices/naming/#labels :

CAUTION: Remember that every unique combination of key-value label pairs represents a new time series, which can dramatically increase the amount of data stored. Do not use labels to store dimensions with high cardinality (many different label values), such as user IDs, email addresses, or other unbounded sets of values.

Each of our metrics has a policy_name label of some kind, plus this new one and compare_objects* have basically an object_name label. Those are the kind of "unbounded sets of values" that are warned against... I don't have a better solution, especially for this exact feature, I'm just commenting that we need to be careful.

[dhaiducek]

I agree--this definitely felt like a "square peg in a round hole" kind of thing since Prometheus isn't really designed for the goals of the metric. (We almost need a separate controller that can handle this sort of object duplication, but that might be a one-off at this point.) Hopefully this metric wouldn't occur frequently, but I definitely wanted to be able to turn it off if it became unruly.

[dhaiducek]

@JustinKuli @gparvin Are we prepared to unhold this PR, particularly given that it can be disabled?

I think we might revisit this metric and ones like it in the future to see if we can generate them, potentially outside of Prometheus, in a manner more fitting to their intentions.

[JustinKuli]

I think it's acceptable.

Maybe a future improvement would be to put the metrics we're worried about at a separate endpoint, so they could still be generated and scraped by some processes, but not the default prometheus configuration (which I assume gets everything at /metrics). But I'd be surprised if there isn't a way in Prometheus to ignore certain metrics, so maybe that would be a better approach

[gparvin]

/unhold