From d274c084040b2def62c28ef9b19859d809a3ebef Mon Sep 17 00:00:00 2001 From: Martin Chodur Date: Thu, 7 Mar 2024 22:30:22 +0100 Subject: [PATCH] feat: make the sourceTenant of the validator a list of regexps instead of single regexp Signed-off-by: Martin Chodur --- CHANGELOG.md | 4 ++++ docs/validations.md | 16 ++++++++------ examples/human_readable.html | 5 +++-- examples/human_readable.md | 3 ++- examples/validation.yaml | 10 +++++---- pkg/validator/others.go | 38 ++++++++++++++++++++------------- pkg/validator/validator_test.go | 10 ++++----- 7 files changed, 53 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0efbfd..b77b69f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.11.0] - 2024-03-07 +- :warning: CHANGED: Params of the `hasSourceTenantsForMetrics` validator (again FACEPALM). Now the tenant can have multiple regexp matchers. + See its [docs](docs/validations.md#hassourcetenantsformetrics). + ## [2.10.0] - 2024-03-05 - Fixed error messages for the `hasSourceTenantsForMetrics` and `expressionDoesNotUseIrate` validators. diff --git a/docs/validations.md b/docs/validations.md index 2cbb3b1..c946043 100644 --- a/docs/validations.md +++ b/docs/validations.md @@ -319,12 +319,16 @@ Fails, if the rule uses metric, that matches the specified regular expression fo params: sourceTenants: : - regexp: # The regexp will be fully anchored (surrounded by ^...$) - description: # Optional, will be shown in the validator output human-readable description - # Example: - # k8s: - # regexp: "kube_.*|container_.*" - # description: "Kubernetes metrics from KSM and cAdvisor provided by the k8s infrastructure team" + - regexp: # The regexp will be fully anchored (surrounded by ^...$) + description: # Optional, will be shown in the validator output human-readable description + # Example: + # k8s: + # - regexp: "kube_.*|container_.*" + # description: "Metrics from KSM" + # - regexp: "container_.*" + # description: "Metrics from cAdvisor " + # - regexp: "node_.*" + # description: "Node exporter metrics provided by the k8s infrastructure team" ``` ## Alert validators diff --git a/examples/human_readable.html b/examples/human_readable.html index d91a5db..681f54b 100644 --- a/examples/human_readable.html +++ b/examples/human_readable.html @@ -39,8 +39,9 @@

check-prometheus-limitations

check-source-tenants
  • All rules rule group, the rule belongs to, has the required source_tenants configured, according to the mapping of metric names to tenants: -
    mysql: ^mysql_.*$ (MySQL metrics from the MySQL team) -
    k8s: ^container_.*|kube_.*$ (Kubernetes metrics from KSM and cAdvisor provided by the k8s infrastructure team)
    • +
    k8s: ^container_.*$ (Metrics from cAdvisor) +
    k8s: ^kube_.*$ (Metrics from KSM) +
    mysql: ^mysql_.*$ (MySQL metrics from the MySQL team)

check-metric-name

diff --git a/examples/human_readable.md b/examples/human_readable.md index 8d055f3..65b4542 100644 --- a/examples/human_readable.md +++ b/examples/human_readable.md @@ -28,8 +28,9 @@ Validation rules: check-source-tenants - All rules rule group, the rule belongs to, has the required `source_tenants` configured, according to the mapping of metric names to tenants: + `k8s`: `^container_.*$` (Metrics from cAdvisor) + `k8s`: `^kube_.*$` (Metrics from KSM) `mysql`: `^mysql_.*$` (MySQL metrics from the MySQL team) - `k8s`: `^container_.*|kube_.*$` (Kubernetes metrics from KSM and cAdvisor provided by the k8s infrastructure team) check-metric-name - Alert expression uses metric name in selectors diff --git a/examples/validation.yaml b/examples/validation.yaml index 61d6de2..6a97598 100644 --- a/examples/validation.yaml +++ b/examples/validation.yaml @@ -78,11 +78,13 @@ validationRules: params: sourceTenants: "k8s": - regexp: "container_.*|kube_.*" - description: "Kubernetes metrics from KSM and cAdvisor provided by the k8s infrastructure team" + - regexp: "container_.*" + description: "Metrics from cAdvisor" + - regexp: "kube_.*" + description: "Metrics from KSM" "mysql": - regexp: "mysql_.*" - description: "MySQL metrics from the MySQL team" + - regexp: "mysql_.*" + description: "MySQL metrics from the MySQL team" - name: check-metric-name scope: Alert diff --git a/pkg/validator/others.go b/pkg/validator/others.go index f65af8b..5b48f65 100644 --- a/pkg/validator/others.go +++ b/pkg/validator/others.go @@ -19,7 +19,7 @@ type SourceTenantMetrics struct { func newHasSourceTenantsForMetrics(paramsConfig yaml.Node) (Validator, error) { params := struct { - SourceTenants map[string]SourceTenantMetrics `yaml:"sourceTenants"` + SourceTenants map[string][]SourceTenantMetrics `yaml:"sourceTenants"` }{} if err := paramsConfig.Decode(¶ms); err != nil { return nil, err @@ -27,16 +27,20 @@ func newHasSourceTenantsForMetrics(paramsConfig yaml.Node) (Validator, error) { if params.SourceTenants == nil || len(params.SourceTenants) == 0 { return nil, fmt.Errorf("sourceTenants metrics mapping needs to be set") } - validator := hasSourceTenantsForMetrics{sourceTenants: map[string]tenantMetrics{}} + validator := hasSourceTenantsForMetrics{sourceTenants: map[string][]tenantMetrics{}} for tenant, metrics := range params.SourceTenants { - compiledRegexp, err := regexp.Compile("^" + metrics.Regexp + "$") - if err != nil { - return nil, fmt.Errorf("invalid metric name regexp: %s", metrics.Regexp) - } - validator.sourceTenants[tenant] = tenantMetrics{ - regexp: compiledRegexp, - description: metrics.Description, + m := make([]tenantMetrics, len(metrics)) + for i, metric := range metrics { + compiledRegexp, err := regexp.Compile("^" + metric.Regexp + "$") + if err != nil { + return nil, fmt.Errorf("invalid metric name regexp: %s", metric.Regexp) + } + m[i] = tenantMetrics{ + regexp: compiledRegexp, + description: metric.Description, + } } + validator.sourceTenants[tenant] = m } return &validator, nil } @@ -47,13 +51,15 @@ type tenantMetrics struct { } type hasSourceTenantsForMetrics struct { - sourceTenants map[string]tenantMetrics + sourceTenants map[string][]tenantMetrics } func (h hasSourceTenantsForMetrics) String() string { tenantStrings := []string{} - for tenant, metricsRegexp := range h.sourceTenants { - tenantStrings = append(tenantStrings, fmt.Sprintf("`%s`: `%s` (%s)", tenant, metricsRegexp.regexp.String(), metricsRegexp.description)) + for tenant, metrics := range h.sourceTenants { + for _, m := range metrics { + tenantStrings = append(tenantStrings, fmt.Sprintf("`%s`: `%s` (%s)", tenant, m.regexp.String(), m.description)) + } } return fmt.Sprintf("rule group, the rule belongs to, has the required `source_tenants` configured, according to the mapping of metric names to tenants: \n %s", strings.Join(tenantStrings, "\n ")) } @@ -66,9 +72,11 @@ func (h hasSourceTenantsForMetrics) Validate(group unmarshaler.RuleGroup, rule r return errs } for _, usedMetric := range usedMetrics { - for tenant, metricsRegexp := range h.sourceTenants { - if metricsRegexp.regexp.MatchString(usedMetric.Name) && !slices.Contains(group.SourceTenants, tenant) { - errs = append(errs, fmt.Errorf("rule uses metric `%s` of the tenant `%s` tenant, you should set the tenant in the groups source_tenants settings", tenant, usedMetric.Name)) + for tenant, metrics := range h.sourceTenants { + for _, metric := range metrics { + if metric.regexp.MatchString(usedMetric.Name) && !slices.Contains(group.SourceTenants, tenant) { + errs = append(errs, fmt.Errorf("rule uses metric `%s` of the tenant `%s` tenant, you should set the tenant in the groups source_tenants settings", tenant, usedMetric.Name)) + } } } } diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index 357c8aa..4e5c585 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -196,11 +196,11 @@ var testCases = []struct { {name: "regexpIsFullyAnchored", validator: expressionDoesNotUseMetrics{metricNameRegexps: []*regexp.Regexp{regexp.MustCompile(`^foo_bar$`)}}, rule: rulefmt.Rule{Expr: `foo_baz_baz{foo="bar"} and foo_bar`}, expectedErrors: 1}, // hasSourceTenantsForMetrics - {name: "emptyMapping", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]tenantMetrics{}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0}, - {name: "usesMetricWithSourceTenantAndGroupHasSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]tenantMetrics{"tenant1": {regexp: regexp.MustCompile(`^teanant1_metric$`)}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant1"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0}, - {name: "usesMetricWithSourceTenantAndGroupDoesNotHaveSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]tenantMetrics{"tenant1": {regexp: regexp.MustCompile(`^teanant1_metric$`)}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1}, - {name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenants", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]tenantMetrics{"tenant1": {regexp: regexp.MustCompile(`^teanant1_metric$`)}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant1"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0}, - {name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenantsAndOneIsMissing", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]tenantMetrics{"tenant1": {regexp: regexp.MustCompile(`^teanant1_metric$`)}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant3"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1}, + {name: "emptyMapping", validator: hasSourceTenantsForMetrics{sourceTenants: map[string][]tenantMetrics{}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0}, + {name: "usesMetricWithSourceTenantAndGroupHasSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string][]tenantMetrics{"tenant1": {{regexp: regexp.MustCompile(`^teanant1_metric$`)}}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant1"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0}, + {name: "usesMetricWithSourceTenantAndGroupDoesNotHaveSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string][]tenantMetrics{"tenant1": {{regexp: regexp.MustCompile(`^teanant1_metric$`)}}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1}, + {name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenants", validator: hasSourceTenantsForMetrics{sourceTenants: map[string][]tenantMetrics{"tenant1": {{regexp: regexp.MustCompile(`^teanant1_metric$`)}}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant1"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0}, + {name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenantsAndOneIsMissing", validator: hasSourceTenantsForMetrics{sourceTenants: map[string][]tenantMetrics{"tenant1": {{regexp: regexp.MustCompile(`^teanant1_metric$`)}}}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant3"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1}, // hasAllowedSourceTenants {name: "emptyAllowedSourceTenantsAndGroupSourceTenants", validator: hasAllowedSourceTenants{allowedSourceTenants: []string{}}, group: unmarshaler.RuleGroup{SourceTenants: []string{}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},