kubernetes
/
kube-state-metrics
mirror of https://github.com/kubernetes/kube-state-metrics.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: fix logic for plain text fallback format 

This pull requests fixes a logic error in metrics_writer.go where metrics
headers are replaced when a protobuf format is requested. However, the
existing logic is never used because the content type negotiation is already
done in a previous step (in metrics_handler.go). There, the content type for
proto-based formats is changed to text/plain before passing the argument
to SanitizeHeaders.

The pull request changes the condition in SanitizeHeaders to check for
the plain-text format instead of protobuf. I changed the signature of
SanitizeHeaders to accept expfmt.Format instead of string. This makes checking
the content type a bit cleaner. If this is considered a breaking change, we
can also change it to a string prefix comparison.

I encountered the error when I tried to use native histogram parsing in
prometheus and found errors while parsing kube-state-metrics' metrics.
The issue is already described in #2587.

Signed-off-by: Timon Engelke <timon.engelke@inovex.de>
This commit is contained in:
Timon Engelke 2025-08-11 17:34:38 +02:00
parent 95b29ac8a9
commit e9e67a2ca2
3 changed files with 47 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/metrics_store/metrics_writer.go
View File

@ -92,7 +92,7 @@ func (m MetricsWriter) WriteAll(w io.Writer) error {
}
// SanitizeHeaders sanitizes the headers of the given MetricsWriterList.
func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWriterList {
func SanitizeHeaders(contentType expfmt.Format, writers MetricsWriterList) MetricsWriterList {
var lastHeader string
for _, writer := range writers {
if len(writer.stores) > 0 {
@ -104,8 +104,9 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite
// 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) {
// If the requested content type is text/plain, replace "info" and "statesets" with "gauge", as they are not recognized by Prometheus' plain text machinery.
// When Prometheus requests proto-based formats, this branch is also used because any requested format that is not OpenMetrics falls back to text/plain in metrics_handler.go
if contentType.FormatType() == expfmt.TypeTextPlain {
infoTypeString := string(metric.Info)
stateSetTypeString := string(metric.StateSet)
if strings.HasSuffix(header, infoTypeString) {

84
pkg/metrics_store/metrics_writer_test.go
View File

@ -275,6 +275,46 @@ func TestSanitizeHeaders(t *testing.T) {
headers []string
expectedHeaders []string
}{
{
name: "OpenMetricsText unique headers",
contentType: expfmt.NewFormat(expfmt.TypeOpenMetrics),
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: "OpenMetricsText consecutive duplicate headers",
contentType: expfmt.NewFormat(expfmt.TypeOpenMetrics),
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: "text-format unique headers",
contentType: expfmt.NewFormat(expfmt.TypeTextPlain),
@ -287,8 +327,6 @@ func TestSanitizeHeaders(t *testing.T) {
},
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",
},
},
@ -308,44 +346,6 @@ func TestSanitizeHeaders(t *testing.T) {
"# 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.NewFormat(expfmt.TypeProtoText), // 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.NewFormat(expfmt.TypeProtoText), // 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",
@ -356,7 +356,7 @@ func TestSanitizeHeaders(t *testing.T) {
for _, testcase := range testcases {
writer := NewMetricsWriter(NewMetricsStore(testcase.headers, nil))
t.Run(testcase.name, func(t *testing.T) {
SanitizeHeaders(string(testcase.contentType), MetricsWriterList{writer})
SanitizeHeaders(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))
}
@ -404,7 +404,7 @@ func BenchmarkSanitizeHeaders(b *testing.B) {
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})
SanitizeHeaders(benchmark.contentType, MetricsWriterList{writer})
}
})
}

2
pkg/metricshandler/metrics_handler.go
View File

@ -220,7 +220,7 @@ func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}
m.metricsWriters = metricsstore.SanitizeHeaders(string(contentType), m.metricsWriters)
m.metricsWriters = metricsstore.SanitizeHeaders(contentType, m.metricsWriters)
for _, w := range m.metricsWriters {
err := w.WriteAll(writer)
if err != nil {