diff --git a/test/instrumentation/error.go b/test/instrumentation/error.go index 0443f4bfefb..a9960713310 100644 --- a/test/instrumentation/error.go +++ b/test/instrumentation/error.go @@ -29,7 +29,7 @@ const ( errInvalidNewMetricCall = "Invalid new metric call, please ensure code compiles" errNonStringAttribute = "Non string attribute is not supported" errBadVariableAttribute = "Metric attribute was not correctly set. Please use only global consts in same file" - errBadImportedVariableAttribute = "Metric attribute was not correctly set. Please use only global consts in correctly impoprted same file" + errBadImportedVariableAttribute = "Metric attribute was not correctly set. Please use only global consts in correctly imported same file" 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" errObjectives = "Objectives should be set to map of floats to floats" diff --git a/test/instrumentation/main.go b/test/instrumentation/main.go index 3cd12bb1a93..1df7b3a7632 100644 --- a/test/instrumentation/main.go +++ b/test/instrumentation/main.go @@ -18,14 +18,17 @@ package main import ( "bufio" + "encoding/json" "flag" "fmt" "go/ast" "go/parser" "go/token" "os" + "os/exec" "path/filepath" "sort" + "strconv" "strings" "gopkg.in/yaml.v2" @@ -35,18 +38,26 @@ const ( kubeMetricImportPath = `"k8s.io/component-base/metrics"` // Should equal to final directory name of kubeMetricImportPath kubeMetricsDefaultImportName = "metrics" - - kubeURLRoot = "k8s.io/kubernetes/" ) var ( // env configs - GOROOT string = os.Getenv("GOROOT") - GOOS string = os.Getenv("GOOS") - KUBE_ROOT string = os.Getenv("KUBE_ROOT") + GOOS string = findGOOS() ALL_STABILITY_CLASSES bool ) +func findGOOS() string { + cmd := exec.Command("go", "env", "GOOS") + out, err := cmd.CombinedOutput() + if err != nil { + panic(fmt.Sprintf("running `go env` failed: %v\n\n%s", err, string(out))) + } + if len(out) == 0 { + panic("empty result from `go env GOOS`") + } + return string(out) +} + func main() { flag.BoolVar(&ALL_STABILITY_CLASSES, "allstabilityclasses", false, "use this flag to enable all stability classes") @@ -202,33 +213,24 @@ func globalVariableDeclarations(tree *ast.File) map[string]ast.Expr { return consts } -func localImportPath(importExpr string) (string, error) { - // parse directory path - var pathPrefix string - if strings.Contains(importExpr, kubeURLRoot) { - // search k/k local checkout - pathPrefix = KUBE_ROOT - importExpr = strings.Replace(importExpr, kubeURLRoot, "", 1) - } else if strings.Contains(importExpr, "k8s.io/klog/v2") || strings.Contains(importExpr, "k8s.io/util") { - pathPrefix = strings.Join([]string{KUBE_ROOT, "vendor"}, string(os.PathSeparator)) - } else if strings.Contains(importExpr, "k8s.io/") { - // search k/k/staging local checkout - pathPrefix = strings.Join([]string{KUBE_ROOT, "staging", "src"}, string(os.PathSeparator)) - } else if strings.Contains(importExpr, ".") { - // not stdlib -> prefix with GOMODCACHE - // pathPrefix = strings.Join([]string{KUBE_ROOT, "vendor"}, string(os.PathSeparator)) +func findPkgDir(pkg string) (string, error) { + // Use Go's module mechanism. + cmd := exec.Command("go", "list", "-find", "-json=Dir", pkg) + out, err := cmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("running `go list` failed: %w\n\n%s", err, string(out)) + } + result := struct { + Dir string + }{} + if err := json.Unmarshal(out, &result); err != nil { + return "", fmt.Errorf("json unmarshal of `go list` failed: %w", err) + } + if result.Dir != "" { + return result.Dir, nil + } - // this requires implementing SIV, skip for now - return "", fmt.Errorf("unable to handle general, non STL imports for metric analysis. import path: %s", importExpr) - } else { - // stdlib -> prefix with GOROOT - pathPrefix = strings.Join([]string{GOROOT, "src"}, string(os.PathSeparator)) - } // ToDo: support non go mod - - crossPlatformImportExpr := strings.Replace(importExpr, "/", string(os.PathSeparator), -1) - importDirectory := strings.Join([]string{pathPrefix, strings.Trim(crossPlatformImportExpr, "\"")}, string(os.PathSeparator)) - - return importDirectory, nil + return "", fmt.Errorf("empty respose from `go list`") } func importedGlobalVariableDeclaration(localVariables map[string]ast.Expr, imports []*ast.ImportSpec) (map[string]ast.Expr, error) { @@ -243,17 +245,18 @@ func importedGlobalVariableDeclaration(localVariables map[string]ast.Expr, impor } // find local path on disk for listed import - importDirectory, err := localImportPath(im.Path.Value) + pkg, err := strconv.Unquote(im.Path.Value) if err != nil { - // uncomment the below log line if you want to start using non k8s/non stl libs for resolving const/var in metric definitions - // fmt.Fprint(os.Stderr, err.Error() + "\n") - continue + return nil, fmt.Errorf("can't handle import '%s': %w", im.Path.Value, err) + } + importDirectory, err := findPkgDir(pkg) + if err != nil { + return nil, fmt.Errorf("can't find import '%s': %w", im.Path.Value, err) } files, err := os.ReadDir(importDirectory) if err != nil { - fmt.Fprintf(os.Stderr, "failed to read import path directory %s with error %s, skipping\n", importDirectory, err) - continue + return nil, fmt.Errorf("failed to read import directory %s: %w", importDirectory, err) } for _, file := range files { diff --git a/test/instrumentation/main_test.go b/test/instrumentation/main_test.go index 2b1f12c7eeb..be1e2b8179d 100644 --- a/test/instrumentation/main_test.go +++ b/test/instrumentation/main_test.go @@ -18,9 +18,7 @@ package main import ( "fmt" - "os" "reflect" - "strings" "testing" "github.com/google/go-cmp/cmp" @@ -121,16 +119,10 @@ var _ = NewCounter( } func TestStableMetric(t *testing.T) { - wd, err := os.Getwd() - if err != nil { - t.Fatalf("unable to fetch path to testing package - needed for simulating import path tests") - } - for _, test := range []struct { testName string src string metric metric - kubeRoot string }{ { testName: "Counter", @@ -459,26 +451,6 @@ var _ = metrics.NewHistogram( Buckets: metrics.DefBuckets, }, ) -`}, - { - testName: "Imported stdlib constant", - metric: metric{ - Name: "importedCounter", - StabilityLevel: "STABLE", - Subsystem: "GET", - Type: counterMetricType, - }, - src: ` -package test -import "k8s.io/component-base/metrics" -import "net/http" -var _ = metrics.NewCounter( - &metrics.CounterOpts{ - Name: "importedCounter", - StabilityLevel: metrics.STABLE, - Subsystem: http.MethodGet, - }, - ) `}, { testName: "Imported k8s.io constant", @@ -488,7 +460,6 @@ var _ = metrics.NewCounter( Subsystem: "kubelet", Type: counterMetricType, }, - kubeRoot: strings.Join([]string{wd, "testdata"}, string(os.PathSeparator)), src: ` package test import compbasemetrics "k8s.io/component-base/metrics" @@ -500,39 +471,9 @@ var _ = compbasemetrics.NewCounter( Subsystem: metrics.KubeletSubsystem, }, ) -`}, - { - testName: "Imported k8s.io/staging constant", - metric: metric{ - Name: "importedCounter", - StabilityLevel: "STABLE", - Subsystem: "ThisIsNotTheSoundOfTheTrain", - Type: counterMetricType, - }, - kubeRoot: strings.Join([]string{wd, "testdata"}, string(os.PathSeparator)), - src: ` -package test -import compbasemetrics "k8s.io/component-base/metrics" -import "k8s.io/metrics" -var _ = compbasemetrics.NewCounter( - &compbasemetrics.CounterOpts{ - Name: "importedCounter", - StabilityLevel: compbasemetrics.STABLE, - Subsystem: metrics.OKGO, - }, - ) `}, } { t.Run(test.testName, func(t *testing.T) { - // these sub-tests cannot be run in parallel with the below - if test.kubeRoot != "" { - priorKRoot := KUBE_ROOT - KUBE_ROOT = test.kubeRoot - defer func() { - KUBE_ROOT = priorKRoot - }() - } - metrics, errors := searchFileForStableMetrics(fakeFilename, test.src) if len(errors) != 0 { t.Errorf("Unexpected errors: %s", errors) @@ -591,10 +532,10 @@ var _ = metrics.NewCounter( src: ` package test import "k8s.io/component-base/metrics" -import "k8s.io/kubernetes/utils" +import "os" var _ = metrics.NewCounter( &metrics.CounterOpts{ - Name: utils.getMetricName(), + Name: os.Getenv("name"), // any imported function will do StabilityLevel: metrics.STABLE, }, ) @@ -724,7 +665,7 @@ var _ = metrics.NewSummary( src: ` package test import "k8s.io/component-base/metrics" -import "github.com/fake_prometheus/prometheus" +import "github.com/prometheus/client_golang/prometheus" var _ = metrics.NewHistogram( &metrics.HistogramOpts{ Name: "histogram", @@ -745,56 +686,3 @@ var _ = metrics.NewHistogram( }) } } - -func Test_localImportPath(t *testing.T) { - KUBE_ROOT = "/home/pchristopher/go/src/k8s.io/kubernetes" - GOROOT := os.Getenv("GOROOT") - - for _, test := range []struct { - name string - importExpr string - expectedPath string - errorExp bool - }{ - { - name: "k8s local package", - importExpr: "k8s.io/kubernetes/pkg/kubelet/metrics", - expectedPath: strings.Join([]string{KUBE_ROOT, "pkg", "kubelet", "metrics"}, string(os.PathSeparator)), - errorExp: false, - }, - { - name: "k8s staging package", - importExpr: "k8s.io/kubelet/metrics", - expectedPath: strings.Join([]string{KUBE_ROOT, "staging", "src", "k8s.io", "kubelet", "metrics"}, string(os.PathSeparator)), - errorExp: false, - }, - { - name: "public package", - importExpr: "github.com/thisisnot/thesoundofthetrain", - errorExp: true, - }, - { - name: "stl package", - importExpr: "os", - expectedPath: strings.Join([]string{GOROOT, "src", "os"}, string(os.PathSeparator)), - errorExp: false, - }, - } { - t.Run(test.name, func(t *testing.T) { - path, err := localImportPath(test.importExpr) - if test.errorExp { - if err == nil { - t.Error("did not receive error as expected") - } - } else { - if err != nil { - t.Errorf("received unexpected error %s", err) - } - } - - if path != test.expectedPath { - t.Errorf("did not received expected path. \nwant: %s \ngot: %s", test.expectedPath, path) - } - }) - } -}