Skip to content

feat: Add placement status metrics #1077

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

Closed
britaniar wants to merge 19 commits into from
Closed

feat: Add placement status metrics #1077

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
623f26d
add placement status metrics
britaniar Mar 12, 2025
8486603
revert controller_test.go
britaniar Apr 3, 2025
8a89ce8
address comments
britaniar Apr 3, 2025
0200684
try to reduce flakiness
britaniar Apr 3, 2025
c6a8490
only check for complete metric for last scenario
britaniar Apr 3, 2025
ed646ef
address comments
britaniar Apr 4, 2025
a6026c9
update test cases
britaniar Apr 4, 2025
222867e
revert changes
britaniar Apr 4, 2025
f337151
minor change
britaniar Apr 4, 2025
e89bb6e
address comments
britaniar Apr 7, 2025
82a9a65
update test w/ new metric names
britaniar Apr 7, 2025
c74a770
add report diff test and address comments
britaniar Apr 9, 2025
4ed96c6
use cmp.Diff to compare metric and expectedMetric labels
britaniar Apr 9, 2025
d446d00
fix lint errors
britaniar Apr 9, 2025
fef4bb6
update check function & tests
britaniar Apr 10, 2025
07ffce0
move metric comparison to common test utils
britaniar Apr 10, 2025
1cec5f4
fix test
britaniar Apr 10, 2025
9bbdf16
fix test
britaniar Apr 10, 2025
d3e408d
address comments
britaniar Apr 11, 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
2 changes: 2 additions & 0 deletions pkg/controllers/clusterresourceplacement/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,7 @@ func emitPlacementStatusMetric(crp *fleetv1beta1.ClusterResourcePlacement) {
if cond != nil {
status = string(cond.Status)
}
metrics.FleetPlacementStatusLastTimeStampSeconds.DeletePartialMatch(prometheus.Labels{"name": crp.Name, "generation": strconv.FormatInt(crp.Generation, 10), "conditionType": string(fleetv1beta1.ClusterResourcePlacementScheduledConditionType)})
metrics.FleetPlacementStatusLastTimeStampSeconds.WithLabelValues(crp.Name, strconv.FormatInt(crp.Generation, 10), string(fleetv1beta1.ClusterResourcePlacementScheduledConditionType), status).SetToCurrentTime()
return
}
Expand All @@ -1061,6 +1062,7 @@ func emitPlacementStatusMetric(crp *fleetv1beta1.ClusterResourcePlacement) {
if cond != nil {
status = string(cond.Status)
}
metrics.FleetPlacementStatusLastTimeStampSeconds.DeletePartialMatch(prometheus.Labels{"name": crp.Name, "generation": strconv.FormatInt(crp.Generation, 10), "conditionType": string(condType.ClusterResourcePlacementConditionType())})
metrics.FleetPlacementStatusLastTimeStampSeconds.WithLabelValues(crp.Name, strconv.FormatInt(crp.Generation, 10), string(condType.ClusterResourcePlacementConditionType()), status).SetToCurrentTime()
return
}
Expand Down
181 changes: 80 additions & 101 deletions pkg/controllers/clusterresourceplacement/controller_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package clusterresourceplacement

import (
"fmt"
"sort"
"strconv"
"time"

Expand Down Expand Up @@ -402,13 +403,9 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
checkPlacementCompleteMetric(customRegistry, testCRPName, false, 1)

By("Ensure placement status metric was emitted")
status := map[string][]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): {
"nil",
},
status := map[string]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): "nil",
}
checkPlacementStatusMetric(customRegistry, crp, status)
})
Expand Down Expand Up @@ -461,11 +458,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
checkPlacementCompleteMetric(customRegistry, testCRPName, false, 1)

By("Ensure placement status metric was emitted")
status := map[string][]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): {
string(corev1.ConditionUnknown),
string(corev1.ConditionFalse),
},
status := map[string]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): string(corev1.ConditionFalse),
}
checkPlacementStatusMetric(customRegistry, crp, status)
})
Expand Down Expand Up @@ -568,13 +562,9 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
checkPlacementCompleteMetric(customRegistry, testCRPName, false, 1)

