diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b0270833..1d3a1480 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,6 +18,7 @@ jobs: run: | go get -t -v ./... go test -v -race ./... + cd hack/tools && go test -v -race ./... lint: runs-on: ubuntu-latest steps: diff --git a/hack/tools/go.mod b/hack/tools/go.mod new file mode 100644 index 00000000..23cf684c --- /dev/null +++ b/hack/tools/go.mod @@ -0,0 +1,5 @@ +module k8s.io/klog/hack/tools + +go 1.15 + +require golang.org/x/tools v0.1.0 diff --git a/hack/tools/go.sum b/hack/tools/go.sum new file mode 100644 index 00000000..21d696a6 --- /dev/null +++ b/hack/tools/go.sum @@ -0,0 +1,26 @@ +github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= +golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= +golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY= +golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/hack/tools/logcheck/README.md b/hack/tools/logcheck/README.md new file mode 100644 index 00000000..008231cc --- /dev/null +++ b/hack/tools/logcheck/README.md @@ -0,0 +1,5 @@ +This directory contains tool for checking use of unstructured logs in a package. It is created to prevent regression after packages have been migrated to use structured logs. + +**Installation:**`go install k8s.io/klog/hack/tools/logcheck` +**Usage:** `$logcheck.go ` +`e.g $logcheck ./pkg/kubelet/lifecycle/` diff --git a/hack/tools/logcheck/main.go b/hack/tools/logcheck/main.go new file mode 100644 index 00000000..496c5d67 --- /dev/null +++ b/hack/tools/logcheck/main.go @@ -0,0 +1,112 @@ +/* +Copyright 2021 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 ( + "fmt" + "go/ast" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/singlechecker" +) + +// Doc explaining the tool. +const Doc = "Tool to check use of unstructured logging patterns." + +// Analyzer runs static analysis. +var Analyzer = &analysis.Analyzer{ + Name: "logcheck", + Doc: Doc, + Run: run, +} + +func main() { + singlechecker.Main(Analyzer) +} + +func run(pass *analysis.Pass) (interface{}, error) { + + for _, file := range pass.Files { + + ast.Inspect(file, func(n ast.Node) bool { + + // We are intrested in function calls, as we want to detect klog.* calls + // passing all function calls to checkForFunctionExpr + if fexpr, ok := n.(*ast.CallExpr); ok { + + checkForFunctionExpr(fexpr.Fun, pass) + } + + return true + }) + } + return nil, nil +} + +// checkForFunctionExpr checks for unstructured logging function, prints error if found any. +func checkForFunctionExpr(fun ast.Expr, pass *analysis.Pass) { + + /* we are extracting external package function calls e.g. klog.Infof fmt.Printf + and eliminating calls like setLocalHost() + basically function calls that has selector expression like . + */ + if selExpr, ok := fun.(*ast.SelectorExpr); ok { + // extracting function Name like Infof + fName := selExpr.Sel.Name + + // for nested function cases klog.V(1).Infof scenerios + // if selExpr.X contains one more caller expression which is selector expression + // we are extracting klog and discarding V(1) + if n, ok := selExpr.X.(*ast.CallExpr); ok { + if _, ok = n.Fun.(*ast.SelectorExpr); ok { + selExpr = n.Fun.(*ast.SelectorExpr) + } + } + + // extracting package name + pName, ok := selExpr.X.(*ast.Ident) + + // Matching if package name is klog and any unstructured logging function is used. + if ok && pName.Name == "klog" && isUnstructured((fName)) { + + msg := fmt.Sprintf("unstructured logging function %q should not be used", fName) + pass.Report(analysis.Diagnostic{ + Pos: fun.Pos(), + Message: msg, + }) + } + } +} + +func isUnstructured(fName string) bool { + + // List of klog functions we do not want to use after migration to structured logging. + unstrucured := []string{ + "Infof", "Info", "Infoln", "InfoDepth", + "Warning", "Warningf", "Warningln", "WarningDepth", + "Error", "Errorf", "Errorln", "ErrorDepth", + "Fatal", "Fatalf", "Fatalln", "FatalDepth", + } + + for _, name := range unstrucured { + if fName == name { + return true + } + } + + return false +} diff --git a/hack/tools/logcheck/main_test.go b/hack/tools/logcheck/main_test.go new file mode 100644 index 00000000..b41f541b --- /dev/null +++ b/hack/tools/logcheck/main_test.go @@ -0,0 +1,27 @@ +/* +Copyright 2021 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 ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), Analyzer) +} diff --git a/hack/tools/logcheck/testdata/data.go b/hack/tools/logcheck/testdata/data.go new file mode 100644 index 00000000..1409d415 --- /dev/null +++ b/hack/tools/logcheck/testdata/data.go @@ -0,0 +1,47 @@ +/* +Copyright 2021 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. +*/ + +// test file for unstructured logging static check tool unit tests. + +package testdata + +import ( + klog "k8s.io/klog/v2" +) + +func printUnstructuredLog() { + klog.V(1).Infof("test log") // want `unstructured logging function "Infof" should not be used` + klog.Infof("test log") // want `unstructured logging function "Infof" should not be used` + klog.Info("test log") // want `unstructured logging function "Info" should not be used` + klog.Infoln("test log") // want `unstructured logging function "Infoln" should not be used` + klog.InfoDepth(1, "test log") // want `unstructured logging function "InfoDepth" should not be used` + klog.Warning("test log") // want `unstructured logging function "Warning" should not be used` + klog.Warningf("test log") // want `unstructured logging function "Warningf" should not be used` + klog.WarningDepth(1, "test log") // want `unstructured logging function "WarningDepth" should not be used` + klog.Error("test log") // want `unstructured logging function "Error" should not be used` + klog.Errorf("test log") // want `unstructured logging function "Errorf" should not be used` + klog.Errorln("test log") // want `unstructured logging function "Errorln" should not be used` + klog.ErrorDepth(1, "test log") // want `unstructured logging function "ErrorDepth" should not be used` + klog.Fatal("test log") // want `unstructured logging function "Fatal" should not be used` + klog.Fatalf("test log") // want `unstructured logging function "Fatalf" should not be used` + klog.Fatalln("test log") // want `unstructured logging function "Fatalln" should not be used` + klog.FatalDepth(1, "test log") // want `unstructured logging function "FatalDepth" should not be used` +} + +func printStructuredLog() { + klog.InfoS("test log") + klog.ErrorS(nil, "test log") +} diff --git a/hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go b/hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go new file mode 100644 index 00000000..63bfafec --- /dev/null +++ b/hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go @@ -0,0 +1,182 @@ +/* +Copyright 2021 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. +*/ + +// This fake package is created as package golang.org/x/tools/go/analysis/analysistest +// expects test data dependency to be testdata/src + +package v2 + +// Verbose is a boolean type that implements Infof (like Printf) etc. +// See the documentation of V for more information. +type Verbose struct { + enabled bool +} + +type Level int32 + +func V(level Level) Verbose { + + return Verbose{enabled: false} +} + +// Info is equivalent to the global Info function, guarded by the value of v. +// See the documentation of V for usage. +func (v Verbose) Info(args ...interface{}) { + +} + +// Infoln is equivalent to the global Infoln function, guarded by the value of v. +// See the documentation of V for usage. +func (v Verbose) Infoln(args ...interface{}) { + +} + +// Infof is equivalent to the global Infof function, guarded by the value of v. +// See the documentation of V for usage. +func (v Verbose) Infof(format string, args ...interface{}) { + +} + +// InfoS is equivalent to the global InfoS function, guarded by the value of v. +// See the documentation of V for usage. +func (v Verbose) InfoS(msg string, keysAndValues ...interface{}) { + +} + +func InfoSDepth(depth int, msg string, keysAndValues ...interface{}) { +} + +// Deprecated: Use ErrorS instead. +func (v Verbose) Error(err error, msg string, args ...interface{}) { + +} + +// ErrorS is equivalent to the global Error function, guarded by the value of v. +// See the documentation of V for usage. +func (v Verbose) ErrorS(err error, msg string, keysAndValues ...interface{}) { + +} + +// Info logs to the INFO log. +// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. +func Info(args ...interface{}) { +} + +// InfoDepth acts as Info but uses depth to determine which call frame to log. +// InfoDepth(0, "msg") is the same as Info("msg"). +func InfoDepth(depth int, args ...interface{}) { +} + +// Infoln logs to the INFO log. +// Arguments are handled in the manner of fmt.Println; a newline is always appended. +func Infoln(args ...interface{}) { +} + +// Infof logs to the INFO log. +// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. +func Infof(format string, args ...interface{}) { +} + +// InfoS structured logs to the INFO log. +// The msg argument used to add constant description to the log line. +// The key/value pairs would be join by "=" ; a newline is always appended. +// +// Basic examples: +// >> klog.InfoS("Pod status updated", "pod", "kubedns", "status", "ready") +// output: +// >> I1025 00:15:15.525108 1 controller_utils.go:116] "Pod status updated" pod="kubedns" status="ready" +func InfoS(msg string, keysAndValues ...interface{}) { +} + +// Warning logs to the WARNING and INFO logs. +// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. +func Warning(args ...interface{}) { +} + +// WarningDepth acts as Warning but uses depth to determine which call frame to log. +// WarningDepth(0, "msg") is the same as Warning("msg"). +func WarningDepth(depth int, args ...interface{}) { +} + +// Warningln logs to the WARNING and INFO logs. +// Arguments are handled in the manner of fmt.Println; a newline is always appended. +func Warningln(args ...interface{}) { +} + +// Warningf logs to the WARNING and INFO logs. +// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. +func Warningf(format string, args ...interface{}) { +} + +// Error logs to the ERROR, WARNING, and INFO logs. +// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. +func Error(args ...interface{}) { +} + +// ErrorDepth acts as Error but uses depth to determine which call frame to log. +// ErrorDepth(0, "msg") is the same as Error("msg"). +func ErrorDepth(depth int, args ...interface{}) { +} + +// Errorln logs to the ERROR, WARNING, and INFO logs. +// Arguments are handled in the manner of fmt.Println; a newline is always appended. +func Errorln(args ...interface{}) { +} + +// Errorf logs to the ERROR, WARNING, and INFO logs. +// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. +func Errorf(format string, args ...interface{}) { +} + +// ErrorS structured logs to the ERROR, WARNING, and INFO logs. +// the err argument used as "err" field of log line. +// The msg argument used to add constant description to the log line. +// The key/value pairs would be join by "=" ; a newline is always appended. +// +// Basic examples: +// >> klog.ErrorS(err, "Failed to update pod status") +// output: +// >> E1025 00:15:15.525108 1 controller_utils.go:114] "Failed to update pod status" err="timeout" +func ErrorS(err error, msg string, keysAndValues ...interface{}) { +} + +// ErrorSDepth acts as ErrorS but uses depth to determine which call frame to log. +// ErrorSDepth(0, "msg") is the same as ErrorS("msg"). +func ErrorSDepth(depth int, err error, msg string, keysAndValues ...interface{}) { +} + +// Fatal logs to the FATAL, ERROR, WARNING, and INFO logs, +// including a stack trace of all running goroutines, then calls os.Exit(255). +// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. +func Fatal(args ...interface{}) { +} + +// FatalDepth acts as Fatal but uses depth to determine which call frame to log. +// FatalDepth(0, "msg") is the same as Fatal("msg"). +func FatalDepth(depth int, args ...interface{}) { +} + +// Fatalln logs to the FATAL, ERROR, WARNING, and INFO logs, +// including a stack trace of all running goroutines, then calls os.Exit(255). +// Arguments are handled in the manner of fmt.Println; a newline is always appended. +func Fatalln(args ...interface{}) { +} + +// Fatalf logs to the FATAL, ERROR, WARNING, and INFO logs, +// including a stack trace of all running goroutines, then calls os.Exit(255). +// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. +func Fatalf(format string, args ...interface{}) { +}