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

feat: validate stable metrics via static analysis, which adapts codes from k/k stable metrics framework #2294

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,24 @@ jobs:
- name: End-to-end tests
run: |
make e2e
ci-validate-stable-metrics-tests:
name: ci-validate-stable-metrics
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@v4

- name: Set up Go 1.x
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
id: go

- name: Setup environment
run: |
make install-tools
- name: End-to-end tests
run: |
make validate-stable-metrics
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ clean:
e2e:
./tests/e2e.sh

validate-stable-metrics:
./tests/validate-stability.sh

fix-stable-metrics:
./tests/fix-stability.sh

generate: build-local
@echo ">> generating docs"
@./scripts/generate-help-text.sh
Expand Down
85 changes: 85 additions & 0 deletions docs/design/validate-stable-metrics.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Kube-State-Metrics - Validate stable metrics Proposal

---

Author: CatherineF-dev@

Date: 1. Jan 2024

---

## Glossary

* STABLE metrics: it's same to kubernetes/kubernetes BETA metrics. It does
not allow breaking changes which break metrics queries. For example, changing
label name is not allowed.

* Experimental metrics: it's same to kubernetes/kubernetes ALPHA metrics. It
allows breaking changes. For example, it allows changing label name.

## Problem Statement

Broken stable metrics bring overhead for down-stream users to migrate metrics
queries.

## Goal

The goal of this proposal is guaranteeing these for stable metrics:

1. metric name not changed

2. metric type not changed

3. old metric labels is a subset of new metric labels

## Status Quo

Kubernetes/kubernetes has implemented stable metrics framework. It can not be
used in KSM repo directly, because kubernetes/kubernetes metrics are defined
using prometheus libraries while KSM metrics are defined using custom functions.

## Proposal - validate stable metrics using static analysis

1. Add a new funciton NewFamilyGeneratorWithStabilityV2 which has labels in its
parameter. It's easier for static analysis in step 2.

2. Adapt stable metrics framework <https://github.com/kubernetes/kubernetes/tree/master/test/instrumentation>
into kube-state-metrics repo.

2.1 Find stable metrics definitions. It finds all function calls with name NewFamilyGeneratorWithStabilityV2 where fourth paraemeter (StabilityLevel) is stable

2.2 Extract metric name, labels, help, stability level from 2.1

```
func createPodInitContainerStatusRestartsTotalFamilyGenerator() generator.FamilyGenerator {
return *generator.NewFamilyGeneratorWithStabilityV2(
"kube_pod_init_container_status_restarts_total",
"The number of restarts for the init container.",
metric.Counter, basemetrics.STABLE,
"",
[]string{"container"}, # labels
...
}
```

2.3 Export all stable metrics, with format

```
- name: kube_pod_init_container_status_restarts_total
help: The number of restarts for the init container.
type: Counter
stabilityLevel: STABLE
labels:
- container
```

2.4 Compare output in 2.3 with expected results tests/stablemetrics/testdata/test-stable-metrics-list.yaml

## Alternatives

### Validate exposed metrics in runtime

Generated metrics are not complete. Some metrics are exposed when certain conditions
are met.

## FAQ / Follow up improvements
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/spf13/cobra v1.8.0
github.com/spf13/viper v1.18.2
github.com/stretchr/testify v1.8.4
gopkg.in/yaml.v2 v2.4.0
gopkg.in/yaml.v3 v3.0.1
k8s.io/api v0.28.4
k8s.io/apimachinery v0.28.4
Expand Down Expand Up @@ -84,7 +85,6 @@ require (
google.golang.org/protobuf v1.31.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
Expand Down
4 changes: 2 additions & 2 deletions internal/store/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -882,17 +882,17 @@ func createPodInitContainerStatusReadyFamilyGenerator() generator.FamilyGenerato
}

