feat: PC-13724 Add custom thresholds for SHR (#530)

## Motivation Currently, System Health Review report is built using error budget remaining metric. Whether an SLO is marked as Red (Exhausted), Green (Healthy), or Yellow (At Risk) depends upon a static value. This PR opens up a window towards user-defined thresholds. If user decides that SLO should be marked as Healthy when it has more than 50% budget remaining, they will set `spec.systemHealthReview.thresholds.greenGt` field to `0.5`. If user wants to remove AtRisk threshold, they will set `spec.systemHealthReview.thresholds.redLte` to the same value as `greenGt` (`0.5`). Additionally, if users decides that NoData SLOs are supposed to be displayed on their report, a field `ShowNoData` is provided that can be set to true in such case. ## Summary `Thresholds` object with three fields (`GreenGreaterThan`, `RedLessThanOrEqualTo` and `ShowNoData`) was added to `SystemHealthReviewConfig` along with validation of said fields. ## Release Notes `Thresholds` object with three fields (`GreenGreaterThan`, `RedLessThanOrEqualTo` and `ShowNoData`) was added to `SystemHealthReviewConfig`. It allows users to define custom values for `Healthy` (`Green`), `AtRisk` (`Yellow`) and `Exhausted` (`Red`) thresholds and to decide whether to display or hide `NoData` SLOs on System Health Review report. It will be available once Nobl9 platform version 1.97.2 is released.
nobl9 · Sep 6, 2024 · 107fd6f · 107fd6f
1 parent 3af59f1
commit 107fd6f
Show file tree

Hide file tree

Showing 7 changed files with 182 additions and 1 deletion.
diff --git a/internal/manifest/v1alpha/examples/report.go b/internal/manifest/v1alpha/examples/report.go
@@ -41,6 +41,10 @@ func Report() []Example {
 								},
 							},
 						},
+						Thresholds: report.Thresholds{
+							RedLessThanOrEqual: ptr(0.8),
+							GreenGreaterThan:   ptr(0.95),
+						},
 					},
 				},
 			),

diff --git a/manifest/v1alpha/report/example_test.go b/manifest/v1alpha/report/example_test.go
@@ -78,6 +78,11 @@ func ExampleReport_systemHealthReview() {
 						},
 					},
 				},
+				Thresholds: report.Thresholds{
+					RedLessThanOrEqual: ptr(0.8),
+					GreenGreaterThan:   ptr(0.95),
+					ShowNoData:         false,
+				},
 			},
 		},
 	)
