Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fallback to gauge for protofmt-based negotiations #2270

Merged
merged 10 commits into from
Mar 27, 2024
Merged
23 changes: 22 additions & 1 deletion pkg/customresourcestate/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
rexagod marked this conversation as resolved.
Show resolved Hide resolved
// 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)))
}
}
}
10 changes: 0 additions & 10 deletions pkg/customresourcestate/config_metrics_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions pkg/customresourcestate/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions pkg/customresourcestate/custom_resource_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
8 changes: 4 additions & 4 deletions pkg/customresourcestate/registry_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand All @@ -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")
}
Expand Down
23 changes: 13 additions & 10 deletions pkg/metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
49 changes: 42 additions & 7 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,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
dgrisonnet marked this conversation as resolved.
Show resolved Hide resolved
}
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
}