Skip to content

Commit

Permalink
feat: add aggregation field in KeptnMetric (#1780)
Browse files Browse the repository at this point in the history 
Co-authored-by: Florian Bacher <florian.bacher@dynatrace.com>
  • Loading branch information
@rakshitgondwal @bacherfl
rakshitgondwal and bacherfl committed Aug 3, 2023
1 parent f2d5bdd commit c0b66ea
Show file tree
Hide file tree
Showing 9 changed files with 261 additions and 41 deletions.
1 change: 1 addition & 0 deletions docs/content/en/docs/crd-ref/metrics/v1alpha3/_index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,6 @@ _Appears in:_
| --- | --- |
| `interval` _string_ | Interval specifies the duration of the time interval for the data query |
| `step` _string_ | Step represents the query resolution step width for the data query |
| `aggregation` _string_ | Aggregation defines as the type of aggregation function to be applied on the data. Accepted values: p90, p95, p99, max, min, avg, median |


3 changes: 3 additions & 0 deletions metrics-operator/api/v1alpha3/keptnmetric_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ type RangeSpec struct {
Interval string `json:"interval,omitempty"`
// Step represents the query resolution step width for the data query
Step string `json:"step,omitempty"`
// Aggregation defines as the type of aggregation function to be applied on the data. Accepted values: p90, p95, p99, max, min, avg, median
// +kubebuilder:validation:Enum:=p90;p95;p99;max;min;avg;median
Aggregation string `json:"aggregation,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
22 changes: 22 additions & 0 deletions metrics-operator/api/v1alpha3/keptnmetric_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ func (s *KeptnMetric) validateKeptnMetric() error {
if err = s.validateRangeStep(); err != nil {
allErrs = append(allErrs, err)
}
if err = s.validateAggregation(); err != nil {
allErrs = append(allErrs, err)
}
if len(allErrs) == 0 {
return nil
}
Expand Down Expand Up @@ -110,3 +113,22 @@ func (s *KeptnMetric) validateRangeStep() *field.Error {
}
return nil
}

func (s *KeptnMetric) validateAggregation() *field.Error {
if s.Spec.Range == nil {
return nil
}
if s.Spec.Range.Step != "" && s.Spec.Range.Aggregation == "" {
return field.Required(
field.NewPath("spec").Child("range").Child("aggregation"),
errors.New("Forbidden! Aggregation field is required if defining the step field").Error(),
)
}
if s.Spec.Range.Step == "" && s.Spec.Range.Aggregation != "" {
return field.Required(
field.NewPath("spec").Child("range").Child("step"),
errors.New("Forbidden! Step interval is required for the aggregation to work").Error(),
)
}
return nil
}
246 changes: 206 additions & 40 deletions metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,29 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) {
oldSpec runtime.Object
}{
{
name: "create-with-wrong-step",
name: "create-with-right-step-right-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
{
name: "create-with-wrong-step-right-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Aggregation: "p90",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"create-with-wrong-step",
"create-with-wrong-step-right-aggregation",
field.ErrorList{
field.Invalid(
field.NewPath("spec").Child("range").Child("step"),
Expand All @@ -173,115 +185,269 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) {
),
},
{
name: "create-with-empty-step",
name: "create-with-right-step-wrong-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "",
Interval: "5m",
Step: "1m",
Aggregation: "p91",
},
},
},
{
name: "create-with-right-step",
name: "create-with-wrong-step-wrong-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Aggregation: "p91",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"create-with-wrong-step-wrong-aggregation",
field.ErrorList{
field.Invalid(
field.NewPath("spec").Child("range").Child("step"),
"1mins",
"Forbidden! The time interval cannot be parsed. Please check for suitable conventions",
),
},
),
},
{
name: "create-with-empty-step-empty-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Interval: "5m",
Step: "",
Aggregation: "",
},
},
},
{
name: "create-with-wrong-interval-right-step",
verb: "update",
name: "create-with-step-empty-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5mins",
Step: "1m",
Interval: "5m",
Step: "1m",
Aggregation: "",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"create-with-wrong-interval-right-step",
"create-with-step-empty-aggregation",
field.ErrorList{
field.Invalid(
field.NewPath("spec").Child("range").Child("interval"),
"5mins",
"Forbidden! The time interval cannot be parsed. Please check for suitable conventions",
field.Required(
field.NewPath("spec").Child("range").Child("aggregation"),
"Forbidden! Aggregation field is required if defining the step field",
),
},
),
},
{
name: "create-with-wrong-interval-wrong-step",
name: "create-empty-step-with-aggregation",
verb: "create",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "",
Aggregation: "p90",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"create-empty-step-with-aggregation",
field.ErrorList{
field.Required(
field.NewPath("spec").Child("range").Child("step"),
"Forbidden! Step interval is required for the aggregation to work",
),
},
),
},
{
name: "update-with-right-step-right-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Aggregation: "p91",
},
},
},
},
{
name: "update-with-wrong-step-wrong-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5mins",
Step: "1mins",
Interval: "5m",
Step: "1mins",
Aggregation: "p91",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"create-with-wrong-interval-wrong-step",
"update-with-wrong-step-wrong-aggregation",
field.ErrorList{
field.Invalid(
field.NewPath("spec").Child("range").Child("interval"),
"5mins",
field.NewPath("spec").Child("range").Child("step"),
"1mins",
"Forbidden! The time interval cannot be parsed. Please check for suitable conventions",
),
},
),
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
},
{
name: "update-with-wrong-step-right-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Aggregation: "p90",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"update-with-wrong-step-right-aggregation",
field.ErrorList{
field.Invalid(
field.NewPath("spec").Child("range").Child("step"),
"1mins",
"Forbidden! The time interval cannot be parsed. Please check for suitable conventions",
),
},
),
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
},
{
name: "update-with-right-step-wrong-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "p91",
},
},
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
},
{
name: "update-with-right-step",
name: "update-with-empty-step-empty-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Interval: "5m",
Step: "",
Aggregation: "",
},
},
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
},
{
name: "update-with-wrong-step",
name: "update-with-empty-step-with-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1mins",
Interval: "5m",
Step: "",
Aggregation: "p90",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"update-with-wrong-step",
"update-with-empty-step-with-aggregation",
field.ErrorList{
field.Invalid(
field.Required(
field.NewPath("spec").Child("range").Child("step"),
"1mins",
"Forbidden! The time interval cannot be parsed. Please check for suitable conventions",
"Forbidden! Step interval is required for the aggregation to work",
),
},
),
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
},
{
name: "update-with-step-empty-aggregation",
verb: "update",
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Aggregation: "",
},
},
want: apierrors.NewInvalid(
schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"},
"update-with-step-empty-aggregation",
field.ErrorList{
field.Required(
field.NewPath("spec").Child("range").Child("aggregation"),
"Forbidden! Aggregation field is required if defining the step field",
),
},
),
oldSpec: &KeptnMetric{
Spec: KeptnMetricSpec{
Range: &RangeSpec{
Interval: "5m",
Step: "1m",
Interval: "5m",
Step: "1m",
Aggregation: "p90",
},
},
},
Expand All @@ -302,7 +468,7 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) {
} else {
s = &KeptnMetric{
ObjectMeta: metav1.ObjectMeta{Name: tt.name},
Spec: KeptnMetricSpec{Range: &RangeSpec{Interval: tt.Spec.Range.Interval, Step: tt.Spec.Range.Step}},
Spec: KeptnMetricSpec{Range: &RangeSpec{Interval: tt.Spec.Range.Interval, Step: tt.Spec.Range.Step, Aggregation: tt.Spec.Range.Aggregation}},
}
}
var err error
Expand Down
Loading

0 comments on commit c0b66ea

Please sign in to comment.