By("Ensure placement status metric was emitted")
status := map[string][]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): {
string(corev1.ConditionUnknown),
},
status := map[string]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): string(corev1.ConditionUnknown),
}
checkPlacementStatusMetric(customRegistry, crp, status)

Expand Down Expand Up @@ -677,19 +667,10 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
checkPlacementCompleteMetric(customRegistry, testCRPName, false, 1)

By("Ensure placement status metric was emitted")
status = map[string][]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementOverriddenConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementWorkSynchronizedConditionType): {
string(corev1.ConditionUnknown),
},
status = map[string]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementWorkSynchronizedConditionType): string(corev1.ConditionUnknown),
}
checkPlacementStatusMetric(customRegistry, crp, status)
})
Expand Down Expand Up @@ -865,25 +846,11 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
checkPlacementCompleteMetric(customRegistry, testCRPName, true, 2)
Copy link
Contributor

@zhiying-lin zhiying-lin Apr 11, 2025

Choose a reason for hiding this comment

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

we also need to validate the crp status metrics

Copy link
Contributor Author

@britaniar britaniar Apr 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For a complete crp, the test is very flaky on figuring out what statuses will be emitted depending on how fast the conditions update since we don't have all the controllers working properly in the integration test. Originally, we wanted the placementstatus to record the last status for incomplete crp to see where they stopped to include in the alert.

Copy link
Contributor

@zhiying-lin zhiying-lin Apr 14, 2025

Choose a reason for hiding this comment

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

i see, could you please add a comment there to clarify why we don't validate the status metrics here.


By("Ensure placement status metric was emitted")
status := map[string][]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementOverriddenConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementWorkSynchronizedConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementAppliedConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementAvailableConditionType): {
string(corev1.ConditionUnknown),
},
status := map[string]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementWorkSynchronizedConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementAppliedConditionType): string(corev1.ConditionUnknown),
}
checkPlacementStatusMetric(customRegistry, crp, status)
})
Expand Down Expand Up @@ -1180,22 +1147,10 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
checkPlacementCompleteMetric(customRegistry, testCRPName, true, 2)

By("Ensure placement status metric was emitted")
status := map[string][]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementOverriddenConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementWorkSynchronizedConditionType): {
string(corev1.ConditionUnknown),
},
string(placementv1beta1.ClusterResourcePlacementDiffReportedConditionType): {
string(corev1.ConditionUnknown),
},
status := map[string]string{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): string(corev1.ConditionUnknown),
string(placementv1beta1.ClusterResourcePlacementDiffReportedConditionType): string(corev1.ConditionUnknown),
}
checkPlacementStatusMetric(customRegistry, crp, status)
})
Expand Down Expand Up @@ -1229,7 +1184,7 @@ func checkPlacementCompleteMetric(registry *prometheus.Registry, crpName string,
}
}

