fixup! fixup! fixup! fixup! fixup! fixup! fixup! fix: fallback to `ga…

…uge` for `protofmt`-based negotiations
kubernetes · Jan 14, 2024 · c3b9a46 · c3b9a46
1 parent b6496c7
commit c3b9a46
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 21 deletions.
diff --git a/pkg/customresourcestate/config.go b/pkg/customresourcestate/config.go
@@ -222,6 +222,7 @@ func configOverrides(config *Metrics) {
 		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/registry_factory.go b/pkg/customresourcestate/registry_factory.go
@@ -152,7 +152,7 @@ type compiledMetric interface {
 
 // newCompiledMetric returns a compiledMetric depending on the given metric type.
 func newCompiledMetric(m Metric) (compiledMetric, error) {
-	switch metric.Type(strings.ToLower(string(m.Type))) {
+	switch m.Type {
 	case metric.Gauge:
 		if m.Gauge == nil {
 			return nil, errors.New("expected each.gauge to not be nil")

diff --git a/pkg/metric/metric.go b/pkg/metric/metric.go
@@ -40,11 +40,6 @@ var (
 // Type represents the type of the metric. See https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types.
 type Type string
 
-// String returns the string representation of the metric type.
-func (t Type) String() string {
-	return string(t)
-}
-
 // Supported metric types.
 var (
 

diff --git a/pkg/metric_generator/generator.go b/pkg/metric_generator/generator.go
@@ -90,7 +90,7 @@ func (g *FamilyGenerator) generateHeader() string {
 	header.WriteString("# TYPE ")
 	header.WriteString(g.Name)
 	header.WriteByte(' ')
-	header.WriteString(g.Type.String())
+	header.WriteString(string(g.Type))
 
 	return header.String()
 }

diff --git a/pkg/metrics_store/metrics_writer.go b/pkg/metrics_store/metrics_writer.go
@@ -106,8 +106,8 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite
 
 					// If the requested content type was proto-based, replace "info" and "statesets" with "gauge", as they are not recognized by Prometheus' protobuf machinery.
 					if strings.HasPrefix(contentType, expfmt.ProtoType) {
-						infoTypeString := metric.Info.String()
-						stateSetTypeString := metric.StateSet.String()
+						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
@@ -121,7 +121,10 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite
 
 				// Nullify duplicate headers after the sanitization to not miss out on any new candidates.
 				if header == lastHeader {
-					writer.stores[0].headers[i] = ""
+					writer.stores[0].headers = append(writer.stores[0].headers[:i], writer.stores[0].headers[i+1:]...)
+
+					// Decrement the inxdex to account for the removed header.
+					i--
 				}
 
 				// Update the last header.
@@ -130,16 +133,5 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite
 		}
 	}
 
-	// Remove all empty headers.
-	for _, writer := range writers {
-		if len(writer.stores) > 0 {
-			for i := len(writer.stores[0].headers) - 1; i >= 0; i-- {
-				if writer.stores[0].headers[i] == "" {
-					writer.stores[0].headers = append(writer.stores[0].headers[:i], writer.stores[0].headers[i+1:]...)
-				}
-			}
-		}
-	}
-
 	return writers
 }