Merge pull request #2270 from rexagod/2248

fix: fallback to `gauge` for `protofmt`-based negotiations

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)))
+		}
+	}
+}

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.

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) {

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{

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")
 		}

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 {

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
 }