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 2 commits
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
6 changes: 3 additions & 3 deletions pkg/cloudevents/generic/agentclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (c *CloudEventAgentClient[T]) Resync(ctx context.Context, source string) er
return err
}

increaseCloudEventsSentCounter(evt.Source(), source, c.clusterName, c.codec.EventDataType().String(), string(eventType.SubResource), string(eventType.Action))
increaseCloudEventsSentCounter(evt.Source(), source, "", c.codec.EventDataType().String(), string(eventType.SubResource), string(eventType.Action))

return nil
}
Expand All @@ -132,7 +132,7 @@ func (c *CloudEventAgentClient[T]) Publish(ctx context.Context, eventType types.
}

originalSource, _ := cloudeventstypes.ToString(evt.Context.GetExtensions()[types.ExtensionOriginalSource])
increaseCloudEventsSentCounter(evt.Source(), originalSource, c.clusterName, eventType.CloudEventsDataType.String(), string(eventType.SubResource), string(eventType.Action))
increaseCloudEventsSentCounter(evt.Source(), originalSource, "", eventType.CloudEventsDataType.String(), string(eventType.SubResource), string(eventType.Action))

return nil
}
Expand All @@ -153,7 +153,7 @@ func (c *CloudEventAgentClient[T]) receive(ctx context.Context, evt cloudevents.
return
}

increaseCloudEventsReceivedCounter(evt.Source(), c.clusterName, eventType.CloudEventsDataType.String(), string(eventType.SubResource), string(eventType.Action))
increaseCloudEventsReceivedCounter(evt.Source(), "", eventType.CloudEventsDataType.String(), string(eventType.SubResource), string(eventType.Action))

