refactor: config for job aggregation strings

[darrenjaneczek]

What this PR does:
(Updated based on suggestions below)

• to make it easier to override, define "cluster_namespace_job" and "cluster, namespace, job" in terms of a new list in $._config: job_labels: ['cluster', 'namespace', 'job']
  • This replaces many hard-coded uses of these labels
• for consistency, based on feedback, we now follow a similar pattern for clusters: cluster_labels: ['cluster', 'namespace']
• the resulting string values are defined using a new groups.libsonnet file as $._group_config, which gets mixed in where it is needed.
• a hard-coded group-by used namespace hard-coded in the query -- it has been updated to use the composition from $._cluster_labels.
• deprecated _config.alert_aggregation_labels, since it is now a function of $._cluster_labels
  • there is a warning message if this old value is still being directly overridden; it still works (for now)
  • encouraged approach is to override the cluster_labels list instead, which is what will happen if alert_aggregation_labels is not overridden.

The resulting output does not change (unless any of the new lists in config are overridden), except in exactly one case where it was recommended to replace a hard-coded reference from namespace to cluster, namespace, composed from $._config.cluster_labels.

Checklist

• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[darrenjaneczek]

I am hoping for some feedback on the variable names, to ensure that they are appropriate.

[56quarters]

LGTM, but I'd wait for someone from the Cortex team

[pracucci]

Thanks for working on this! I left few comments 🙏

[darrenjaneczek]

@pracucci , with the [cluster, namespace] list (cluster_labels) defining which labels identify a cluster, does it make sense to seek out references to cluster_namespace* string patterns and replace them with the corresponding prefix (group_prefix_clusters)?

[pracucci]

LGTM (modulo a comment about alert_aggregation_labels_override). Please also add a CHANGELOG entry to explain the new config options.

[pracucci]

Good job here! I'm just wondering if we actually need this complexity. What we typically do is to introduce a breaking change (eg. stop using alert_aggregation_labels) and document it in the CHANGELOG. This mixin policy doesn't guarantee backward compatibility.

[darrenjaneczek]

I was concerned that my change could cause a phantom failure for someone, where their custom override would just stop working without any warning/error.

A plain old "error if alert_aggregation_labels is defined" would be less complex and solve my concern.

Let me know if/how you want this addressed in this PR.

[pracucci]

Let's not block on this. What you did looks correct, I'm just a bit worried about its maintenance, but not a blocker.

[pracucci]

@pracucci , with the [cluster, namespace] list (cluster_labels) defining which labels identify a cluster, does it make sense to seek out references to cluster_namespace* string patterns and replace them with the corresponding prefix (group_prefix_clusters)?

I would start as smallest as possible because the potential replacement is huge. If this PR solves your use case, I would start from here and work on it on incremental PRs.