func createPodInitContainerStatusRestartsTotalFamilyGenerator() generator.FamilyGenerator {
return *generator.NewFamilyGeneratorWithStability(
return *generator.NewFamilyGeneratorWithStabilityV2(
"kube_pod_init_container_status_restarts_total",
"The number of restarts for the init container.",
metric.Counter, basemetrics.STABLE,
"",
[]string{"container"},
wrapPodFunc(func(p *v1.Pod) *metric.Family {
ms := make([]*metric.Metric, len(p.Status.InitContainerStatuses))

for i, cs := range p.Status.InitContainerStatuses {
ms[i] = &metric.Metric{
LabelKeys: []string{"container"},
LabelValues: []string{cs.Name},
Value: float64(cs.RestartCount),
}
Expand Down
31 changes: 31 additions & 0 deletions pkg/metric_generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ type FamilyGenerator struct {
GenerateFunc func(obj interface{}) *metric.Family
}

// NewFamilyGeneratorWithStabilityV2 creates new FamilyGenerator instances with metric
// stabilityLevel and explicit labels. These explicit labels are used for verifying stable metrics.
func NewFamilyGeneratorWithStabilityV2(name string, help string, metricType metric.Type, stabilityLevel basemetrics.StabilityLevel, deprecatedVersion string, labels []string, generateFunc func(obj interface{}) *metric.Family) *FamilyGenerator {
f := &FamilyGenerator{
Name: name,
Type: metricType,
Help: help,
OptIn: false,
StabilityLevel: stabilityLevel,
DeprecatedVersion: deprecatedVersion,
GenerateFunc: wrapLabels(generateFunc, labels),
}
if deprecatedVersion != "" {
f.Help = fmt.Sprintf("(Deprecated since %s) %s", deprecatedVersion, help)
}
return f
}

// NewFamilyGeneratorWithStability creates new FamilyGenerator instances with metric
// stabilityLevel.
func NewFamilyGeneratorWithStability(name string, help string, metricType metric.Type, stabilityLevel basemetrics.StabilityLevel, deprecatedVersion string, generateFunc func(obj interface{}) *metric.Family) *FamilyGenerator {
Expand All @@ -57,6 +75,19 @@ func NewFamilyGeneratorWithStability(name string, help string, metricType metric
return f
}

func wrapLabels(generateFunc func(obj interface{}) *metric.Family, labels []string) func(obj interface{}) *metric.Family {
return func(obj interface{}) *metric.Family {

metricFamily := generateFunc(obj)

for _, m := range metricFamily.Metrics {
m.LabelKeys = append(m.LabelKeys, labels...)
}

return metricFamily
}
}

// NewOptInFamilyGenerator creates new FamilyGenerator instances for opt-in metric families.
func NewOptInFamilyGenerator(name string, help string, metricType metric.Type, stabilityLevel basemetrics.StabilityLevel, deprecatedVersion string, generateFunc func(obj interface{}) *metric.Family) *FamilyGenerator {
f := NewFamilyGeneratorWithStability(name, help, metricType, stabilityLevel,
Expand Down
17 changes: 17 additions & 0 deletions tests/fix-stability.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bash

set -o errexit

out_file='tests/stablemetrics/testdata/test-stable-metrics-list.yaml'
metric_file='internal/store/pod.go'

go run \
"tests/stablemetrics/main.go" \
"tests/stablemetrics/decode_metric.go" \
"tests/stablemetrics/find_stable_metric.go" \
"tests/stablemetrics/error.go" \
"tests/stablemetrics/metric.go" \
-- \
"${metric_file}" >"${out_file}"


173 changes: 173 additions & 0 deletions tests/stablemetrics/decode_metric.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/*
Copyright 2019 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
"go/ast"
"go/token"
"strings"

"k8s.io/component-base/metrics"
)

func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName string, variables map[string]ast.Expr) ([]metric, []error) {
finder := metricDecoder{
kubeMetricsImportName: metricsImportName,
variables: variables,
}
ms := make([]metric, 0, len(fs))
errors := []error{}
for _, f := range fs {
m, err := finder.decodeNewMetricCall(f)
if err != nil {
errors = append(errors, err)
continue
}
if m != nil {
ms = append(ms, *m)
}
}
return ms, errors
}

type metricDecoder struct {
kubeMetricsImportName string
variables map[string]ast.Expr
}

func (c *metricDecoder) decodeNewMetricCall(fc *ast.CallExpr) (*metric, error) {
var m metric
var err error

_, ok := fc.Fun.(*ast.SelectorExpr)
if !ok {
return nil, newDecodeErrorf(fc, errNotDirectCall)
}
m, err = c.decodeDesc(fc)
return &m, err
}

func (c *metricDecoder) decodeDesc(ce *ast.CallExpr) (metric, error) {
m := &metric{}

name, err := c.decodeString(ce.Args[0])
if err != nil {
return *m, newDecodeErrorf(ce, errorDecodingString)
}
m.Name = *name

help, err := c.decodeString(ce.Args[1])
if err != nil {
return *m, newDecodeErrorf(ce, errorDecodingString)
}
m.Help = *help

metricType, err := decodeStabilityLevel(ce.Args[2], "metric")
if err != nil {
return *m, newDecodeErrorf(ce, errorDecodingString)
}
m.Type = string(*metricType)

sl, err := decodeStabilityLevel(ce.Args[3], "basemetrics")
if err != nil {
return *m, newDecodeErrorf(ce, "can't decode stability level")
}

if sl != nil {
m.StabilityLevel = string(*sl)
}

labels, err := c.decodeLabels(ce.Args[5])
if err != nil {
return *m, newDecodeErrorf(ce, errorDecodingLabels)
}
m.Labels = labels

return *m, nil
}

func (c *metricDecoder) decodeString(expr ast.Expr) (*string, error) {

switch e := expr.(type) {

Check failure on line 105 in tests/stablemetrics/decode_metric.go

View workflow job for this annotation

GitHub Actions / ci-go-lint

singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
case *ast.BasicLit:
value, err := stringValue(e)
if err != nil {
return nil, err
}
return &value, nil
}
return nil, newDecodeErrorf(expr, errorDecodingString)
}

func (c *metricDecoder) decodeLabelsFromArray(exprs []ast.Expr) ([]string, error) {
retval := []string{}
for _, e := range exprs {
v, err := c.decodeString(e)
if err != nil || v == nil {
return nil, newDecodeErrorf(e, errNonStringAttribute)
}
retval = append(retval, *v)
}

return retval, nil
}

func (c *metricDecoder) decodeLabels(expr ast.Expr) ([]string, error) {
cl, ok := expr.(*ast.CompositeLit)
if !ok {
switch e := expr.(type) {

Check failure on line 132 in tests/stablemetrics/decode_metric.go

View workflow job for this annotation

GitHub Actions / ci-go-lint

singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
case *ast.Ident:
if e.Name == "nil" {
return []string{}, nil
}
variableExpr, found := c.variables[e.Name]
if !found {
return nil, newDecodeErrorf(expr, errorFindingVariableForLabels)
}
cl2, ok := variableExpr.(*ast.CompositeLit)
if !ok {
return nil, newDecodeErrorf(expr, errorFindingVariableForLabels)
}
cl = cl2
}
}
return c.decodeLabelsFromArray(cl.Elts)
}

func stringValue(bl *ast.BasicLit) (string, error) {
if bl.Kind != token.STRING {
return "", newDecodeErrorf(bl, errNonStringAttribute)
}
return strings.Trim(bl.Value, `"`), nil
}

func decodeStabilityLevel(expr ast.Expr, metricsFrameworkImportName string) (*metrics.StabilityLevel, error) {
se, ok := expr.(*ast.SelectorExpr)
if !ok {
return nil, newDecodeErrorf(expr, errStabilityLevel)
}
s, ok := se.X.(*ast.Ident)
if !ok {
return nil, newDecodeErrorf(expr, errStabilityLevel)
}
if s.String() != metricsFrameworkImportName {
return nil, newDecodeErrorf(expr, errStabilityLevel)
}

stability := metrics.StabilityLevel(se.Sel.Name)
return &stability, nil
}
Loading
Loading