Skip to content

Commit 4be2ddf

Browse files
committed
fixup! fix: fallback to gauge for protofmt-based negotiations
1 parent ee2f53a commit 4be2ddf

File tree

8 files changed

+110
-39
lines changed

8 files changed

+110
-39
lines changed

pkg/customresourcestate/config.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"fmt"
2121
"strings"
2222

23+
"k8s.io/kube-state-metrics/v2/pkg/metric"
24+
2325
"github.com/gobuffalo/flect"
2426
"k8s.io/apimachinery/pkg/runtime/schema"
2527
"k8s.io/klog/v2"
@@ -148,7 +150,7 @@ type Generator struct {
148150
type Metric struct {
149151
// Type defines the type of the metric.
150152
// +unionDiscriminator
151-
Type MetricType `yaml:"type" json:"type"`
153+
Type metric.Type `yaml:"type" json:"type"`
152154

153155
// Gauge defines a gauge metric.
154156
// +optional
@@ -170,9 +172,16 @@ type ConfigDecoder interface {
170172
func FromConfig(decoder ConfigDecoder, discovererInstance *discovery.CRDiscoverer) (func() ([]customresource.RegistryFactory, error), error) {
171173
var customResourceConfig Metrics
172174
factoriesIndex := map[string]bool{}
175+
176+
// Decode the configuration.
173177
if err := decoder.Decode(&customResourceConfig); err != nil {
174178
return nil, fmt.Errorf("failed to parse Custom Resource State metrics: %w", err)
175179
}
180+
181+
// Override the configuration with any custom overrides.
182+
configOverrides(&customResourceConfig)
183+
184+
// Create a factory for each resource.
176185
fn := func() (factories []customresource.RegistryFactory, err error) {
177186
resources := customResourceConfig.Spec.Resources
178187
// resolvedGVKPs will have the final list of GVKs, in addition to the resolved G** resources.
@@ -206,3 +215,14 @@ func FromConfig(decoder ConfigDecoder, discovererInstance *discovery.CRDiscovere
206215
}
207216
return fn, nil
208217
}
218+
219+
// configOverrides applies overrides to the configuration.
220+
func configOverrides(config *Metrics) {
221+
for i := range config.Spec.Resources {
222+
for j := range config.Spec.Resources[i].Metrics {
223+
224+
// Override the metric type to lowercase, so the internals have a single source of truth for metric type definitions.
225+
config.Spec.Resources[i].Metrics[j].Each.Type = metric.Type(strings.ToLower(string(config.Spec.Resources[i].Metrics[j].Each.Type)))
226+
}
227+
}
228+
}

pkg/customresourcestate/config_metrics_types.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,6 @@ limitations under the License.
1616

1717
package customresourcestate
1818

19-
// MetricType is the type of a metric.
20-
type MetricType string
21-
22-
// Supported metric types.
23-
const (
24-
MetricTypeGauge MetricType = "Gauge"
25-
MetricTypeStateSet MetricType = "StateSet"
26-
MetricTypeInfo MetricType = "Info"
27-
)
28-
2919
// MetricMeta are variables which may used for any metric type.
3020
type MetricMeta struct {
3121
// LabelsFromPath adds additional labels where the value of the label is taken from a field under Path.

pkg/customresourcestate/custom_resource_metrics_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"reflect"
2222
"testing"
2323

24+
"k8s.io/kube-state-metrics/v2/pkg/metric"
25+
2426
"k8s.io/apimachinery/pkg/runtime/schema"
2527
"k8s.io/utils/ptr"
2628
)
@@ -55,7 +57,7 @@ func TestNewCustomResourceMetrics(t *testing.T) {
5557
Name: "test_metrics",
5658
Help: "metrics for testing",
5759
Each: Metric{
58-
Type: MetricTypeInfo,
60+
Type: metric.Info,
5961
Info: &MetricInfo{
6062
MetricMeta: MetricMeta{
6163
Path: []string{
@@ -117,7 +119,7 @@ func TestNewCustomResourceMetrics(t *testing.T) {
117119
Name: "test_metrics",
118120
Help: "metrics for testing",
119121
Each: Metric{
120-
Type: MetricTypeInfo,
122+
Type: metric.Info,
121123
Info: &MetricInfo{
122124
MetricMeta: MetricMeta{
123125
Path: []string{
@@ -180,7 +182,7 @@ func TestNewCustomResourceMetrics(t *testing.T) {
180182
Name: "test_metrics",
181183
Help: "metrics for testing",
182184
Each: Metric{
183-
Type: MetricTypeInfo,
185+
Type: metric.Info,
184186
Info: &MetricInfo{
185187
MetricMeta: MetricMeta{
186188
Path: []string{

pkg/customresourcestate/registry_factory.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func compileCommon(c MetricMeta) (*compiledCommon, error) {
7272
func compileFamily(f Generator, resource Resource) (*compiledFamily, error) {
7373
labels := resource.Labels.Merge(f.Labels)
7474

75-
if f.Each.Type == MetricTypeInfo && !strings.HasSuffix(f.Name, "_info") {
75+
if f.Each.Type == metric.Info && !strings.HasSuffix(f.Name, "_info") {
7676
klog.InfoS("Info metric does not have _info suffix", "gvk", resource.GroupVersionKind.String(), "name", f.Name)
7777
}
7878

@@ -153,7 +153,7 @@ type compiledMetric interface {
153153
// newCompiledMetric returns a compiledMetric depending on the given metric type.
154154
func newCompiledMetric(m Metric) (compiledMetric, error) {
155155
switch m.Type {
156-
case MetricTypeGauge:
156+
case metric.Gauge:
157157
if m.Gauge == nil {
158158
return nil, errors.New("expected each.gauge to not be nil")
159159
}
@@ -172,7 +172,7 @@ func newCompiledMetric(m Metric) (compiledMetric, error) {
172172
NilIsZero: m.Gauge.NilIsZero,
173173
labelFromKey: m.Gauge.LabelFromKey,
174174
}, nil
175-
case MetricTypeInfo:
175+
case metric.Info:
176176
if m.Info == nil {
177177
return nil, errors.New("expected each.info to not be nil")
178178
}
@@ -185,7 +185,7 @@ func newCompiledMetric(m Metric) (compiledMetric, error) {
185185
compiledCommon: *cc,
186186
labelFromKey: m.Info.LabelFromKey,
187187
}, nil
188-
case MetricTypeStateSet:
188+
case metric.StateSet:
189189
if m.StateSet == nil {
190190
return nil, errors.New("expected each.stateSet to not be nil")
191191
}

pkg/metric/metric.go

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,68 @@ var (
3737
}
3838
)
3939

40-
// Type represents the type of a metric e.g. a counter. See
41-
// https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types.
40+
// Type represents the type of the metric. See https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types.
4241
type Type string
4342

44-
// Gauge defines a OpenMetrics gauge.
45-
var Gauge Type = "gauge"
43+
// TypeN represents the type of the metric as an integer.
44+
type TypeN int
4645

47-
// Info defines an OpenMetrics info.
48-
var Info Type = "info"
46+
// String returns the string representation of the metric type.
47+
func (i TypeN) String() Type {
48+
return MetricTypeNMap[i]
49+
}
50+
51+
// NString returns the string representation of the metric type as an integer.
52+
func (i TypeN) NString() string {
53+
return strconv.Itoa(int(i))
54+
}
55+
56+
// Supported metric types.
57+
const (
58+
// GaugeN defines a OpenMetrics gauge.
59+
GaugeN TypeN = iota
60+
61+
// InfoN defines an OpenMetrics info.
62+
InfoN
63+
64+
// StateSetN defines an OpenMetrics stateset.
65+
StateSetN
66+
67+
// CounterN defines a OpenMetrics counter.
68+
CounterN
69+
)
4970

50-
// StateSet defines an OpenMetrics stateset.
51-
var StateSet Type = "stateset"
71+
// Supported metric types.
72+
var (
73+
74+
// Gauge defines a OpenMetrics gauge.
75+
Gauge Type = "gauge"
76+
77+
// Info defines an OpenMetrics info.
78+
Info Type = "info"
79+
80+
// StateSet defines an OpenMetrics stateset.
81+
StateSet Type = "stateset"
82+
83+
// Counter defines a OpenMetrics counter.
84+
Counter Type = "counter"
5285

53-
// Counter defines a OpenMetrics counter.
54-
var Counter Type = "counter"
86+
// MetricTypeNMap is a map of MetricTypeN to MetricType.
87+
MetricTypeNMap = map[TypeN]Type{
88+
GaugeN: Gauge,
89+
InfoN: Info,
90+
StateSetN: StateSet,
91+
CounterN: Counter,
92+
}
93+
94+
// MetricTypeMap is a map of MetricType to MetricTypeN.
95+
MetricTypeMap = map[Type]TypeN{
96+
Gauge: GaugeN,
97+
Info: InfoN,
98+
StateSet: StateSetN,
99+
Counter: CounterN,
100+
}
101+
)
55102

56103
// Metric represents a single time series.
57104
type Metric struct {

pkg/metric_generator/generator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func (g *FamilyGenerator) generateHeader() string {
9090
header.WriteString("# TYPE ")
9191
header.WriteString(g.Name)
9292
header.WriteByte(' ')
93-
header.WriteString(string(g.Type))
93+
header.WriteString(metric.MetricTypeMap[g.Type].NString())
9494

9595
return header.String()
9696
}

pkg/metrics_store/metrics_writer.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,20 +93,31 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite
9393
for _, writer := range writers {
9494
if len(writer.stores) > 0 {
9595
for i, header := range writer.stores[0].headers {
96-
// If the requested content type was proto-based, replace the type with "gauge", as "info" and "statesets" are not recognized by Prometheus' protobuf machinery.
97-
if strings.HasPrefix(contentType, expfmt.ProtoType) &&
98-
strings.HasPrefix(header, "# HELP") &&
99-
(strings.HasSuffix(header, " "+string(metric.Info)) || strings.HasSuffix(header, " "+string(metric.StateSet))) {
100-
typeStringWithoutTypePaddedIndex := strings.LastIndex(header, " ")
101-
typeStringWithoutType := header[:typeStringWithoutTypePaddedIndex]
102-
writer.stores[0].headers[i] = typeStringWithoutType + " " + string(metric.Gauge)
96+
97+
// Nothing to sanitize.
98+
if len(header) == 0 {
99+
continue
103100
}
101+
104102
// Removes duplicate headers from the given MetricsWriterList for the same family (generated through CRS).
105103
// These are expected to be consecutive since G** resolution generates groups of similar metrics with same headers before moving onto the next G** spec in the CRS configuration.
106104
if header == lastHeader {
107105
writer.stores[0].headers[i] = ""
108-
} else {
106+
} else if strings.HasPrefix(header, "# HELP") {
109107
lastHeader = header
108+
109+
// If the requested content type was proto-based, replace the type with "gauge", as "info" and "statesets" are not recognized by Prometheus' protobuf machinery,
110+
// else replace them by their respective string representations.
111+
if strings.HasPrefix(contentType, expfmt.ProtoType) &&
112+
(strings.HasSuffix(header, metric.InfoN.NString()) || strings.HasSuffix(header, metric.StateSetN.NString())) {
113+
writer.stores[0].headers[i] = header[:len(header)-1] + string(metric.Gauge)
114+
}
115+
116+
// Replace all remaining type enums with their string representations.
117+
n := int(header[len(header)-1]) - '0'
118+
if n >= 0 && n < len(metric.MetricTypeNMap) {
119+
writer.stores[0].headers[i] = header[:len(header)-1] + string(metric.MetricTypeNMap[metric.TypeN(n)])
120+
}
110121
}
111122
}
112123
}

pkg/metrics_store/metrics_writer_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ package metricsstore_test
1818

1919
import (
2020
"fmt"
21-
"github.com/prometheus/common/expfmt"
2221
"strings"
2322
"testing"
2423

24+
"github.com/prometheus/common/expfmt"
25+
2526
v1 "k8s.io/api/core/v1"
2627
"k8s.io/apimachinery/pkg/api/meta"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -297,7 +298,7 @@ func BenchmarkSanitizeHeaders(b *testing.B) {
297298
headers := []string{}
298299
for j := 0; j < 10e4; j++ {
299300
if benchmark.writersContainsDuplicates {
300-
headers = append(headers, fmt.Sprintf("# HELP foo foo_help\n# TYPE foo info"))
301+
headers = append(headers, "# HELP foo foo_help\n# TYPE foo info")
301302
} else {
302303
headers = append(headers, fmt.Sprintf("# HELP foo_%d foo_help\n# TYPE foo_%d info", j, j))
303304
}

0 commit comments

Comments
 (0)