Skip to content

🐛 Remove cluster label #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 3 commits into from
Sep 15, 2025
Merged

🐛 Remove cluster label #142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02daddd
Remove cluster label
clyang82 Sep 12, 2025
525b638
rename cluster to consumer
clyang82 Sep 12, 2025
6700532
cannot share labels by agent and manager
clyang82 Sep 12, 2025
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
62 changes: 27 additions & 35 deletions pkg/cloudevents/generic/metrics_collector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const (
const (
metricsSourceLabel = "source"
metricsOriginalSourceLabel = "original_source"
metricsClusterLabel = "cluster"
metricsDataTypeLabel = "type"
metricsSubResourceLabel = "subresource"
metricsActionLabel = "action"
Expand All @@ -31,7 +30,6 @@ const noneOriginalSource = "none"
// cloudeventsReceivedMetricsLabels - Array of labels added to cloudevents received metrics:
var cloudeventsReceivedMetricsLabels = []string{
metricsSourceLabel, // source
metricsClusterLabel, // cluster
metricsDataTypeLabel, // data type, e.g. manifests, manifestbundles
metricsSubResourceLabel, // subresource, eg, spec or status
metricsActionLabel, // action, eg, create, update, delete, resync_request, resync_response
Expand All @@ -41,7 +39,6 @@ var cloudeventsReceivedMetricsLabels = []string{
var cloudeventsSentMetricsLabels = []string{
metricsSourceLabel, // source
metricsOriginalSourceLabel, // original source, if no, set to "none"
metricsClusterLabel, // cluster
metricsDataTypeLabel, // data type, e.g. manifests, manifestbundles
metricsSubResourceLabel, // subresource, eg, spec or status
metricsActionLabel, // action, eg, create, update, delete, resync_request, resync_response
Expand All @@ -50,7 +47,6 @@ var cloudeventsSentMetricsLabels = []string{
// cloudeventsResyncMetricsLabels - Array of labels added to cloudevents resync metrics:
var cloudeventsResyncMetricsLabels = []string{
metricsSourceLabel, // source
metricsClusterLabel, // cluster
metricsDataTypeLabel, // data type, e.g. manifests, manifestbundles
}

Comment on lines 67 to 73

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Agent-status resync metrics still carry “consumer” label; remove it to meet PR goal (agent has no cluster/consumer label)

Per PR objective, agent-produced metrics must drop cluster/consumer entirely. Split spec/status label sets so status (agent) has no consumer dimension.

Apply:

-// cloudeventsResyncMetricsLabels - Array of labels added to cloudevents resync metrics:
-var cloudeventsResyncMetricsLabels = []string{
-	metricsSourceLabel,   // source
-	metricsConsumerLabel, // consumer
-	metricsDataTypeLabel, // data type, e.g. manifests, manifestbundles
-}
+// resourceSpecResyncMetricsLabels: server-side (source) resync labels
+var resourceSpecResyncMetricsLabels = []string{
+	metricsSourceLabel,   // source
+	metricsConsumerLabel, // consumer
+	metricsDataTypeLabel, // data type
+}
+
+// resourceStatusResyncMetricsLabels: agent-side (client) resync labels (no consumer)
+var resourceStatusResyncMetricsLabels = []string{
+	metricsSourceLabel,   // source
+	metricsDataTypeLabel, // data type
+}
-	resourceSpecResyncDurationMetric.With(labels).Observe(duration.Seconds())
+	resourceSpecResyncDurationMetric.With(labels).Observe(duration.Seconds())
-	cloudeventsResyncMetricsLabels,
+	resourceSpecResyncMetricsLabels,
-	cloudeventsResyncMetricsLabels,
+	resourceStatusResyncMetricsLabels,
 func updateResourceStatusResyncDurationMetric(source, consumer, dataType string, startTime time.Time) {
 	labels := prometheus.Labels{
 		metricsSourceLabel:   source,
-		metricsConsumerLabel: consumer,
 		metricsDataTypeLabel: dataType,
 	}
 	duration := time.Since(startTime)
 	resourceStatusResyncDurationMetric.With(labels).Observe(duration.Seconds())
 }

Please also update the status-resync doc examples here to drop consumer=… accordingly.

Also applies to: 185-217, 323-331

🤖 Prompt for AI Agents 
In pkg/cloudevents/generic/metrics_collector.go around lines 67-73, the
cloudeventsResyncMetricsLabels array currently includes metricsConsumerLabel but
agent-produced status resync metrics must not have a consumer/cluster label;
remove metricsConsumerLabel from this status label set, create/ensure a separate
spec-resync label array that still includes consumer where appropriate, and
update the other affected ranges (lines ~185-217 and ~323-331) to use the
correct label set for status vs spec metrics (remove any consumer reference for
status metrics). Also update the status-resync documentation examples to drop
consumer=... accordingly.

Expand All @@ -77,10 +73,10 @@ const (

// The cloudevents received counter metric is a counter with a base metric name of 'received_total'
// and a help string of 'The total number of received CloudEvents.'
// For example, 2 CloudEvents received from source1 to agent on cluster1 with data type manifests, one for resource create,
// another for resource updatewould result in the following metrics:
// cloudevents_received_total{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",subresource="spec",action="create"} 1
// cloudevents_received_total{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",subresource="spec",action="update"} 1
// For example, 2 CloudEvents received from source1 to agent with data type manifests, one for resource create,
// another for resource update would result in the following metrics:
// cloudevents_received_total{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",subresource="spec",action="create"} 1
// cloudevents_received_total{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",subresource="spec",action="update"} 1
var cloudeventsReceivedCounterMetric = prometheus.NewCounterVec(
prometheus.CounterOpts{
Subsystem: cloudeventsMetricsSubsystem,
Expand All @@ -93,9 +89,9 @@ var cloudeventsReceivedCounterMetric = prometheus.NewCounterVec(
// The cloudevents sent counter metric is a counter with a base metric name of 'sent_total'
// and a help string of 'The total number of sent CloudEvents.'
// For example, 1 cloudevent sent from source1 with data type manifestbundles for resource spec create (original source is empty),
// and 2 CloudEvents sent from agent on cluster1 back to source1 for resource status update would result in the following metrics:
// cloudevents_sent_total{source="source1",original_source="none",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",subresource="spec",action="create"} 1
// cloudevents_sent_total{source="cluster1-work-agent",original_source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",subresource="status",action="update"} 2
// and 2 CloudEvents sent from agent back to source1 for resource status update would result in the following metrics:
// cloudevents_sent_total{source="source1",original_source="none",type="io.open-cluster-management.works.v1alpha1.manifestbundles",subresource="spec",action="create"} 1
// cloudevents_sent_total{source="cluster1-work-agent",original_source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",subresource="status",action="update"} 2
var cloudeventsSentCounterMetric = prometheus.NewCounterVec(
prometheus.CounterOpts{
Subsystem: cloudeventsMetricsSubsystem,
Expand All @@ -111,16 +107,16 @@ var cloudeventsSentCounterMetric = prometheus.NewCounterVec(
// 2. the total sum of all observed values, exposed as 'resource_spec_resync_duration_seconds_sum'
// 3. the count of events that have been observed, exposed as 'resource_spec_resync_duration_seconds_count' (identical to 'resource_spec_resync_duration_seconds_bucket{le="+Inf"}' above)
// For example, 2 resource spec resync for manifests type that have been observed, one taking 0.5s and the other taking 0.7s, would result in the following metrics:
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.1"} 0
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.2"} 0
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.5"} 1
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="1.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="2.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="10.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="30.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests",le="+Inf"} 2
// resource_spec_resync_duration_seconds_sum{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests"} 1.2
// resource_spec_resync_duration_seconds_count{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifests"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.1"} 0
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.2"} 0
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.5"} 1
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="1.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="2.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="10.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="30.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests",le="+Inf"} 2
// resource_spec_resync_duration_seconds_sum{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests"} 1.2
// resource_spec_resync_duration_seconds_count{source="source1",type="io.open-cluster-management.works.v1alpha1.manifests"} 2
var resourceSpecResyncDurationMetric = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Subsystem: resourcesMetricsSubsystem,
Expand All @@ -145,16 +141,16 @@ var resourceSpecResyncDurationMetric = prometheus.NewHistogramVec(
// 2. the total sum of all observed values, exposed as 'resource_status_resync_duration_seconds_sum'
// 3. the count of events that have been observed, exposed as 'resource_status_resync_duration_seconds_count' (identical to 'resource_status_resync_duration_seconds_bucket{le="+Inf"}' above)
// For example, 2 resource status resync for manifestbundles type that have been observed, one taking 0.5s and the other taking 1.1s, would result in the following metrics:
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.1"} 0
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.2"} 0
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.5"} 1
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="1.0"} 1
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="2.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="10.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="30.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="+Inf"} 2
// resource_status_resync_duration_seconds_sum{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles"} 1.6
// resource_status_resync_duration_seconds_count{source="source1",cluster="cluster1",type="io.open-cluster-management.works.v1alpha1.manifestbundles"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.1"} 0
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.2"} 0
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.5"} 1
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="1.0"} 1
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="2.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="10.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="30.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="+Inf"} 2
// resource_status_resync_duration_seconds_sum{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles"} 1.6
// resource_status_resync_duration_seconds_count{source="source1",type="io.open-cluster-management.works.v1alpha1.manifestbundles"} 2
var resourceStatusResyncDurationMetric = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Subsystem: resourcesMetricsSubsystem,
Expand Down Expand Up @@ -229,7 +225,6 @@ func ResetCloudEventsMetrics() {
func increaseCloudEventsReceivedCounter(source, cluster, dataType, subresource, action string) {
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsClusterLabel: cluster,
metricsDataTypeLabel: dataType,
metricsSubResourceLabel: subresource,
metricsActionLabel: action,
Expand All @@ -245,7 +240,6 @@ func increaseCloudEventsSentCounter(source, originalSource, cluster, dataType, s
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsOriginalSourceLabel: originalSource,
metricsClusterLabel: cluster,
metricsDataTypeLabel: dataType,
metricsSubResourceLabel: subresource,
metricsActionLabel: action,
Expand All @@ -257,7 +251,6 @@ func increaseCloudEventsSentCounter(source, originalSource, cluster, dataType, s
func updateResourceSpecResyncDurationMetric(source, cluster, dataType string, startTime time.Time) {
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsClusterLabel: cluster,
metricsDataTypeLabel: dataType,
}
duration := time.Since(startTime)
Expand All @@ -268,7 +261,6 @@ func updateResourceSpecResyncDurationMetric(source, cluster, dataType string, st
func updateResourceStatusResyncDurationMetric(source, cluster, dataType string, startTime time.Time) {
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsClusterLabel: cluster,
metricsDataTypeLabel: dataType,
}
duration := time.Since(startTime)
Expand Down
16 changes: 8 additions & 8 deletions pkg/cloudevents/generic/metrics_collector_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ func TestCloudEventsMetrics(t *testing.T) {
time.Sleep(time.Second)

// ensure metrics are updated
sentTotal := cloudeventsSentCounterMetric.WithLabelValues(c.sourceID, noneOriginalSource, c.clusterName, c.dataType.String(), string(c.subresource), string(c.action))
sentTotal := cloudeventsSentCounterMetric.WithLabelValues(c.sourceID, noneOriginalSource, c.dataType.String(), string(c.subresource), string(c.action))
require.Equal(t, len(c.resources), int(toFloat64Counter(sentTotal)))
receivedTotal := cloudeventsReceivedCounterMetric.WithLabelValues(c.sourceID, c.clusterName, c.dataType.String(), string(c.subresource), string(c.action))
receivedTotal := cloudeventsReceivedCounterMetric.WithLabelValues(c.sourceID, c.dataType.String(), string(c.subresource), string(c.action))
require.Equal(t, len(c.resources), int(toFloat64Counter(receivedTotal)))

cancel()
Expand Down Expand Up @@ -235,20 +235,20 @@ func TestResyncMetrics(t *testing.T) {
// receive resync request and publish associated resources
source.receive(ctx, evt)

receivedTotal := cloudeventsReceivedCounterMetric.WithLabelValues(c.clusterName, c.clusterName, c.dataType.String(), string(types.SubResourceSpec), string(types.ResyncRequestAction))
receivedTotal := cloudeventsReceivedCounterMetric.WithLabelValues(c.clusterName, c.dataType.String(), string(types.SubResourceSpec), string(types.ResyncRequestAction))
require.Equal(t, 1, int(toFloat64Counter(receivedTotal)))

// wait 1 seconds to respond to the spec resync request
time.Sleep(1 * time.Second)

// check spec resync duration metric as a histogram
h := resourceSpecResyncDurationMetric.WithLabelValues(c.sourceID, c.clusterName, c.dataType.String())
h := resourceSpecResyncDurationMetric.WithLabelValues(c.sourceID, c.dataType.String())
count, sum := toFloat64HistCountAndSum(h)
require.Equal(t, uint64(1), count)
require.Greater(t, sum, 0.0)
require.Less(t, sum, 1.0)

sentTotal := cloudeventsSentCounterMetric.WithLabelValues(c.sourceID, noneOriginalSource, c.clusterName, c.dataType.String(), string(types.SubResourceSpec), string(types.ResyncResponseAction))
sentTotal := cloudeventsSentCounterMetric.WithLabelValues(c.sourceID, noneOriginalSource, c.dataType.String(), string(types.SubResourceSpec), string(types.ResyncResponseAction))
require.Equal(t, len(c.resources), int(toFloat64Counter(sentTotal)))
}

Expand All @@ -274,20 +274,20 @@ func TestResyncMetrics(t *testing.T) {
// receive resync request and publish associated resources
agent.receive(ctx, evt)

receivedTotal := cloudeventsReceivedCounterMetric.WithLabelValues(c.sourceID, c.clusterName, c.dataType.String(), string(types.SubResourceStatus), string(types.ResyncRequestAction))
receivedTotal := cloudeventsReceivedCounterMetric.WithLabelValues(c.sourceID, c.dataType.String(), string(types.SubResourceStatus), string(types.ResyncRequestAction))
require.Equal(t, 1, int(toFloat64Counter(receivedTotal)))

// wait 1 seconds to respond to the resync request
time.Sleep(1 * time.Second)

// check status resync duration metric as a histogram
h := resourceStatusResyncDurationMetric.WithLabelValues(c.sourceID, c.clusterName, c.dataType.String())
h := resourceStatusResyncDurationMetric.WithLabelValues(c.sourceID, c.dataType.String())
count, sum := toFloat64HistCountAndSum(h)
require.Equal(t, uint64(1), count)
require.Greater(t, sum, 0.0)
require.Less(t, sum, 1.0)

sentTotal := cloudeventsSentCounterMetric.WithLabelValues(testAgentName, noneOriginalSource, c.clusterName, c.dataType.String(), string(types.SubResourceStatus), string(types.ResyncResponseAction))
sentTotal := cloudeventsSentCounterMetric.WithLabelValues(testAgentName, noneOriginalSource, c.dataType.String(), string(types.SubResourceStatus), string(types.ResyncResponseAction))
require.Equal(t, len(c.resources), int(toFloat64Counter(sentTotal)))
}

Expand Down
Loading