diff --git a/pkg/customresourcestate/config.go b/pkg/customresourcestate/config.go index 385858b8ba..873591372f 100644 --- a/pkg/customresourcestate/config.go +++ b/pkg/customresourcestate/config.go @@ -20,6 +20,8 @@ import ( "fmt" "strings" + "k8s.io/kube-state-metrics/v2/pkg/metric" + "github.com/gobuffalo/flect" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/klog/v2" @@ -148,7 +150,7 @@ type Generator struct { type Metric struct { // Type defines the type of the metric. // +unionDiscriminator - Type MetricType `yaml:"type" json:"type"` + Type metric.Type `yaml:"type" json:"type"` // Gauge defines a gauge metric. // +optional @@ -170,9 +172,16 @@ type ConfigDecoder interface { func FromConfig(decoder ConfigDecoder, discovererInstance *discovery.CRDiscoverer) (func() ([]customresource.RegistryFactory, error), error) { var customResourceConfig Metrics factoriesIndex := map[string]bool{} + + // Decode the configuration. if err := decoder.Decode(&customResourceConfig); err != nil { return nil, fmt.Errorf("failed to parse Custom Resource State metrics: %w", err) } + + // Override the configuration with any custom overrides. + configOverrides(&customResourceConfig) + + // Create a factory for each resource. fn := func() (factories []customresource.RegistryFactory, err error) { resources := customResourceConfig.Spec.Resources // resolvedGVKPs will have the final list of GVKs, in addition to the resolved G** resources. @@ -206,3 +215,15 @@ func FromConfig(decoder ConfigDecoder, discovererInstance *discovery.CRDiscovere } return fn, nil } + +// configOverrides applies overrides to the configuration. +func configOverrides(config *Metrics) { + for i := range config.Spec.Resources { + for j := range config.Spec.Resources[i].Metrics { + + // Override the metric type to lowercase, so the internals have a single source of truth for metric type definitions. + // This is done as a convenience measure for users, so they don't have to remember the exact casing. + config.Spec.Resources[i].Metrics[j].Each.Type = metric.Type(strings.ToLower(string(config.Spec.Resources[i].Metrics[j].Each.Type))) + } + } +} diff --git a/pkg/customresourcestate/config_metrics_types.go b/pkg/customresourcestate/config_metrics_types.go index 6e8e9167cd..25d63bfb69 100644 --- a/pkg/customresourcestate/config_metrics_types.go +++ b/pkg/customresourcestate/config_metrics_types.go @@ -16,16 +16,6 @@ limitations under the License. package customresourcestate -// MetricType is the type of a metric. -type MetricType string - -// Supported metric types. -const ( - MetricTypeGauge MetricType = "Gauge" - MetricTypeStateSet MetricType = "StateSet" - MetricTypeInfo MetricType = "Info" -) - // MetricMeta are variables which may used for any metric type. type MetricMeta struct { // LabelsFromPath adds additional labels where the value of the label is taken from a field under Path. diff --git a/pkg/customresourcestate/config_test.go b/pkg/customresourcestate/config_test.go index 7ae11985f2..6927b3c211 100644 --- a/pkg/customresourcestate/config_test.go +++ b/pkg/customresourcestate/config_test.go @@ -32,6 +32,7 @@ var testData string func Test_Metrics_deserialization(t *testing.T) { var m Metrics assert.NoError(t, yaml.NewDecoder(strings.NewReader(testData)).Decode(&m)) + configOverrides(&m) assert.Equal(t, "active_count", m.Spec.Resources[0].Metrics[0].Name) t.Run("can create resource factory", func(t *testing.T) { diff --git a/pkg/customresourcestate/custom_resource_metrics_test.go b/pkg/customresourcestate/custom_resource_metrics_test.go index 928f668ddd..31121de75c 100644 --- a/pkg/customresourcestate/custom_resource_metrics_test.go +++ b/pkg/customresourcestate/custom_resource_metrics_test.go @@ -21,6 +21,8 @@ import ( "reflect" "testing" + "k8s.io/kube-state-metrics/v2/pkg/metric" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/utils/ptr" ) @@ -55,7 +57,7 @@ func TestNewCustomResourceMetrics(t *testing.T) { Name: "test_metrics", Help: "metrics for testing", Each: Metric{ - Type: MetricTypeInfo, + Type: metric.Info, Info: &MetricInfo{ MetricMeta: MetricMeta{ Path: []string{ @@ -117,7 +119,7 @@ func TestNewCustomResourceMetrics(t *testing.T) { Name: "test_metrics", Help: "metrics for testing", Each: Metric{ - Type: MetricTypeInfo, + Type: metric.Info, Info: &MetricInfo{ MetricMeta: MetricMeta{ Path: []string{ @@ -180,7 +182,7 @@ func TestNewCustomResourceMetrics(t *testing.T) { Name: "test_metrics", Help: "metrics for testing", Each: Metric{ - Type: MetricTypeInfo, + Type: metric.Info, Info: &MetricInfo{ MetricMeta: MetricMeta{ Path: []string{ diff --git a/pkg/customresourcestate/registry_factory.go b/pkg/customresourcestate/registry_factory.go index 09e678a6c3..c59f5aeabe 100644 --- a/pkg/customresourcestate/registry_factory.go +++ b/pkg/customresourcestate/registry_factory.go @@ -72,7 +72,7 @@ func compileCommon(c MetricMeta) (*compiledCommon, error) { func compileFamily(f Generator, resource Resource) (*compiledFamily, error) { labels := resource.Labels.Merge(f.Labels) - if f.Each.Type == MetricTypeInfo && !strings.HasSuffix(f.Name, "_info") { + if f.Each.Type == metric.Info && !strings.HasSuffix(f.Name, "_info") { klog.InfoS("Info metric does not have _info suffix", "gvk", resource.GroupVersionKind.String(), "name", f.Name) } @@ -153,7 +153,7 @@ type compiledMetric interface { // newCompiledMetric returns a compiledMetric depending on the given metric type. func newCompiledMetric(m Metric) (compiledMetric, error) { switch m.Type { - case MetricTypeGauge: + case metric.Gauge: if m.Gauge == nil { return nil, errors.New("expected each.gauge to not be nil") } @@ -172,7 +172,7 @@ func newCompiledMetric(m Metric) (compiledMetric, error) { NilIsZero: m.Gauge.NilIsZero, labelFromKey: m.Gauge.LabelFromKey, }, nil - case MetricTypeInfo: + case metric.Info: if m.Info == nil { return nil, errors.New("expected each.info to not be nil") } @@ -185,7 +185,7 @@ func newCompiledMetric(m Metric) (compiledMetric, error) { compiledCommon: *cc, labelFromKey: m.Info.LabelFromKey, }, nil - case MetricTypeStateSet: + case metric.StateSet: if m.StateSet == nil { return nil, errors.New("expected each.stateSet to not be nil") } diff --git a/pkg/metric/metric.go b/pkg/metric/metric.go index 007eb9fa87..d138b240a4 100644 --- a/pkg/metric/metric.go +++ b/pkg/metric/metric.go @@ -37,21 +37,24 @@ var ( } ) -// Type represents the type of a metric e.g. a counter. See -// https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types. +// Type represents the type of the metric. See https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types. type Type string -// Gauge defines a OpenMetrics gauge. -var Gauge Type = "gauge" +// Supported metric types. +var ( + + // Gauge defines an OpenMetrics gauge. + Gauge Type = "gauge" -// Info defines an OpenMetrics info. -var Info Type = "info" + // Info defines an OpenMetrics info. + Info Type = "info" -// StateSet defines an OpenMetrics stateset. -var StateSet Type = "stateset" + // StateSet defines an OpenMetrics stateset. + StateSet Type = "stateset" -// Counter defines a OpenMetrics counter. -var Counter Type = "counter" + // Counter defines an OpenMetrics counter. + Counter Type = "counter" +) // Metric represents a single time series. type Metric struct { diff --git a/pkg/metrics_store/metrics_writer.go b/pkg/metrics_store/metrics_writer.go index 77a9df6106..0e263e4654 100644 --- a/pkg/metrics_store/metrics_writer.go +++ b/pkg/metrics_store/metrics_writer.go @@ -19,6 +19,11 @@ package metricsstore import ( "fmt" "io" + "strings" + + "github.com/prometheus/common/expfmt" + + "k8s.io/kube-state-metrics/v2/pkg/metric" ) // MetricsWriterList represent a list of MetricsWriter @@ -82,20 +87,50 @@ func (m MetricsWriter) WriteAll(w io.Writer) error { return nil } -// SanitizeHeaders removes duplicate headers from the given MetricsWriterList for the same family (generated through CRS). -// 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. -func SanitizeHeaders(writers MetricsWriterList) MetricsWriterList { +// SanitizeHeaders sanitizes the headers of the given MetricsWriterList. +func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWriterList { var lastHeader string for _, writer := range writers { if len(writer.stores) > 0 { - for i, header := range writer.stores[0].headers { + for i := 0; i < len(writer.stores[0].headers); { + header := writer.stores[0].headers[i] + + // Removes duplicate headers from the given MetricsWriterList for the same family (generated through CRS). + // 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. + // Skip this step if we encounter a repeated header, as it will be removed. + if header != lastHeader && strings.HasPrefix(header, "# HELP") { + + // If the requested content type was proto-based (such as FmtProtoDelim, FmtProtoText, or FmtProtoCompact), replace "info" and "statesets" with "gauge", as they are not recognized by Prometheus' protobuf machinery. + if strings.HasPrefix(contentType, expfmt.ProtoType) { + infoTypeString := string(metric.Info) + stateSetTypeString := string(metric.StateSet) + if strings.HasSuffix(header, infoTypeString) { + header = header[:len(header)-len(infoTypeString)] + string(metric.Gauge) + writer.stores[0].headers[i] = header + } + if strings.HasSuffix(header, stateSetTypeString) { + header = header[:len(header)-len(stateSetTypeString)] + string(metric.Gauge) + writer.stores[0].headers[i] = header + } + } + } + + // Nullify duplicate headers after the sanitization to not miss out on any new candidates. if header == lastHeader { - writer.stores[0].headers[i] = "" - } else { - lastHeader = header + writer.stores[0].headers = append(writer.stores[0].headers[:i], writer.stores[0].headers[i+1:]...) + + // Do not increment the index, as the next header is now at the current index. + continue } + + // Update the last header. + lastHeader = header + + // Move to the next header. + i++ } } } + return writers } diff --git a/pkg/metrics_store/metrics_writer_test.go b/pkg/metrics_store/metrics_writer_test.go index 73bf164765..4d33a12369 100644 --- a/pkg/metrics_store/metrics_writer_test.go +++ b/pkg/metrics_store/metrics_writer_test.go @@ -14,19 +14,22 @@ See the License for the specific language governing permissions and limitations under the License. */ -package metricsstore_test +package metricsstore import ( "fmt" + "reflect" "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/prometheus/common/expfmt" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/kube-state-metrics/v2/pkg/metric" - metricsstore "k8s.io/kube-state-metrics/v2/pkg/metrics_store" ) func TestWriteAllWithSingleStore(t *testing.T) { @@ -60,7 +63,7 @@ func TestWriteAllWithSingleStore(t *testing.T) { return []metric.FamilyInterface{&mf1, &mf2} } - store := metricsstore.NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) + store := NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) svcs := []v1.Service{ { ObjectMeta: metav1.ObjectMeta{ @@ -84,7 +87,7 @@ func TestWriteAllWithSingleStore(t *testing.T) { } } - multiNsWriter := metricsstore.NewMetricsWriter(store) + multiNsWriter := NewMetricsWriter(store) w := strings.Builder{} err := multiNsWriter.WriteAll(&w) if err != nil { @@ -148,7 +151,7 @@ func TestWriteAllWithMultipleStores(t *testing.T) { return []metric.FamilyInterface{&mf1, &mf2} } - s1 := metricsstore.NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) + s1 := NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) svcs1 := []v1.Service{ { ObjectMeta: metav1.ObjectMeta{ @@ -188,7 +191,7 @@ func TestWriteAllWithMultipleStores(t *testing.T) { }, }, } - s2 := metricsstore.NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) + s2 := NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) for _, s := range svcs2 { svc := s if err := s2.Add(&svc); err != nil { @@ -196,7 +199,7 @@ func TestWriteAllWithMultipleStores(t *testing.T) { } } - multiNsWriter := metricsstore.NewMetricsWriter(s1, s2) + multiNsWriter := NewMetricsWriter(s1, s2) w := strings.Builder{} err := multiNsWriter.WriteAll(&w) if err != nil { @@ -248,9 +251,9 @@ func TestWriteAllWithEmptyStores(t *testing.T) { return []metric.FamilyInterface{&mf1, &mf2} } - store := metricsstore.NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) + store := NewMetricsStore([]string{"Info 1 about services", "Info 2 about services"}, genFunc) - multiNsWriter := metricsstore.NewMetricsWriter(store) + multiNsWriter := NewMetricsWriter(store) w := strings.Builder{} err := multiNsWriter.WriteAll(&w) if err != nil { @@ -263,3 +266,146 @@ func TestWriteAllWithEmptyStores(t *testing.T) { t.Fatalf("Unexpected output, got %q, want %q", result, "") } } + +// No two consecutive headers will be entirely the same. The cases used below are only for their suffixes. +func TestSanitizeHeaders(t *testing.T) { + testcases := []struct { + name string + contentType expfmt.Format + headers []string + expectedHeaders []string + }{ + { + name: "text-format unique headers", + contentType: expfmt.FmtText, + headers: []string{ + "", + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo counter", + }, + expectedHeaders: []string{ + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo counter", + }, + }, + { + name: "text-format consecutive duplicate headers", + contentType: expfmt.FmtText, + headers: []string{ + "", + "", + "", + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo counter", + "# HELP foo foo_help\n# TYPE foo counter", + }, + expectedHeaders: []string{ + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo counter", + }, + }, + { + name: "proto-format unique headers", + contentType: expfmt.ProtoFmt, // Prometheus ProtoFmt is the only proto-based format we check for. + headers: []string{ + "", + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo counter", + }, + expectedHeaders: []string{ + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo counter", + }, + }, + { + name: "proto-format consecutive duplicate headers", + contentType: expfmt.ProtoFmt, // Prometheus ProtoFmt is the only proto-based format we check for. + headers: []string{ + "", + "", + "", + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo info", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo stateset", + "# HELP foo foo_help\n# TYPE foo counter", + "# HELP foo foo_help\n# TYPE foo counter", + }, + expectedHeaders: []string{ + "# HELP foo foo_help\n# TYPE foo gauge", + "# HELP foo foo_help\n# TYPE foo counter", + }, + }, + } + + for _, testcase := range testcases { + writer := NewMetricsWriter(NewMetricsStore(testcase.headers, nil)) + t.Run(testcase.name, func(t *testing.T) { + SanitizeHeaders(string(testcase.contentType), MetricsWriterList{writer}) + if !reflect.DeepEqual(testcase.expectedHeaders, writer.stores[0].headers) { + t.Fatalf("(-want, +got):\n%s", cmp.Diff(testcase.expectedHeaders, writer.stores[0].headers)) + } + }) + } +} + +func BenchmarkSanitizeHeaders(b *testing.B) { + benchmarks := []struct { + name string + contentType expfmt.Format + writersContainsDuplicates bool + }{ + { + name: "text-format unique headers", + contentType: expfmt.FmtText, + writersContainsDuplicates: false, + }, + { + name: "text-format duplicate headers", + contentType: expfmt.FmtText, + writersContainsDuplicates: true, + }, + { + name: "proto-format unique headers", + contentType: expfmt.ProtoFmt, // Prometheus ProtoFmt is the only proto-based format we check for. + writersContainsDuplicates: false, + }, + { + name: "proto-format duplicate headers", + contentType: expfmt.ProtoFmt, // Prometheus ProtoFmt is the only proto-based format we check for. + writersContainsDuplicates: true, + }, + } + + for _, benchmark := range benchmarks { + headers := []string{} + for j := 0; j < 10e4; j++ { + if benchmark.writersContainsDuplicates { + headers = append(headers, "# HELP foo foo_help\n# TYPE foo info") + } else { + headers = append(headers, fmt.Sprintf("# HELP foo_%d foo_help\n# TYPE foo_%d info", j, j)) + } + } + writer := NewMetricsWriter(NewMetricsStore(headers, nil)) + b.Run(benchmark.name, func(b *testing.B) { + for i := 0; i < b.N; i++ { + SanitizeHeaders(string(benchmark.contentType), MetricsWriterList{writer}) + } + }) + } +} diff --git a/pkg/metricshandler/metrics_handler.go b/pkg/metricshandler/metrics_handler.go index 911d24b161..8cee50b4f5 100644 --- a/pkg/metricshandler/metrics_handler.go +++ b/pkg/metricshandler/metrics_handler.go @@ -178,8 +178,8 @@ func (m *MetricsHandler) Run(ctx context.Context) error { return ctx.Err() } -// ServeHTTP implements the http.Handler interface. It writes all generated -// metrics to the response body. +// ServeHTTP implements the http.Handler interface. It writes all generated metrics to the response body. +// Note that all operations defined within this procedure are performed at every request. func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { m.mtx.RLock() defer m.mtx.RUnlock() @@ -188,7 +188,7 @@ func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { contentType := expfmt.NegotiateIncludingOpenMetrics(r.Header) - // We do not support protobuf at the moment. Fall back to FmtText if the negotiated exposition format is not FmtOpenMetrics See: https://github.com/kubernetes/kube-state-metrics/issues/2022 + // We do not support protobuf at the moment. Fall back to FmtText if the negotiated exposition format is not FmtOpenMetrics See: https://github.com/kubernetes/kube-state-metrics/issues/2022. if contentType != expfmt.FmtOpenMetrics_1_0_0 && contentType != expfmt.FmtOpenMetrics_0_0_1 { contentType = expfmt.FmtText } @@ -208,7 +208,7 @@ func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } - m.metricsWriters = metricsstore.SanitizeHeaders(m.metricsWriters) + m.metricsWriters = metricsstore.SanitizeHeaders(string(contentType), m.metricsWriters) for _, w := range m.metricsWriters { err := w.WriteAll(writer) if err != nil {