Skip to content

Commit

Permalink
fix: fallback to gauge for protofmt-based negotiations
Browse files Browse the repository at this point in the history
Fallback to `gauge` metric type when the negotiated content-type is
`protofmt`-based, since Prometheus' protobuf machinery does not
recognize all OpenMetrics types (`info` and `statesets` in this
context).

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
  • Loading branch information
rexagod committed Dec 14, 2023
1 parent 14e935a commit 6409603
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 4 deletions.
20 changes: 17 additions & 3 deletions pkg/metrics_store/metrics_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -82,13 +87,22 @@ 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 {
// If the requested content type was proto-based, replace the type with "gauge", as "info" and "statesets" are not recognized by Prometheus' protobuf machinery.
if strings.HasPrefix(contentType, expfmt.ProtoType) &&
strings.HasPrefix(header, "# HELP") &&
(strings.HasSuffix(header, " "+string(metric.Info)) || strings.HasSuffix(header, " "+string(metric.StateSet))) {
typeStringWithoutTypePaddedIndex := strings.LastIndex(header, " ")
typeStringWithoutType := header[:typeStringWithoutTypePaddedIndex]
writer.stores[0].headers[i] = typeStringWithoutType + " " + string(metric.Gauge)
}
// 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.
if header == lastHeader {
writer.stores[0].headers[i] = ""
} else {
Expand Down
47 changes: 47 additions & 0 deletions pkg/metrics_store/metrics_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package metricsstore_test

import (
"fmt"
"github.com/prometheus/common/expfmt"
"strings"
"testing"

Expand Down Expand Up @@ -263,3 +264,49 @@ func TestWriteAllWithEmptyStores(t *testing.T) {
t.Fatalf("Unexpected output, got %q, want %q", result, "")
}
}

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, fmt.Sprintf("# HELP foo foo_help\n# TYPE foo info"))

Check failure on line 300 in pkg/metrics_store/metrics_writer_test.go

View workflow job for this annotation

GitHub Actions / ci-go-lint

S1039: unnecessary use of fmt.Sprintf (gosimple)
} else {
headers = append(headers, fmt.Sprintf("# HELP foo_%d foo_help\n# TYPE foo_%d info", j, j))
}
}
writer := metricsstore.NewMetricsWriter(metricsstore.NewMetricsStore(headers, nil))
b.Run(benchmark.name, func(b *testing.B) {
for i := 0; i < b.N; i++ {
metricsstore.SanitizeHeaders(string(benchmark.contentType), metricsstore.MetricsWriterList{writer})
}
})
}
}
3 changes: 2 additions & 1 deletion pkg/metricshandler/metrics_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
var writer io.Writer = w

contentType := expfmt.NegotiateIncludingOpenMetrics(r.Header)
gotContentType := contentType

// 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 {
Expand All @@ -208,7 +209,7 @@ func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}

m.metricsWriters = metricsstore.SanitizeHeaders(m.metricsWriters)
m.metricsWriters = metricsstore.SanitizeHeaders(string(gotContentType), m.metricsWriters)
for _, w := range m.metricsWriters {
err := w.WriteAll(writer)
if err != nil {
Expand Down

0 comments on commit 6409603

Please sign in to comment.