From 6d2a555bbc9819c69fc30b9e4f34189d3e1af46a Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Thu, 3 Nov 2022 23:07:09 +0530 Subject: [PATCH 1/5] Allow `labelFromKey` field Allow `labelFromKey` field for the following types: * Gauge: Done. * Info: Done. * StateSet: N/A (redundant use case, see doc changes for more info). Signed-off-by: Pranshu Srivastava --- docs/customresourcestate-metrics.md | 1 + .../config_metrics_types.go | 2 + pkg/customresourcestate/registry_factory.go | 81 ++++++++++++------- .../registry_factory_test.go | 17 +++- 4 files changed, 67 insertions(+), 34 deletions(-) diff --git a/docs/customresourcestate-metrics.md b/docs/customresourcestate-metrics.md index 4e0f5e5e25..5296091ebc 100644 --- a/docs/customresourcestate-metrics.md +++ b/docs/customresourcestate-metrics.md @@ -146,6 +146,7 @@ spec: path: [status, sub] # if path targets an object, the object key will be used as label value + # This is not supported for StateSet type as all values will be truthy, which is redundant. labelFromKey: type # label values can be resolved specific to this path labelsFromPath: diff --git a/pkg/customresourcestate/config_metrics_types.go b/pkg/customresourcestate/config_metrics_types.go index 110c4f9934..6e8e9167cd 100644 --- a/pkg/customresourcestate/config_metrics_types.go +++ b/pkg/customresourcestate/config_metrics_types.go @@ -51,6 +51,8 @@ type MetricGauge struct { // Ref: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#info type MetricInfo struct { MetricMeta `yaml:",inline" json:",inline"` + // LabelFromKey adds a label with the given name if Path is an object. The label value will be the object key. + LabelFromKey string `yaml:"labelFromKey" json:"labelFromKey"` } // MetricStateSet is a metric which represent a series of related boolean values, also called a bitset. diff --git a/pkg/customresourcestate/registry_factory.go b/pkg/customresourcestate/registry_factory.go index 5c78294bd9..3cc575504d 100644 --- a/pkg/customresourcestate/registry_factory.go +++ b/pkg/customresourcestate/registry_factory.go @@ -157,6 +157,7 @@ func newCompiledMetric(m Metric) (compiledMetric, error) { compiledCommon: *cc, ValueFrom: valueFromPath, NilIsZero: m.Gauge.NilIsZero, + labelFromKey: m.Gauge.LabelFromKey, }, nil case MetricTypeInfo: if m.Info == nil { @@ -168,6 +169,7 @@ func newCompiledMetric(m Metric) (compiledMetric, error) { } return &compiledInfo{ compiledCommon: *cc, + labelFromKey: m.Info.LabelFromKey, }, nil case MetricTypeStateSet: if m.StateSet == nil { @@ -195,23 +197,8 @@ func newCompiledMetric(m Metric) (compiledMetric, error) { type compiledGauge struct { compiledCommon ValueFrom valuePath - LabelFromKey string NilIsZero bool -} - -func newCompiledGauge(m *MetricGauge) (*compiledGauge, error) { - cc, err := compileCommon(m.MetricMeta) - if err != nil { - return nil, fmt.Errorf("compile common: %w", err) - } - valueFromPath, err := compilePath(m.ValueFrom) - if err != nil { - return nil, fmt.Errorf("compile path ValueFrom: %w", err) - } - return &compiledGauge{ - compiledCommon: *cc, - ValueFrom: valueFromPath, - }, nil + labelFromKey string } func (c *compiledGauge) Values(v interface{}) (result []eachValue, errs []error) { @@ -227,8 +214,8 @@ func (c *compiledGauge) Values(v interface{}) (result []eachValue, errs []error) onError(fmt.Errorf("[%s]: %w", key, err)) continue } - if key != "" && c.LabelFromKey != "" { - ev.Labels[c.LabelFromKey] = key + if key != "" && c.labelFromKey != "" { + ev.Labels[c.labelFromKey] = key } addPathLabels(it, c.LabelFromPath(), ev.Labels) result = append(result, *ev) @@ -257,22 +244,54 @@ func (c *compiledGauge) Values(v interface{}) (result []eachValue, errs []error) type compiledInfo struct { compiledCommon + labelFromKey string } func (c *compiledInfo) Values(v interface{}) (result []eachValue, errs []error) { - if vs, isArray := v.([]interface{}); isArray { - for _, obj := range vs { + onError := func(err ...error) { + errs = append(errs, fmt.Errorf("%s: %v", c.Path(), err)) + } + + switch iter := v.(type) { + case []interface{}: + for _, obj := range iter { ev, err := c.values(obj) if len(err) > 0 { - errs = append(errs, err...) + onError(err...) continue } result = append(result, ev...) } - return + default: + value, err := c.values(v) + if err != nil { + onError(err...) + break + } + // labelFromKey logic + if vv, ok := v.(map[string]interface{}); ok { + for key, val := range vv { + if key != "" && c.labelFromKey != "" { + n, err := toFloat64(val, false) + if err != nil { + onError(err) + continue + } + result = append(result, eachValue{ + Labels: map[string]string{ + c.labelFromKey: key, + }, + Value: n, + }) + } + } + } + if len(result) == 0 { + result = value + } } - return c.values(v) + return } func (c *compiledInfo) values(v interface{}) (result []eachValue, err []error) { @@ -355,7 +374,7 @@ func less(a, b map[string]string) bool { func (c compiledGauge) value(it interface{}) (*eachValue, error) { labels := make(map[string]string) - value, err := getNum(c.ValueFrom.Get(it), c.NilIsZero) + value, err := toFloat64(c.ValueFrom.Get(it), c.NilIsZero) if err != nil { return nil, fmt.Errorf("%s: %w", c.ValueFrom, err) } @@ -478,7 +497,7 @@ func compilePath(path []string) (out valuePath, _ error) { return nil, fmt.Errorf("invalid list lookup: %s", part) } key, val := eq[0], eq[1] - num, notNum := getNum(val, false) + num, notNum := toFloat64(val, false) boolVal, notBool := strconv.ParseBool(val) out = append(out, pathOp{ part: part, @@ -496,7 +515,7 @@ func compilePath(path []string) (out valuePath, _ error) { } if notNum == nil { - if i, err := getNum(candidate, false); err == nil && num == i { + if i, err := toFloat64(candidate, false); err == nil && num == i { return m } } @@ -522,13 +541,14 @@ func compilePath(path []string) (out valuePath, _ error) { } else if s, ok := m.([]interface{}); ok { i, err := strconv.Atoi(part) if err != nil { - return nil + return fmt.Errorf("invalid list index: %s", part) } if i < 0 { + // negative index i += len(s) } if !(0 <= i && i < len(s)) { - return nil + return fmt.Errorf("list index out of range: %s", part) } return s[i] } @@ -544,6 +564,7 @@ func famGen(f compiledFamily) generator.FamilyGenerator { errLog := klog.V(f.ErrorLogV) return generator.FamilyGenerator{ Name: f.Name, + // TODO(@rexagod): This should be dynamic. Type: metric.Gauge, Help: f.Help, GenerateFunc: func(obj interface{}) *metric.Family { @@ -585,8 +606,8 @@ func scrapeValuesFor(e compiledEach, obj map[string]interface{}) ([]eachValue, [ return result, errs } -// getNum converts the value to a float64 which is the value type for any metric. -func getNum(value interface{}, nilIsZero bool) (float64, error) { +// toFloat64 converts the value to a float64 which is the value type for any metric. +func toFloat64(value interface{}, nilIsZero bool) (float64, error) { var v float64 // same as bool==false but for bool pointers if value == nil { diff --git a/pkg/customresourcestate/registry_factory_test.go b/pkg/customresourcestate/registry_factory_test.go index 1c05cd55ce..df41129db4 100644 --- a/pkg/customresourcestate/registry_factory_test.go +++ b/pkg/customresourcestate/registry_factory_test.go @@ -155,7 +155,7 @@ func Test_values(t *testing.T) { compiledCommon: compiledCommon{ path: mustCompilePath(t, "status", "active"), }, - LabelFromKey: "type", + labelFromKey: "type", }, wantResult: []eachValue{ newEachValue(t, 1, "type", "type-a"), newEachValue(t, 3, "type", "type-b"), @@ -167,7 +167,7 @@ func Test_values(t *testing.T) { "active": mustCompilePath(t, "active"), }, }, - LabelFromKey: "type", + labelFromKey: "type", ValueFrom: mustCompilePath(t, "ready"), }, wantResult: []eachValue{ newEachValue(t, 2, "type", "type-a", "active", "1"), @@ -201,7 +201,7 @@ func Test_values(t *testing.T) { newEachValue(t, 0), }}, {name: "info", each: &compiledInfo{ - compiledCommon{ + compiledCommon: compiledCommon{ labelFromPath: map[string]valuePath{ "version": mustCompilePath(t, "spec", "version"), }, @@ -210,10 +210,19 @@ func Test_values(t *testing.T) { newEachValue(t, 1, "version", "v0.0.0"), }}, {name: "info nil path", each: &compiledInfo{ - compiledCommon{ + compiledCommon: compiledCommon{ path: mustCompilePath(t, "does", "not", "exist"), }, }, wantResult: nil}, + {name: "info label from key", each: &compiledInfo{ + compiledCommon: compiledCommon{ + path: mustCompilePath(t, "status", "active"), + }, + labelFromKey: "type", + }, wantResult: []eachValue{ + newEachValue(t, 1, "type", "type-a"), + newEachValue(t, 3, "type", "type-b"), + }}, {name: "stateset", each: &compiledStateSet{ compiledCommon: compiledCommon{ path: mustCompilePath(t, "status", "phase"), From 83d68c57fdfa935ee5200c30021d5661893f4a56 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Wed, 9 Nov 2022 03:34:59 +0530 Subject: [PATCH 2/5] fixup! Allow `labelFromKey` field --- pkg/customresourcestate/registry_factory.go | 30 ++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/customresourcestate/registry_factory.go b/pkg/customresourcestate/registry_factory.go index 3cc575504d..4a8996f69f 100644 --- a/pkg/customresourcestate/registry_factory.go +++ b/pkg/customresourcestate/registry_factory.go @@ -262,33 +262,33 @@ func (c *compiledInfo) Values(v interface{}) (result []eachValue, errs []error) } result = append(result, ev...) } - default: + case map[string]interface{}: value, err := c.values(v) if err != nil { onError(err...) break } // labelFromKey logic - if vv, ok := v.(map[string]interface{}); ok { - for key, val := range vv { - if key != "" && c.labelFromKey != "" { - n, err := toFloat64(val, false) - if err != nil { - onError(err) - continue - } - result = append(result, eachValue{ - Labels: map[string]string{ - c.labelFromKey: key, - }, - Value: n, - }) + for key, val := range iter { + if key != "" && c.labelFromKey != "" { + n, err := toFloat64(val, false) + if err != nil { + onError(err) + continue } + result = append(result, eachValue{ + Labels: map[string]string{ + c.labelFromKey: key, + }, + Value: n, + }) } } if len(result) == 0 { result = value } + default: + result, errs = c.values(v) } return From a1716fa39dbfc5ebf5941764f8814067a1240722 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Wed, 9 Nov 2022 12:27:18 +0530 Subject: [PATCH 3/5] fixup! fixup! Allow `labelFromKey` field --- pkg/customresourcestate/registry_factory.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/customresourcestate/registry_factory.go b/pkg/customresourcestate/registry_factory.go index 4a8996f69f..8782f43194 100644 --- a/pkg/customresourcestate/registry_factory.go +++ b/pkg/customresourcestate/registry_factory.go @@ -269,18 +269,13 @@ func (c *compiledInfo) Values(v interface{}) (result []eachValue, errs []error) break } // labelFromKey logic - for key, val := range iter { + for key := range iter { if key != "" && c.labelFromKey != "" { - n, err := toFloat64(val, false) - if err != nil { - onError(err) - continue - } result = append(result, eachValue{ Labels: map[string]string{ c.labelFromKey: key, }, - Value: n, + Value: 1, }) } } From b9f0d8ddf955936406566e401a3d7c9c6c8287dd Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Wed, 9 Nov 2022 16:49:28 +0530 Subject: [PATCH 4/5] fixup! fixup! fixup! Allow `labelFromKey` field --- pkg/customresourcestate/registry_factory.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/customresourcestate/registry_factory.go b/pkg/customresourcestate/registry_factory.go index 8782f43194..608446cdac 100644 --- a/pkg/customresourcestate/registry_factory.go +++ b/pkg/customresourcestate/registry_factory.go @@ -268,6 +268,12 @@ func (c *compiledInfo) Values(v interface{}) (result []eachValue, errs []error) onError(err...) break } + for _, ev := range value { + if _, ok := ev.Labels[c.labelFromKey]; ok { + onError(fmt.Errorf("labelFromKey (%s) generated labels conflict with labelsFromPath, consider renaming it", c.labelFromKey)) + continue + } + } // labelFromKey logic for key := range iter { if key != "" && c.labelFromKey != "" { From 30725023436018e609faddd5b1969379a4c46fb9 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Thu, 10 Nov 2022 03:10:58 +0530 Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! Allow `labelFromKey` field --- pkg/customresourcestate/registry_factory.go | 12 ++++++++---- pkg/customresourcestate/registry_factory_test.go | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/customresourcestate/registry_factory.go b/pkg/customresourcestate/registry_factory.go index 608446cdac..2c9ae80297 100644 --- a/pkg/customresourcestate/registry_factory.go +++ b/pkg/customresourcestate/registry_factory.go @@ -214,6 +214,10 @@ func (c *compiledGauge) Values(v interface{}) (result []eachValue, errs []error) onError(fmt.Errorf("[%s]: %w", key, err)) continue } + if _, ok := ev.Labels[c.labelFromKey]; ok { + onError(fmt.Errorf("labelFromKey (%s) generated labels conflict with labelsFromPath, consider renaming it", c.labelFromKey)) + continue + } if key != "" && c.labelFromKey != "" { ev.Labels[c.labelFromKey] = key } @@ -285,9 +289,7 @@ func (c *compiledInfo) Values(v interface{}) (result []eachValue, errs []error) }) } } - if len(result) == 0 { - result = value - } + result = append(result, value...) default: result, errs = c.values(v) } @@ -301,7 +303,9 @@ func (c *compiledInfo) values(v interface{}) (result []eachValue, err []error) { } value := eachValue{Value: 1, Labels: map[string]string{}} addPathLabels(v, c.labelFromPath, value.Labels) - result = append(result, value) + if len(value.Labels) != 0 { + result = append(result, value) + } return } diff --git a/pkg/customresourcestate/registry_factory_test.go b/pkg/customresourcestate/registry_factory_test.go index df41129db4..e4da7d997c 100644 --- a/pkg/customresourcestate/registry_factory_test.go +++ b/pkg/customresourcestate/registry_factory_test.go @@ -221,7 +221,7 @@ func Test_values(t *testing.T) { labelFromKey: "type", }, wantResult: []eachValue{ newEachValue(t, 1, "type", "type-a"), - newEachValue(t, 3, "type", "type-b"), + newEachValue(t, 1, "type", "type-b"), }}, {name: "stateset", each: &compiledStateSet{ compiledCommon: compiledCommon{