Allow usage of consts and variables for stable metrics in static analysis

This commit is contained in:
Marek Siarkowicz 2019-10-25 18:49:47 +02:00
parent 08410cbf06
commit 4dd546cf2c
4 changed files with 144 additions and 32 deletions

View File

@ -27,10 +27,10 @@ import (
"k8s.io/component-base/metrics" "k8s.io/component-base/metrics"
) )
func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName, prometheusImportName string) ([]metric, []error) { func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName string, variables map[string]ast.Expr) ([]metric, []error) {
finder := metricDecoder{ finder := metricDecoder{
kubeMetricsImportName: metricsImportName, kubeMetricsImportName: metricsImportName,
prometheusImportName: prometheusImportName, variables: variables,
} }
ms := make([]metric, 0, len(fs)) ms := make([]metric, 0, len(fs))
errors := []error{} errors := []error{}
@ -47,7 +47,7 @@ func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName, prometheusImportNa
type metricDecoder struct { type metricDecoder struct {
kubeMetricsImportName string kubeMetricsImportName string
prometheusImportName string variables map[string]ast.Expr
} }
func (c *metricDecoder) decodeNewMetricCall(fc *ast.CallExpr) (metric, error) { func (c *metricDecoder) decodeNewMetricCall(fc *ast.CallExpr) (metric, error) {
@ -130,10 +130,11 @@ func decodeLabels(expr ast.Expr) ([]string, error) {
if !ok { if !ok {
return nil, newDecodeErrorf(bl, errLabels) return nil, newDecodeErrorf(bl, errLabels)
} }
if bl.Kind != token.STRING { value, err := stringValue(bl)
return nil, newDecodeErrorf(bl, errLabels) if err != nil {
return nil, err
} }
labels[i] = strings.Trim(bl.Value, `"`) labels[i] = value
} }
return labels, nil return labels, nil
} }
@ -160,14 +161,30 @@ func (c *metricDecoder) decodeOpts(expr ast.Expr) (metric, error) {
switch key { switch key {
case "Namespace", "Subsystem", "Name", "Help", "DeprecatedVersion": case "Namespace", "Subsystem", "Name", "Help", "DeprecatedVersion":
k, ok := kv.Value.(*ast.BasicLit) var value string
if !ok { var err error
switch v := kv.Value.(type) {
case *ast.BasicLit:
value, err = stringValue(v)
if err != nil {
return m, err
}
case *ast.Ident:
variableExpr, found := c.variables[v.Name]
if !found {
return m, newDecodeErrorf(expr, errBadVariableAttribute)
}
bl, ok := variableExpr.(*ast.BasicLit)
if !ok {
return m, newDecodeErrorf(expr, errNonStringAttribute)
}
value, err = stringValue(bl)
if err != nil {
return m, err
}
default:
return m, newDecodeErrorf(expr, errNonStringAttribute) return m, newDecodeErrorf(expr, errNonStringAttribute)
} }
if k.Kind != token.STRING {
return m, newDecodeErrorf(expr, errNonStringAttribute)
}
value := strings.Trim(k.Value, `"`)
switch key { switch key {
case "Namespace": case "Namespace":
m.Namespace = value m.Namespace = value
@ -200,6 +217,13 @@ func (c *metricDecoder) decodeOpts(expr ast.Expr) (metric, error) {
return m, nil return m, nil
} }
func stringValue(bl *ast.BasicLit) (string, error) {
if bl.Kind != token.STRING {
return "", newDecodeErrorf(bl, errNonStringAttribute)
}
return strings.Trim(bl.Value, `"`), nil
}
func (c *metricDecoder) decodeBuckets(expr ast.Expr) ([]float64, error) { func (c *metricDecoder) decodeBuckets(expr ast.Expr) ([]float64, error) {
switch v := expr.(type) { switch v := expr.(type) {
case *ast.CompositeLit: case *ast.CompositeLit:
@ -207,7 +231,7 @@ func (c *metricDecoder) decodeBuckets(expr ast.Expr) ([]float64, error) {
case *ast.SelectorExpr: case *ast.SelectorExpr:
variableName := v.Sel.String() variableName := v.Sel.String()
importName, ok := v.X.(*ast.Ident) importName, ok := v.X.(*ast.Ident)
if ok && importName.String() == c.prometheusImportName && variableName == "DefBuckets" { if ok && importName.String() == c.kubeMetricsImportName && variableName == "DefBuckets" {
return metrics.DefBuckets, nil return metrics.DefBuckets, nil
} }
case *ast.CallExpr: case *ast.CallExpr:
@ -220,7 +244,7 @@ func (c *metricDecoder) decodeBuckets(expr ast.Expr) ([]float64, error) {
if !ok { if !ok {
return nil, newDecodeErrorf(v, errBuckets) return nil, newDecodeErrorf(v, errBuckets)
} }
if functionImport.String() != c.prometheusImportName { if functionImport.String() != c.kubeMetricsImportName {
return nil, newDecodeErrorf(v, errBuckets) return nil, newDecodeErrorf(v, errBuckets)
} }
firstArg, secondArg, thirdArg, err := decodeBucketArguments(v) firstArg, secondArg, thirdArg, err := decodeBucketArguments(v)

View File

@ -29,6 +29,7 @@ const (
errStableSummary = "Stable summary metric is not supported" errStableSummary = "Stable summary metric is not supported"
errInvalidNewMetricCall = "Invalid new metric call, please ensure code compiles" errInvalidNewMetricCall = "Invalid new metric call, please ensure code compiles"
errNonStringAttribute = "Non string attribute it not supported" errNonStringAttribute = "Non string attribute it not supported"
errBadVariableAttribute = "Metric attribute was not correctly set. Please use only global consts in same file"
errFieldNotSupported = "Field %s is not supported" errFieldNotSupported = "Field %s is not supported"
errBuckets = "Buckets should be set to list of floats, result from function call of prometheus.LinearBuckets or prometheus.ExponentialBuckets" errBuckets = "Buckets should be set to list of floats, result from function call of prometheus.LinearBuckets or prometheus.ExponentialBuckets"
errLabels = "Labels were not set to list of strings" errLabels = "Labels were not set to list of strings"

View File

@ -35,9 +35,6 @@ const (
kubeMetricImportPath = `"k8s.io/component-base/metrics"` kubeMetricImportPath = `"k8s.io/component-base/metrics"`
// Should equal to final directory name of kubeMetricImportPath // Should equal to final directory name of kubeMetricImportPath
kubeMetricsDefaultImportName = "metrics" kubeMetricsDefaultImportName = "metrics"
prometheusImportPath = `"github.com/prometheus/client_golang/prometheus"`
// Should equal to final directory name of kubeMetricImportPath
prometheusDefaultImportName = "prometheus"
) )
func main() { func main() {
@ -121,13 +118,10 @@ func searchFileForStableMetrics(filename string, src interface{}) ([]metric, []e
if metricsImportName == "" { if metricsImportName == "" {
return []metric{}, []error{} return []metric{}, []error{}
} }
prometheusImportName, err := getLocalNameOfImportedPackage(tree, prometheusImportPath, prometheusDefaultImportName) variables := globalVariableDeclarations(tree)
if err != nil {
return []metric{}, addFileInformationToErrors([]error{err}, fileset)
}
stableMetricsFunctionCalls, errors := findStableMetricDeclaration(tree, metricsImportName) stableMetricsFunctionCalls, errors := findStableMetricDeclaration(tree, metricsImportName)
metrics, es := decodeMetricCalls(stableMetricsFunctionCalls, metricsImportName, prometheusImportName) metrics, es := decodeMetricCalls(stableMetricsFunctionCalls, metricsImportName, variables)
errors = append(errors, es...) errors = append(errors, es...)
return metrics, addFileInformationToErrors(errors, fileset) return metrics, addFileInformationToErrors(errors, fileset)
} }
@ -157,3 +151,21 @@ func addFileInformationToErrors(es []error, fileset *token.FileSet) []error {
} }
return es return es
} }
func globalVariableDeclarations(tree *ast.File) map[string]ast.Expr {
consts := make(map[string]ast.Expr)
for _, d := range tree.Decls {
if gd, ok := d.(*ast.GenDecl); ok && (gd.Tok == token.CONST || gd.Tok == token.VAR) {
for _, spec := range gd.Specs {
if vspec, ok := spec.(*ast.ValueSpec); ok {
for _, name := range vspec.Names {
for _, value := range vspec.Values {
consts[name.Name] = value
}
}
}
}
}
}
return consts
}

View File

@ -298,6 +298,85 @@ var _ = custom.NewCounter(
StabilityLevel: custom.STABLE, StabilityLevel: custom.STABLE,
}, },
) )
`},
{
testName: "Const",
metric: metric{
Name: "metric",
StabilityLevel: "STABLE",
Type: counterMetricType,
},
src: `
package test
import "k8s.io/component-base/metrics"
const name = "metric"
var _ = metrics.NewCounter(
&metrics.CounterOpts{
Name: name,
StabilityLevel: metrics.STABLE,
},
)
`},
{
testName: "Variable",
metric: metric{
Name: "metric",
StabilityLevel: "STABLE",
Type: counterMetricType,
},
src: `
package test
import "k8s.io/component-base/metrics"
var name = "metric"
var _ = metrics.NewCounter(
&metrics.CounterOpts{
Name: name,
StabilityLevel: metrics.STABLE,
},
)
`},
{
testName: "Multiple consts in block",
metric: metric{
Name: "metric",
StabilityLevel: "STABLE",
Type: counterMetricType,
},
src: `
package test
import "k8s.io/component-base/metrics"
const (
unrelated1 = "unrelated1"
name = "metric"
unrelated2 = "unrelated2"
)
var _ = metrics.NewCounter(
&metrics.CounterOpts{
Name: name,
StabilityLevel: metrics.STABLE,
},
)
`},
{
testName: "Multiple variables in Block",
metric: metric{
Name: "metric",
StabilityLevel: "STABLE",
Type: counterMetricType,
},
src: `
package test
import "k8s.io/component-base/metrics"
var (
unrelated1 = "unrelated1"
name = "metric"
_ = metrics.NewCounter(
&metrics.CounterOpts{
Name: name,
StabilityLevel: metrics.STABLE,
},
)
)
`}, `},
{ {
testName: "Histogram with linear buckets", testName: "Histogram with linear buckets",
@ -310,12 +389,11 @@ var _ = custom.NewCounter(
src: ` src: `
package test package test
import "k8s.io/component-base/metrics" import "k8s.io/component-base/metrics"
import "github.com/prometheus/client_golang/prometheus"
var _ = metrics.NewHistogram( var _ = metrics.NewHistogram(
&metrics.HistogramOpts{ &metrics.HistogramOpts{
Name: "histogram", Name: "histogram",
StabilityLevel: metrics.STABLE, StabilityLevel: metrics.STABLE,
Buckets: prometheus.LinearBuckets(1, 1, 3), Buckets: metrics.LinearBuckets(1, 1, 3),
}, },
) )
`}, `},
@ -330,12 +408,11 @@ var _ = metrics.NewHistogram(
src: ` src: `
package test package test
import "k8s.io/component-base/metrics" import "k8s.io/component-base/metrics"
import "github.com/prometheus/client_golang/prometheus"
var _ = metrics.NewHistogram( var _ = metrics.NewHistogram(
&metrics.HistogramOpts{ &metrics.HistogramOpts{
Name: "histogram", Name: "histogram",
StabilityLevel: metrics.STABLE, StabilityLevel: metrics.STABLE,
Buckets: prometheus.ExponentialBuckets(1, 2, 3), Buckets: metrics.ExponentialBuckets(1, 2, 3),
}, },
) )
`}, `},
@ -350,12 +427,11 @@ var _ = metrics.NewHistogram(
src: ` src: `
package test package test
import "k8s.io/component-base/metrics" import "k8s.io/component-base/metrics"
import "github.com/prometheus/client_golang/prometheus"
var _ = metrics.NewHistogram( var _ = metrics.NewHistogram(
&metrics.HistogramOpts{ &metrics.HistogramOpts{
Name: "histogram", Name: "histogram",
StabilityLevel: metrics.STABLE, StabilityLevel: metrics.STABLE,
Buckets: prometheus.DefBuckets, Buckets: metrics.DefBuckets,
}, },
) )
`}, `},
@ -397,15 +473,14 @@ var _ = metrics.NewSummary(
) )
`}, `},
{ {
testName: "Fail on stable metric with attribute set to variable", testName: "Fail on stable metric with attribute set to unknown variable",
err: fmt.Errorf("testdata/metric.go:7:4: Non string attribute it not supported"), err: fmt.Errorf("testdata/metric.go:6:4: Metric attribute was not correctly set. Please use only global consts in same file"),
src: ` src: `
package test package test
import "k8s.io/component-base/metrics" import "k8s.io/component-base/metrics"
const name = "metric"
var _ = metrics.NewCounter( var _ = metrics.NewCounter(
&metrics.CounterOpts{ &metrics.CounterOpts{
Name: name, Name: unknownVariable,
StabilityLevel: metrics.STABLE, StabilityLevel: metrics.STABLE,
}, },
) )