Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions helm/bundles/cortex-nova/templates/kpis_kvm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,23 @@ spec:
- name: nova-flavors
- name: identity-projects
- name: identity-domains
---
apiVersion: cortex.cloud/v1alpha1
kind: KPI
metadata:
name: multicluster-object-count
spec:
schedulingDomain: nova
impl: multicluster_object_count_kpi
opts:
gvks:
- cortex.cloud/v1alpha1/HistoryList
- cortex.cloud/v1alpha1/ReservationList
- cortex.cloud/v1alpha1/CommittedResourceList
- kvm.cloud.sap/v1/HypervisorList
description: |
This KPI reports the number of objects of each configured GVK per remote
cluster, labelled by the configured cluster labels. It uses
PartialObjectMetadataList so only object metadata is fetched, making it
efficient even for large object counts.
{{- end }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
// Copyright SAP SE
// SPDX-License-Identifier: Apache-2.0

package deployment

import (
"context"
"fmt"
"log/slog"
"strings"
"unicode"

"github.com/cobaltcore-dev/cortex/internal/knowledge/db"
"github.com/cobaltcore-dev/cortex/internal/knowledge/kpis/plugins"
"github.com/cobaltcore-dev/cortex/pkg/conf"
"github.com/cobaltcore-dev/cortex/pkg/multicluster"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// MulticlusterObjectCountKPIOpts configures which GVKs to count per cluster.
// Each entry is a "group/version/Kind" string, e.g.
// "cortex.cloud/v1alpha1/HypervisorList".
type MulticlusterObjectCountKPIOpts struct {
GVKs []string `json:"gvks"`
}

type gvkDesc struct {
gvk schema.GroupVersionKind
labelKeys []string // snake_cased routing label keys (stable order)
desc *prometheus.Desc
}

// multiclusterReader is the subset of *multicluster.Client this KPI needs.
// Keeping it narrow lets the KPI logic be unit-tested without wiring real or
// fake clusters into a multicluster.Client.
type multiclusterReader interface {
ConfiguredRouteLabels(gvk schema.GroupVersionKind) []map[string]string
ListMetadataPerCluster(ctx context.Context, gvk schema.GroupVersionKind, opts ...client.ListOption) ([]multicluster.ClusterObjectMetadata, error)
}

// MulticlusterObjectCountKPI reports the number of objects of each configured
// GVK per remote cluster, labelled by the cluster's routing labels.
Comment on lines +43 to +44

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document configured home and remote clusters consistently. Collect emits home-cluster metrics with is_home=true, so both descriptions should include that behavior.

  • internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go#L43-L44: describe counts for configured home and remote clusters.
  • helm/bundles/cortex-nova/templates/kpis_kvm.yaml#L59-L61: state that home-cluster metrics are emitted with is_home=true and empty routing-label values.
📍 Affects 2 files
  • internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go#L43-L44 (this comment)
  • helm/bundles/cortex-nova/templates/kpis_kvm.yaml#L59-L61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go`
around lines 43 - 44, Update the MulticlusterObjectCountKPI description in
internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go
lines 43-44 to mention counts for configured home and remote clusters. Update
the corresponding description in
helm/bundles/cortex-nova/templates/kpis_kvm.yaml lines 59-61 to state that
home-cluster metrics use is_home=true with empty routing-label values.

type MulticlusterObjectCountKPI struct {
plugins.BaseKPI[MulticlusterObjectCountKPIOpts]
mcl multiclusterReader
descs []gvkDesc
}

func (MulticlusterObjectCountKPI) GetName() string { return "multicluster_object_count_kpi" }

func (k *MulticlusterObjectCountKPI) Init(_ *db.DB, c client.Client, opts conf.RawOpts) error {
if err := k.BaseKPI.Init(nil, c, opts); err != nil {
return err
}
mcl, ok := c.(*multicluster.Client)
if !ok {
return fmt.Errorf("multicluster_object_count_kpi requires a *multicluster.Client, got %T", c)
}
k.mcl = mcl

for _, raw := range k.Options.GVKs {
gvk, err := parseGVK(raw)
if err != nil {
return fmt.Errorf("invalid GVK %q: %w", raw, err)
}
routeLabels := mcl.ConfiguredRouteLabels(gvk)
// Each remote cluster is registered with a routing label map (e.g.
// {"availabilityZone": "eu-de-1a"}). We collect the union of all key names
// across every remote cluster and snake_case them to produce Prometheus label
// names (availabilityZone → availability_zone). These become variable labels
// on the descriptor so each cluster gets its own time series.
keySet := map[string]bool{}
var labelKeys []string
for _, lm := range routeLabels {
for k := range lm {
snake := toSnakeCase(k)
if !keySet[snake] {
keySet[snake] = true
labelKeys = append(labelKeys, snake)
}
}
}
// Fixed labels: group/version/kind identify the resource type; is_home
// distinguishes the home cluster (no routing labels) from remote clusters.
// The routing label keys follow (e.g. availability_zone).
varLabels := append([]string{"group", "version", "kind", "is_home"}, labelKeys...)
desc := prometheus.NewDesc(
"cortex_multicluster_object_count",
"Number of objects of a given GVK per cluster",
varLabels,
nil,
)
k.descs = append(k.descs, gvkDesc{gvk: gvk, labelKeys: labelKeys, desc: desc})
Comment on lines +68 to +95

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For github.com/prometheus/client_golang v1.23.2, does prometheus.Registry.Register reject descriptors with the same fully qualified metric name but different variable-label names?

💡 Result:

Yes, in prometheus/client_golang v1.23.2, the prometheus.Registry.Register method does reject descriptors that share the same fully-qualified metric name but have different variable-label names (or different help strings) [1]. The Prometheus registry enforces consistency for all metrics sharing the same fully-qualified name [1][2]. Specifically, when registering a collector, the registry performs internal consistency checks: 1. It checks if the metric name has been seen before [1]. 2. If it has, it compares the current descriptor's dimension hash (which represents the set of label names and help string) against the previously registered descriptor's dimension hash [1]. 3. If the dimension hashes do not match, the registry returns an error stating: "a previously registered descriptor with the same fully-qualified name as %s has different label names or a different help string" [1]. This design ensures that all metrics under a single name maintain a consistent structure, which is a fundamental requirement of the Prometheus data model [3][2]. While recent versions of the client library have refined these checks—for instance, by allowing inconsistent label dimensions in specific scenarios or by detecting collisions with "magic" names more effectively—the core requirement for structural consistency (same help string and same label names) for metrics sharing the same fully-qualified name remains enforced [1][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate target file =="
git ls-files | rg 'internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi\.go$' || true

echo "== outline target file =="
ast-grep outline internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go --view expanded || true

echo "== relevant file sections =="
sed -n '1,180p' internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go | nl

echo "== search metric name and route labels usages =="
rg -n "cortex_multicluster_object_count|ConfiguredRouteLabels|labelKeys|ToMetrics|Collect|Describe" internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go internal/knowledge/kpis/plugins/deployment || true

echo "== module prometheus dependency =="
sed -n '/github.com\/prometheus\/client_golang/p' go.mod go.sum | head -20 || true

Repository: cobaltcore-dev/cortex

Length of output: 1569


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant file contents with line numbers =="
awk '{printf "%5d\t%s\n", NR, $0}' internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go

echo "== metric name and label schema usage =="
rg -n "cortex_multicluster_object_count|ConfiguredRouteLabels|labelKeys|ToMetrics|Collect|Describe|register" internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go internal/knowledge/kpis/plugins/deployment || true

echo "== module prometheus dependency =="
if [ -f go.mod ]; then sed -n '/github.com\/prometheus\/client_golang/p' go.mod; fi
if [ -f go.sum ]; then rg 'github.com/prometheus/client_golang ' go.sum | head -20; fi

echo "== repository files under deployment plugins =="
git ls-files 'internal/knowledge/kpis/plugins/deployment/*' | sort

Repository: cobaltcore-dev/cortex

Length of output: 17977


Use one label schema for the metric family.

labelKeys is computed per GVK, but every descriptor uses cortex_multicluster_object_count. If configured GVKs expose different routing-label keys, Prometheus rejects the collector because one metric name ends up with incompatible variable labels. Build one stable union of routing-label keys across all configured GVKs, emit empty values where a cluster lacks a key, and add a registry test covering two differing label sets.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go`
around lines 68 - 95, The metric family currently creates descriptors with
different routing-label schemas per GVK. Refactor descriptor construction to
first compute one stable union of snake_case routing-label keys across all
configured GVKs, then use that shared schema for every
cortex_multicluster_object_count descriptor and emit empty values when a cluster
lacks a key. Add a registry test covering two GVKs with differing routing-label
sets and verifying collection succeeds.

}
return nil
}

func (k *MulticlusterObjectCountKPI) Describe(ch chan<- *prometheus.Desc) {
for _, d := range k.descs {
ch <- d.desc
}
}

func (k *MulticlusterObjectCountKPI) Collect(ch chan<- prometheus.Metric) {
ctx := context.Background()
for _, d := range k.descs {
perCluster, err := k.mcl.ListMetadataPerCluster(ctx, d.gvk)
if err != nil {
slog.Error("multicluster_object_count_kpi: failed to list object metadata",
"gvk", d.gvk, "err", err)
continue
}
for _, c := range perCluster {
isHome := "false"
if c.IsHome {
isHome = "true"
}
// Label values must match the descriptor order: group, version, kind,
// is_home, then one value per routing label key. For remote clusters the
// routing label value comes from the cluster's registration labels (e.g.
// Labels["availabilityZone"] → label value for "availability_zone"). The
// home cluster has no routing labels, so those positions are empty strings.
labelVals := make([]string, 0, 4+len(d.labelKeys))
labelVals = append(labelVals, d.gvk.Group, d.gvk.Version, d.gvk.Kind, isHome)
for _, key := range d.labelKeys {
labelVals = append(labelVals, labelValueForSnakeKey(key, c.Labels))
}
ch <- prometheus.MustNewConstMetric(d.desc, prometheus.GaugeValue,
float64(len(c.Items)), labelVals...)
}
}
}

// parseGVK parses a "group/version/Kind" string.
func parseGVK(s string) (schema.GroupVersionKind, error) {
parts := strings.SplitN(s, "/", 3)
if len(parts) != 3 {
return schema.GroupVersionKind{}, fmt.Errorf("expected group/version/Kind, got: %s", s)
}
return schema.GroupVersionKind{Group: parts[0], Version: parts[1], Kind: parts[2]}, nil
Comment on lines +138 to +142

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject empty GVK segments.

apps//DeploymentList and /v1/ pass the slash-count check, then fail later during collection. Require non-empty version and kind during Init.

Proposed fix
- if len(parts) != 3 {
+ if len(parts) != 3 || parts[1] == "" || parts[2] == "" {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
parts := strings.SplitN(s, "/", 3)
if len(parts) != 3 {
return schema.GroupVersionKind{}, fmt.Errorf("expected group/version/Kind, got: %s", s)
}
return schema.GroupVersionKind{Group: parts[0], Version: parts[1], Kind: parts[2]}, nil
parts := strings.SplitN(s, "/", 3)
if len(parts) != 3 || parts[1] == "" || parts[2] == "" {
return schema.GroupVersionKind{}, fmt.Errorf("expected group/version/Kind, got: %s", s)
}
return schema.GroupVersionKind{Group: parts[0], Version: parts[1], Kind: parts[2]}, nil
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/knowledge/kpis/plugins/deployment/multicluster_object_count_kpi.go`
around lines 138 - 142, Update the GVK parsing logic around strings.SplitN to
reject empty version or kind segments during Init, while preserving the existing
group/version/kind format validation and error behavior. Validate parts[1] and
parts[2] before constructing schema.GroupVersionKind so inputs such as
apps//DeploymentList and /v1/ fail immediately.

}

// toSnakeCase converts camelCase to snake_case (e.g. availabilityZone → availability_zone).
func toSnakeCase(s string) string {
var b strings.Builder
for i, r := range s {
if unicode.IsUpper(r) && i > 0 {
b.WriteByte('_')
}
b.WriteRune(unicode.ToLower(r))
}
return b.String()
}

// labelValueForSnakeKey finds the value in labels whose key, when snake_cased,
// matches snakeKey. Returns empty string for nil labels or no match (home cluster).
func labelValueForSnakeKey(snakeKey string, labels map[string]string) string {
for k, v := range labels {
if toSnakeCase(k) == snakeKey {
return v
}
}
return ""
}
Loading