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

Finish implementation of stable metrics static analysis #81510

Merged
merged 1 commit into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion hack/.staticcheck_failures
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ test/images/agnhost/pause
test/images/agnhost/serve-hostname
test/images/agnhost/webhook
test/images/pets/peer-finder
test/instrumentation
test/integration/apiserver
test/integration/apiserver/admissionwebhook
test/integration/auth
Expand Down
1 change: 1 addition & 0 deletions test/instrumentation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ go_test(
srcs = ["main_test.go"],
data = glob(["testdata/**"]),
embed = [":go_default_library"],
deps = ["//vendor/github.com/prometheus/client_golang/prometheus:go_default_library"],
)
95 changes: 82 additions & 13 deletions test/instrumentation/decode_metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ import (
"strconv"
"strings"

"github.com/prometheus/client_golang/prometheus"
"k8s.io/component-base/metrics"
)

func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName string) ([]metric, []error) {
func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName, prometheusImportName string) ([]metric, []error) {
finder := metricDecoder{
metricsImportName: metricsImportName,
kubeMetricsImportName: metricsImportName,
prometheusImportName: prometheusImportName,
}
ms := make([]metric, 0, len(fs))
errors := []error{}
Expand All @@ -45,7 +47,8 @@ func decodeMetricCalls(fs []*ast.CallExpr, metricsImportName string) ([]metric,
}

type metricDecoder struct {
metricsImportName string
kubeMetricsImportName string
prometheusImportName string
}

func (c *metricDecoder) decodeNewMetricCall(fc *ast.CallExpr) (metric, error) {
Expand All @@ -60,7 +63,7 @@ func (c *metricDecoder) decodeNewMetricCall(fc *ast.CallExpr) (metric, error) {
if !ok {
return m, newDecodeErrorf(fc, errNotDirectCall)
}
if functionImport.String() != c.metricsImportName {
if functionImport.String() != c.kubeMetricsImportName {
return m, newDecodeErrorf(fc, errNotDirectCall)
}
switch functionName {
Expand Down Expand Up @@ -157,7 +160,7 @@ func (c *metricDecoder) decodeOpts(expr ast.Expr) (metric, error) {
key := fmt.Sprintf("%v", kv.Key)

switch key {
case "Namespace", "Subsystem", "Name", "Help":
case "Namespace", "Subsystem", "Name", "Help", "DeprecatedVersion":
k, ok := kv.Value.(*ast.BasicLit)
if !ok {
return m, newDecodeErrorf(expr, errNonStringAttribute)
Expand All @@ -173,18 +176,20 @@ func (c *metricDecoder) decodeOpts(expr ast.Expr) (metric, error) {
m.Subsystem = value
case "Name":
m.Name = value
case "DeprecatedVersion":
m.DeprecatedVersion = value
case "Help":
m.Help = value
}
case "Buckets":
buckets, err := decodeBuckets(kv)
buckets, err := c.decodeBuckets(kv.Value)
if err != nil {
return m, err
}
sort.Float64s(buckets)
m.Buckets = buckets
case "StabilityLevel":
level, err := decodeStabilityLevel(kv.Value, c.metricsImportName)
level, err := decodeStabilityLevel(kv.Value, c.kubeMetricsImportName)
if err != nil {
return m, err
}
Expand All @@ -196,13 +201,46 @@ func (c *metricDecoder) decodeOpts(expr ast.Expr) (metric, error) {
return m, nil
}

func decodeBuckets(kv *ast.KeyValueExpr) ([]float64, error) {
cl, ok := kv.Value.(*ast.CompositeLit)
if !ok {
return nil, newDecodeErrorf(kv, errBuckets)
func (c *metricDecoder) decodeBuckets(expr ast.Expr) ([]float64, error) {
switch v := expr.(type) {
case *ast.CompositeLit:
return decodeListOfFloats(v.Elts)
case *ast.SelectorExpr:
variableName := v.Sel.String()
importName, ok := v.X.(*ast.Ident)
if ok && importName.String() == c.prometheusImportName && variableName == "DefBuckets" {
return prometheus.DefBuckets, nil
}
case *ast.CallExpr:
se, ok := v.Fun.(*ast.SelectorExpr)
if !ok {
return nil, newDecodeErrorf(v, errBuckets)
}
functionName := se.Sel.String()
functionImport, ok := se.X.(*ast.Ident)
if !ok {
return nil, newDecodeErrorf(v, errBuckets)
}
if functionImport.String() != c.prometheusImportName {
return nil, newDecodeErrorf(v, errBuckets)
}
firstArg, secondArg, thirdArg, err := decodeBucketArguments(v)
if err != nil {
return nil, err
}
switch functionName {
case "LinearBuckets":
return prometheus.LinearBuckets(firstArg, secondArg, thirdArg), nil
case "ExponentialBuckets":
return prometheus.ExponentialBuckets(firstArg, secondArg, thirdArg), nil
}
}
buckets := make([]float64, len(cl.Elts))
for i, elt := range cl.Elts {
return nil, newDecodeErrorf(expr, errBuckets)
}

func decodeListOfFloats(exprs []ast.Expr) ([]float64, error) {
buckets := make([]float64, len(exprs))
for i, elt := range exprs {
bl, ok := elt.(*ast.BasicLit)
if !ok {
return nil, newDecodeErrorf(bl, errBuckets)
Expand All @@ -219,6 +257,37 @@ func decodeBuckets(kv *ast.KeyValueExpr) ([]float64, error) {
return buckets, nil
}

func decodeBucketArguments(fc *ast.CallExpr) (float64, float64, int, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the return args supposed to represent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decode arguments for prometheus.LinearBuckets and prometheus.ExponentialBuckets(float64, float64, int)

if len(fc.Args) != 3 {
return 0, 0, 0, newDecodeErrorf(fc, errBuckets)
}
strArgs := make([]string, len(fc.Args))
for i, elt := range fc.Args {
bl, ok := elt.(*ast.BasicLit)
if !ok {
return 0, 0, 0, newDecodeErrorf(bl, errBuckets)
}
if bl.Kind != token.FLOAT && bl.Kind != token.INT {
return 0, 0, 0, newDecodeErrorf(bl, errBuckets)
}
strArgs[i] = bl.Value
}
firstArg, err := strconv.ParseFloat(strArgs[0], 64)
if err != nil {
return 0, 0, 0, newDecodeErrorf(fc.Args[0], errBuckets)
}
secondArg, err := strconv.ParseFloat(strArgs[1], 64)
if err != nil {
return 0, 0, 0, newDecodeErrorf(fc.Args[1], errBuckets)
}
thirdArg, err := strconv.ParseInt(strArgs[2], 10, 64)
if err != nil {
return 0, 0, 0, newDecodeErrorf(fc.Args[2], errBuckets)
}

return firstArg, secondArg, int(thirdArg), nil
}

func decodeStabilityLevel(expr ast.Expr, metricsFrameworkImportName string) (*metrics.StabilityLevel, error) {
se, ok := expr.(*ast.SelectorExpr)
if !ok {
Expand Down
4 changes: 2 additions & 2 deletions test/instrumentation/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ const (
errInvalidNewMetricCall = "Invalid new metric call, please ensure code compiles"
errNonStringAttribute = "Non string attribute it not supported"
errFieldNotSupported = "Field %s is not supported"
errBuckets = "Buckets were not set to list of floats"
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"
errImport = `Importing through "." metrics framework is not supported`
errImport = `Importing using "." is not supported`
)

type decodeError struct {
Expand Down
29 changes: 18 additions & 11 deletions test/instrumentation/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ import (
)

const (
metricFrameworkPath = `"k8s.io/kubernetes/staging/src/k8s.io/component-base/metrics"`
// Should equal to final directory name of metricFrameworkPath
defaultFrameworkImportName = "metrics"
kubeMetricImportPath = `"k8s.io/component-base/metrics"`
// Should equal to final directory name of kubeMetricImportPath
kubeMetricsDefaultImportName = "metrics"
prometheusImportPath = `"github.com/prometheus/client_golang/prometheus"`
// Should equal to final directory name of kubeMetricImportPath
prometheusDefaultImportName = "prometheus"
)

func main() {
Expand Down Expand Up @@ -67,7 +70,7 @@ func main() {
}

func searchPathForStableMetrics(path string) ([]metric, []error) {
ms := []metric{}
metrics := []metric{}
errors := []error{}
err := filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
if strings.HasPrefix(path, "vendor") {
Expand All @@ -78,13 +81,13 @@ func searchPathForStableMetrics(path string) ([]metric, []error) {
}
ms, es := searchFileForStableMetrics(path, nil)
errors = append(errors, es...)
ms = append(ms, ms...)
metrics = append(metrics, ms...)
return nil
})
if err != nil {
errors = append(errors, err)
}
return ms, errors
return metrics, errors
}

// Pass either only filename of existing file or src including source code in any format and a filename that it comes from
Expand All @@ -94,26 +97,30 @@ func searchFileForStableMetrics(filename string, src interface{}) ([]metric, []e
if err != nil {
return []metric{}, []error{err}
}
metricsImportName, err := getMetricsFrameworkImportName(tree)
metricsImportName, err := getLocalNameOfImportedPackage(tree, kubeMetricImportPath, kubeMetricsDefaultImportName)
if err != nil {
return []metric{}, addFileInformationToErrors([]error{err}, fileset)
}
if metricsImportName == "" {
return []metric{}, []error{}
}
prometheusImportName, err := getLocalNameOfImportedPackage(tree, prometheusImportPath, prometheusDefaultImportName)
if err != nil {
return []metric{}, addFileInformationToErrors([]error{err}, fileset)
}

stableMetricsFunctionCalls, errors := findStableMetricDeclaration(tree, metricsImportName)
metrics, es := decodeMetricCalls(stableMetricsFunctionCalls, metricsImportName)
metrics, es := decodeMetricCalls(stableMetricsFunctionCalls, metricsImportName, prometheusImportName)
errors = append(errors, es...)
return metrics, addFileInformationToErrors(errors, fileset)
}

func getMetricsFrameworkImportName(tree *ast.File) (string, error) {
func getLocalNameOfImportedPackage(tree *ast.File, importPath, defaultImportName string) (string, error) {
var importName string
for _, im := range tree.Imports {
if im.Path.Value == metricFrameworkPath {
if im.Path.Value == importPath {
if im.Name == nil {
importName = defaultFrameworkImportName
importName = defaultImportName
} else {
if im.Name.Name == "." {
return "", newDecodeErrorf(im, errImport)
Expand Down