Skip to content

Commit

Permalink
add klog.v(0) and logger.v(0) check
Browse files Browse the repository at this point in the history
  • Loading branch information
yangjunmyfm192085 committed Aug 27, 2022
1 parent 601985d commit 038c0b9
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 0 deletions.
8 changes: 8 additions & 0 deletions logcheck/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ func TestAnalyzer(t *testing.T) {
},
testPackage: "helpers",
},
{
name: "Do not allow Verbose Zero logs",
enabled: map[string]string{
"structured": "false",
"verbose-zero": "true",
},
testPackage: "doNotAllowVerboseZeroLogs",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down
46 changes: 46 additions & 0 deletions logcheck/pkg/logcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const (
parametersCheck = "parameters"
contextualCheck = "contextual"
withHelpersCheck = "with-helpers"
verboseZeroCheck = "verbose-zero"
)

type checks map[string]*bool
Expand All @@ -57,6 +58,7 @@ func Analyser() *analysis.Analyzer {
parametersCheck: new(bool),
contextualCheck: new(bool),
withHelpersCheck: new(bool),
verboseZeroCheck: new(bool),
},
}
c.fileOverrides.validChecks = map[string]bool{}
Expand All @@ -70,6 +72,7 @@ klog methods (Info, Infof, Error, Errorf, Warningf, etc).`)
logcheckFlags.BoolVar(c.enabled[parametersCheck], prefix+parametersCheck, true, `When true, logcheck will check parameters of structured logging calls.`)
logcheckFlags.BoolVar(c.enabled[contextualCheck], prefix+contextualCheck, false, `When true, logcheck will only allow log calls for contextual logging (retrieving a Logger from klog or the context and logging through that) and warn about all others.`)
logcheckFlags.BoolVar(c.enabled[withHelpersCheck], prefix+withHelpersCheck, false, `When true, logcheck will warn about direct calls to WithName, WithValues and NewContext.`)
logcheckFlags.BoolVar(c.enabled[verboseZeroCheck], prefix+verboseZeroCheck, false, `When true, logcheck will check whether verbose is 0.`)
logcheckFlags.Var(&c.fileOverrides, "config", `A file which overrides the global settings for checks on a per-file basis via regular expressions.`)

// Use env variables as defaults. This is necessary when used as plugin
Expand Down Expand Up @@ -172,6 +175,11 @@ func checkForFunctionExpr(fexpr *ast.CallExpr, pass *analysis.Pass, c *config) {
checkForFormatSpecifier(fexpr, pass)
}
}
// verbose Zero Check
if c.isEnabled(verboseZeroCheck, filename) {
checkForVerboseZero(fexpr, pass)
}

} else if isGoLogger(selExpr.X, pass) {
if c.isEnabled(parametersCheck, filename) {
checkForFormatSpecifier(fexpr, pass)
Expand All @@ -193,6 +201,10 @@ func checkForFunctionExpr(fexpr *ast.CallExpr, pass *analysis.Pass, c *config) {
})
}
}
// verbose Zero Check
if c.isEnabled(verboseZeroCheck, filename) {
checkForVerboseZero(fexpr, pass)
}
} else if fName == "NewContext" &&
isPackage(selExpr.X, "github.com/go-logr/logr", pass) &&
c.isEnabled(withHelpersCheck, filename) {
Expand Down Expand Up @@ -496,3 +508,37 @@ func checkForIfEnabled(i *ast.IfStmt, pass *analysis.Pass, c *config) {
funcCall, varName, funcCall, varName, varName),
})
}

func checkForVerboseZero(expr *ast.CallExpr, pass *analysis.Pass) {
if typeAndValue, ok := pass.TypesInfo.Types[expr]; ok {
switch t := typeAndValue.Type.(type) {
case *types.Named:
if typeName := t.Obj(); typeName != nil {
if pkg := typeName.Pkg(); pkg != nil {
if typeName.Name() == "Verbose" && pkg.Path() == "k8s.io/klog/v2" {
lit, ok := expr.Args[0].(*ast.BasicLit)
if !ok || lit.Value != "0" {
return
}
msg := fmt.Sprintf("klog does not allow verbose zero")
pass.Report(analysis.Diagnostic{
Pos: expr.Fun.Pos(),
Message: msg,
})
}
if typeName.Name() == "Logger" && pkg.Path() == "github.com/go-logr/logr" {
lit, ok := expr.Args[0].(*ast.BasicLit)
if !ok || lit.Value != "0" {
return
}
msg := fmt.Sprintf("logger does not allow verbose zero")
pass.Report(analysis.Diagnostic{
Pos: expr.Fun.Pos(),
Message: msg,
})
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
Copyright 2022 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 golang.org/x/tools/go/analysis/analysistest
// expects it to be here for loading. This package is used to test allow-unstructured
// flag which suppresses errors when unstructured logging is used.
// This is a test file for unstructured logging static check tool unit tests.

package verbose

import (
"github.com/go-logr/logr"
klog "k8s.io/klog/v2"
)

var l, logger logr.Logger

func verboseLogging() {

klog.V(0).Info("test log") // want `klog does not allow verbose zero`
klog.V(0).Infof("test log") // want `klog does not allow verbose zero`
klog.V(0).Infoln("test log") // want `klog does not allow verbose zero`
klog.V(0).InfoS("I'm logging at level 0.") // want `klog does not allow verbose zero`
klog.V(1).Info("test log")
klog.V(1).Infof("test log")
klog.V(1).Infoln("test log")
klog.V(1).InfoS("I'm logging at level 1.")
klog.Info("test log")
klog.Infof("test log")
klog.Infoln("test log")
klog.InfoS("I'm logging at level 0.")

logger.Info("hello", "1", "2")
logger.Error(nil, "hello", "1", "2")
logger.WithValues("1", "2")

logger.V(0).Info("hello", "1", "2") // want `logger does not allow verbose zero`
logger.V(0).Error(nil, "hello", "1", "2") // want `logger does not allow verbose zero`
logger.V(0).WithValues("1", "2") // want `logger does not allow verbose zero`

logger.V(1).Info("hello", "1", "2")
logger.V(1).Error(nil, "hello", "1", "2")
logger.V(1).WithValues("1", "2")
}

0 comments on commit 038c0b9

Please sign in to comment.