if eventType.Action == types.ResyncRequestAction {
if eventType.SubResource != types.SubResourceStatus {
Expand Down
86 changes: 46 additions & 40 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,7 @@ const (
const (
metricsSourceLabel = "source"
metricsOriginalSourceLabel = "original_source"
metricsClusterLabel = "cluster"
metricsConsumerLabel = "consumer"
metricsDataTypeLabel = "type"
metricsSubResourceLabel = "subresource"
metricsActionLabel = "action"
Expand All @@ -31,7 +31,7 @@ const noneOriginalSource = "none"
// cloudeventsReceivedMetricsLabels - Array of labels added to cloudevents received metrics:
var cloudeventsReceivedMetricsLabels = []string{
metricsSourceLabel, // source
metricsClusterLabel, // cluster
metricsConsumerLabel, // consumer
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 +41,7 @@ var cloudeventsReceivedMetricsLabels = []string{
var cloudeventsSentMetricsLabels = []string{
metricsSourceLabel, // source
metricsOriginalSourceLabel, // original source, if no, set to "none"
metricsClusterLabel, // cluster
metricsConsumerLabel, // consumer
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 +50,7 @@ var cloudeventsSentMetricsLabels = []string{
// cloudeventsResyncMetricsLabels - Array of labels added to cloudevents resync metrics:
var cloudeventsResyncMetricsLabels = []string{
metricsSourceLabel, // source
metricsClusterLabel, // cluster
metricsConsumerLabel, // consumer
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 +77,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 consumer1 with data type manifests, one for resource create,
// another for resource update would result in the following metrics:
// cloudevents_received_total{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",subresource="spec",action="create"} 1
// cloudevents_received_total{source="source1",consumer="consumer1",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 +93,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 consumer1-work-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="consumer1-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 +111,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",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.1"} 0
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.2"} 0
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="0.5"} 1
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="1.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="2.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="10.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="30.0"} 2
// resource_spec_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests",le="+Inf"} 2
// resource_spec_resync_duration_seconds_sum{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests"} 1.2
// resource_spec_resync_duration_seconds_count{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifests"} 2
var resourceSpecResyncDurationMetric = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Subsystem: resourcesMetricsSubsystem,
Expand All @@ -145,16 +145,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",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.1"} 0
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.2"} 0
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="0.5"} 1
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="1.0"} 1
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="2.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="10.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="30.0"} 2
// resource_status_resync_duration_seconds_bucket{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles",le="+Inf"} 2
// resource_status_resync_duration_seconds_sum{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles"} 1.6
// resource_status_resync_duration_seconds_count{source="source1",consumer="consumer1",type="io.open-cluster-management.works.v1alpha1.manifestbundles"} 2
var resourceStatusResyncDurationMetric = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Subsystem: resourcesMetricsSubsystem,
Expand Down Expand Up @@ -225,50 +225,56 @@ func ResetCloudEventsMetrics() {
workProcessedCounterMetric.Reset()
}
Comment on lines +257 to 271

@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

Reset functions: move reconnect reset to client set

 func ResetSourceCloudEventsMetrics() {
 	cloudeventsReceivedBySourceCounterMetric.Reset()
 	cloudeventsSentFromSourceCounterMetric.Reset()
 	resourceSpecResyncDurationMetric.Reset()
-	clientReconnectedCounterMetric.Reset()
 }
 
 func ResetClientCloudEventsMetrics() {
 	cloudeventsReceivedByClientCounterMetric.Reset()
 	cloudeventsSentFromClientCounterMetric.Reset()
 	resourceStatusResyncDurationMetric.Reset()
+	clientReconnectedCounterMetric.Reset()
 	workProcessedCounterMetric.Reset()
 }
📝 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
// ResetSourceCloudEventsMetrics resets all collectors from source
func ResetSourceCloudEventsMetrics() {
cloudeventsReceivedBySourceCounterMetric.Reset()
cloudeventsSentFromSourceCounterMetric.Reset()
resourceSpecResyncDurationMetric.Reset()
resourceStatusResyncDurationMetric.Reset()
clientReconnectedCounterMetric.Reset()
}
// ResetClientCloudEventsMetrics resets all collectors from client
func ResetClientCloudEventsMetrics() {
cloudeventsReceivedByClientCounterMetric.Reset()
cloudeventsSentFromClientCounterMetric.Reset()
resourceStatusResyncDurationMetric.Reset()
workProcessedCounterMetric.Reset()
}
// ResetSourceCloudEventsMetrics resets all collectors from source
func ResetSourceCloudEventsMetrics() {
cloudeventsReceivedBySourceCounterMetric.Reset()
cloudeventsSentFromSourceCounterMetric.Reset()
resourceSpecResyncDurationMetric.Reset()
}
// ResetClientCloudEventsMetrics resets all collectors from client
func ResetClientCloudEventsMetrics() {
cloudeventsReceivedByClientCounterMetric.Reset()
cloudeventsSentFromClientCounterMetric.Reset()
resourceStatusResyncDurationMetric.Reset()
clientReconnectedCounterMetric.Reset()
workProcessedCounterMetric.Reset()
}
🤖 Prompt for AI Agents 
In pkg/cloudevents/generic/metrics_collector.go around lines 257 to 271, the
reconnect metric reset (clientReconnectedCounterMetric.Reset()) is currently
placed in ResetSourceCloudEventsMetrics but belongs to the client group; remove
that Reset() call from ResetSourceCloudEventsMetrics and add
clientReconnectedCounterMetric.Reset() into ResetClientCloudEventsMetrics so all
client-related collectors are reset together.


// increaseCloudEventsReceivedCounter increases the cloudevents sent counter metric:
func increaseCloudEventsReceivedCounter(source, cluster, dataType, subresource, action string) {
// increaseCloudEventsReceivedCounter increases the cloudevents received counter metric:
func increaseCloudEventsReceivedCounter(source, consumer, dataType, subresource, action string) {
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsClusterLabel: cluster,
metricsDataTypeLabel: dataType,
metricsSubResourceLabel: subresource,
metricsActionLabel: action,
}
// if it is called by the agent, the consumer is empty
if consumer != "" {
labels[metricsConsumerLabel] = consumer
}
cloudeventsReceivedCounterMetric.With(labels).Inc()
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// increaseCloudEventsSentCounter increases the cloudevents sent counter metric:
func increaseCloudEventsSentCounter(source, originalSource, cluster, dataType, subresource, action string) {
func increaseCloudEventsSentCounter(source, originalSource, consumer, dataType, subresource, action string) {
if originalSource == "" {
originalSource = noneOriginalSource
}
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsOriginalSourceLabel: originalSource,
metricsClusterLabel: cluster,
metricsDataTypeLabel: dataType,
metricsSubResourceLabel: subresource,
metricsActionLabel: action,
}
// if it is called by the agent, the consumer is empty
if consumer != "" {
labels[metricsConsumerLabel] = consumer
}
cloudeventsSentCounterMetric.With(labels).Inc()
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// updateResourceSpecResyncDurationMetric updates the resource spec resync duration metric:
func updateResourceSpecResyncDurationMetric(source, cluster, dataType string, startTime time.Time) {
func updateResourceSpecResyncDurationMetric(source, consumer, dataType string, startTime time.Time) {
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsClusterLabel: cluster,
metricsConsumerLabel: consumer,
metricsDataTypeLabel: dataType,
}
duration := time.Since(startTime)
resourceSpecResyncDurationMetric.With(labels).Observe(duration.Seconds())
}

// updateResourceStatusResyncDurationMetric updates the resource status resync duration metric:
func updateResourceStatusResyncDurationMetric(source, cluster, dataType string, startTime time.Time) {
func updateResourceStatusResyncDurationMetric(source, consumer, dataType string, startTime time.Time) {
labels := prometheus.Labels{
metricsSourceLabel: source,
metricsClusterLabel: cluster,
metricsConsumerLabel: consumer,
metricsDataTypeLabel: dataType,
}
duration := time.Since(startTime)
Expand Down
Loading
Loading