@@ -135,6 +140,10 @@ func ExampleReport_systemHealthReview() {
 	//       labels:
 	//         key3:
 	//         - value1
+	//     thresholds:
+	//       redLte: 0.8
+	//       greenGt: 0.95
+	//       showNoData: false
 }
 
 func ExampleReport_sloHistory() {

diff --git a/manifest/v1alpha/report/examples.yaml b/manifest/v1alpha/report/examples.yaml
@@ -25,6 +25,10 @@
           key2:
           - value2
           - value3
+      thresholds:
+        redLte: 0.8
+        greenGt: 0.95
+        showNoData: false
 - apiVersion: n9/v1alpha
   kind: Report
   metadata:

diff --git a/manifest/v1alpha/report/system_health_review.go b/manifest/v1alpha/report/system_health_review.go
@@ -6,6 +6,16 @@ type SystemHealthReviewConfig struct {
 	TimeFrame  SystemHealthReviewTimeFrame `json:"timeFrame" validate:"required"`
 	RowGroupBy RowGroupBy                  `json:"rowGroupBy" validate:"required" example:"project"`
 	Columns    []ColumnSpec                `json:"columns" validate:"min=1,max=30"`
+	Thresholds Thresholds                  `json:"thresholds" validate:"required"`
+}
+
+type Thresholds struct {
+	RedLessThanOrEqual *float64 `json:"redLte" validate:"required" example:"0.8"`
+	// Yellow is calculated as the difference between Red and Green
+	// thresholds. If Red and Green are the same, Yellow is not used on the report.
+	GreenGreaterThan *float64 `json:"greenGt" validate:"required" example:"0.95"`
+	// ShowNoData customizes the report to either show or hide rows with no data.
+	ShowNoData bool `json:"showNoData"`
 }
 
 type ColumnSpec struct {

diff --git a/manifest/v1alpha/report/validation_system_health_review.go b/manifest/v1alpha/report/validation_system_health_review.go
@@ -24,6 +24,10 @@ var systemHealthReviewValidation = govy.New[SystemHealthReviewConfig](
 		WithName("timeFrame").
 		Required().
 		Include(timeFrameValidation),
+	govy.For(func(s SystemHealthReviewConfig) Thresholds { return s.Thresholds }).
+		WithName("thresholds").
+		Required().
+		Include(reportThresholdsValidation),
 )
 
 var columnValidation = govy.New[ColumnSpec](
@@ -53,6 +57,31 @@ var timeFrameValidation = govy.New[SystemHealthReviewTimeFrame](
 		Include(snapshotTimeFrameLatestPointValidation),
 )
 
+var reportThresholdsValidation = govy.New[Thresholds](
+	govy.For(govy.GetSelf[Thresholds]()).
+		Rules(redLteValidation),
+	govy.ForPointer(func(s Thresholds) *float64 { return s.RedLessThanOrEqual }).
+		WithName("redLte").
+		Required().
+		Rules(rules.GTE(0.0), rules.LTE(1.0)),
+	govy.ForPointer(func(s Thresholds) *float64 { return s.GreenGreaterThan }).
+		WithName("greenGt").
+		Required().
+		Rules(rules.GTE(0.0), rules.LTE(1.0)),
+)
+
+var redLteValidation = govy.NewRule(func(v Thresholds) error {
+	if v.RedLessThanOrEqual != nil && v.GreenGreaterThan != nil {
+		if *v.RedLessThanOrEqual > *v.GreenGreaterThan {
+			return govy.NewPropertyError(
+				"redLte",
+				v.RedLessThanOrEqual,
+				errors.Errorf("must be less than or equal to 'greenGt' (%v)", *v.GreenGreaterThan))
+		}
+	}
+	return nil
+})
+
 var snapshotValidation = govy.New[SnapshotTimeFrame](
 	govy.For(func(s SnapshotTimeFrame) SnapshotPoint { return s.Point }).
 		WithName("point").

diff --git a/manifest/v1alpha/report/validation_test.go b/manifest/v1alpha/report/validation_test.go
@@ -107,7 +107,7 @@ func TestValidate_Spec(t *testing.T) {
 			},
 		}
 		err := validate(report)
-		testutils.AssertContainsErrors(t, report, err, 1, testutils.ExpectedError{
+		testutils.AssertContainsErrors(t, report, err, 2, testutils.ExpectedError{
 			Prop:    "spec",
 			Message: "exactly one report type configuration is required",
 		})
@@ -512,6 +512,10 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: properLabel},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with empty columns": {
@@ -531,6 +535,10 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) {
 				},
 				RowGroupBy: RowGroupByProject,
 				Columns:    []ColumnSpec{},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with too many columns": {
@@ -582,6 +590,10 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) {
 					{DisplayName: "Column 30", Labels: properLabel},
 					{DisplayName: "Column 31", Labels: properLabel},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with empty labels": {
@@ -603,6 +615,10 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with empty displayName": {
@@ -624,6 +640,60 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) {
 				Columns: []ColumnSpec{
 					{Labels: properLabel},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
+			},
+		},
+		"fails with empty thresholds": {
+			ExpectedErrorsCount: 1,
+			ExpectedErrors: []testutils.ExpectedError{
+				{
+					Prop: "spec.systemHealthReview.thresholds",
+					Code: rules.ErrorCodeRequired,
+				},
+			},
+			Config: SystemHealthReviewConfig{
+				TimeFrame: SystemHealthReviewTimeFrame{
+					Snapshot: SnapshotTimeFrame{
+						Point: SnapshotPointLatest,
+					},
+					TimeZone: "Europe/Warsaw",
+				},
+				RowGroupBy: RowGroupByProject,
+				Columns: []ColumnSpec{
+					{DisplayName: "Column 1", Labels: properLabel},
+				},
+			},
+		},
+		"fails with invalid thresholds": {
+			ExpectedErrorsCount: 2,
+			ExpectedErrors: []testutils.ExpectedError{
+				{
+					Prop: "spec.systemHealthReview.thresholds.redLte",
+					Code: rules.ErrorCodeGreaterThanOrEqualTo,
+				},
+				{
+					Prop: "spec.systemHealthReview.thresholds.greenGt",
+					Code: rules.ErrorCodeLessThanOrEqualTo,
+				},
+			},
+			Config: SystemHealthReviewConfig{
+				TimeFrame: SystemHealthReviewTimeFrame{
+					Snapshot: SnapshotTimeFrame{
+						Point: SnapshotPointLatest,
+					},
+					TimeZone: "Europe/Warsaw",
+				},
+				RowGroupBy: RowGroupByProject,
+				Columns: []ColumnSpec{
+					{DisplayName: "Column 1", Labels: properLabel},
+				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(-0.1),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(1.1),
+				},
 			},
 		},
 	} {
@@ -634,6 +704,32 @@ func TestValidate_Spec_SystemHealthReview(t *testing.T) {
 			testutils.AssertContainsErrors(t, report, err, test.ExpectedErrorsCount, test.ExpectedErrors...)
 		})
 	}