func checkPlacementStatusMetric(registry *prometheus.Registry, crp *placementv1beta1.ClusterResourcePlacement, statuses map[string][]string) {
func checkPlacementStatusMetric(registry *prometheus.Registry, crp *placementv1beta1.ClusterResourcePlacement, statuses map[string]string) {
metricFamilies, err := registry.Gather()
Expect(err).Should(Succeed())
var placementStatusMetrics []*prometheusclientmodel.Metric
Expand All @@ -1238,41 +1193,65 @@ func checkPlacementStatusMetric(registry *prometheus.Registry, crp *placementv1b
placementStatusMetrics = mf.GetMetric()
}
}
By(fmt.Sprint("placementStatusMetrics: %w ", placementStatusMetrics))
// Iterate over the labels and compare with expected values from statuses map
for _, emittedMetric := range placementStatusMetrics {
metricLabels := emittedMetric.GetLabel()

// Initialize variables for each metric iteration
var condType string
var expectedStatuses []string
var statusExists bool
// Sorting metrics based on the "condition_type" label
sortMetricsByConditionType(placementStatusMetrics)
By(fmt.Sprint("placementStatusMetrics: %w ", placementStatusMetrics))
// Check the number of emitted metrics
Expect(len(placementStatusMetrics)).Should(Equal(len(statuses)))

// Iterate over the labels and compare with expected values from conditionType
for _, label := range metricLabels {
switch label.GetName() {
case "generation":
Expect(label.GetValue()).Should(Equal(strconv.FormatInt(crp.Generation, 10)))
case "name":
Expect(label.GetValue()).Should(Equal(crp.Name))
case "conditionType":
condType = label.GetValue()
// Retrieve possible statuses from the map directly
expectedStatuses, statusExists = statuses[condType]
// Ensure that the conditionType exists in the map
Expect(statusExists).Should(BeTrue(), fmt.Sprintf("status for conditionType '%s' is missing in the conditionTypeStatuses map", condType))
case "status":
// Compare the status label with the expected statuses for this conditionType
statusFound := false
for _, expectedStatus := range expectedStatuses {
if label.GetValue() == expectedStatus {
statusFound = true
break
}
}
// Ensure the status is one of the expected values
Expect(statusFound).Should(BeTrue(), fmt.Sprintf("status '%s' for conditionType '%s' does not match any expected value", label.GetValue(), condType))
}
expectedPlacementStatusMetrics := make([]*prometheusclientmodel.Metric, 0, len(statuses))
for condType, status := range statuses {
metric := &prometheusclientmodel.Metric{
Label: []*prometheusclientmodel.LabelPair{
{
Name: ptr.To("conditionType"),
Value: ptr.To(condType),
},
{
Name: ptr.To("generation"),
Value: ptr.To(strconv.FormatInt(crp.Generation, 10)),
},
{
Name: ptr.To("name"),
Value: ptr.To(crp.Name),
},
{
Name: ptr.To("status"),
Value: ptr.To(status),
},
},
}
expectedPlacementStatusMetrics = append(expectedPlacementStatusMetrics, metric)
}
sortMetricsByConditionType(expectedPlacementStatusMetrics)
fmt.Println("\nExpectedMetrics:", expectedPlacementStatusMetrics)
// Use cmp.Diff to compare the slices of metrics
var previousTime float64
previousTime = 0
for i := range placementStatusMetrics {
diff := cmp.Diff(placementStatusMetrics[i].Label, expectedPlacementStatusMetrics[i].Label, cmpopts.IgnoreUnexported(prometheusclientmodel.LabelPair{}))
Expect(diff).Should(BeEmpty(), fmt.Sprintf("Metric mismatch: %s", diff))
Expect(placementStatusMetrics[i].GetGauge().GetValue()).ShouldNot(Equal(previousTime))
Expect(placementStatusMetrics[i].GetGauge().GetValue() > previousTime).Should(BeTrue())
previousTime = placementStatusMetrics[i].GetGauge().GetValue()
}
}

func sortMetricsByConditionType(metrics []*prometheusclientmodel.Metric) {
var conditionTypeOrder = map[string]int{
string(placementv1beta1.ClusterResourcePlacementScheduledConditionType): 0,
string(placementv1beta1.ClusterResourcePlacementRolloutStartedConditionType): 1,
string(placementv1beta1.ClusterResourcePlacementOverriddenConditionType): 2,
string(placementv1beta1.ClusterResourcePlacementWorkSynchronizedConditionType): 3,
string(placementv1beta1.ClusterResourcePlacementAppliedConditionType): 4,
string(placementv1beta1.ClusterResourcePlacementAvailableConditionType): 5,
string(placementv1beta1.ClusterResourcePlacementDiffReportedConditionType): 6,
}
sort.Slice(metrics, func(i, j int) bool {
// Get condition type values using the condition label
conditionTypeI := conditionTypeOrder[metrics[i].GetLabel()[0].GetValue()]
conditionTypeJ := conditionTypeOrder[metrics[j].GetLabel()[0].GetValue()]
return conditionTypeI < conditionTypeJ
})
}
Loading