+
+	t.Run("fails when red is greater than green", func(t *testing.T) {
+		report := validReport()
+		report.Spec.SystemHealthReview = &SystemHealthReviewConfig{
+			TimeFrame: SystemHealthReviewTimeFrame{
+				Snapshot: SnapshotTimeFrame{
+					Point: SnapshotPointLatest,
+				},
+				TimeZone: "Europe/Warsaw",
+			},
+			RowGroupBy: RowGroupByProject,
+			Columns: []ColumnSpec{
+				{DisplayName: "Column 1", Labels: properLabel},
+			},
+			Thresholds: Thresholds{
+				RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.2),
+				GreenGreaterThan:   func(f float64) *float64 { return &f }(0.1),
+			},
+		}
+		err := validate(report)
+		testutils.AssertContainsErrors(t, report, err, 1, testutils.ExpectedError{
+			Prop:    "spec.systemHealthReview.thresholds.redLte",
+			Message: "must be less than or equal to 'greenGt' (0.1)",
+		},
+		)
+	})
 }
 
 func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
@@ -655,6 +751,10 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with empty point in snapshot": {
@@ -674,6 +774,10 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with empty data in past point snapshot": {
@@ -695,6 +799,10 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with wrong rrule format in past point snapshot": {
@@ -721,6 +829,10 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with invalid rrule in past point snapshot": {
@@ -747,6 +859,10 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 		"fails with forbidden fields provided in latest point snapshot": {
@@ -774,6 +890,10 @@ func TestValidate_Spec_SystemHealthReview_TimeFrame(t *testing.T) {
 				Columns: []ColumnSpec{
 					{DisplayName: "Column 1", Labels: map[LabelKey][]LabelValue{"key1": {"value1"}}},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.0),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.2),
+				},
 			},
 		},
 	} {
@@ -853,6 +973,11 @@ func validReport() Report {
 						},
 					},
 				},
+				Thresholds: Thresholds{
+					RedLessThanOrEqual: func(f float64) *float64 { return &f }(0.8),
+					GreenGreaterThan:   func(f float64) *float64 { return &f }(0.95),
+					ShowNoData:         true,
+				},
 			},
 		},
 	}

diff --git a/sdk/config_activity.png b/sdk/config_activity.png