Skip to content

Commit

Permalink
structured-logging: enable KobjSlice usage and warn Kobjs usage
Browse files Browse the repository at this point in the history
add explicit flag for deprecations check
  • Loading branch information
harshanarayana committed Sep 19, 2022
1 parent baaa0bc commit 3a1631d
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 4 deletions.
27 changes: 27 additions & 0 deletions logcheck/pkg/logcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const (
contextualCheck = "contextual"
withHelpersCheck = "with-helpers"
verbosityZeroCheck = "verbosity-zero"
deprecationsCheck = "deprecations"
)

type checks map[string]*bool
Expand All @@ -59,6 +60,7 @@ func Analyser() *analysis.Analyzer {
contextualCheck: new(bool),
withHelpersCheck: new(bool),
verbosityZeroCheck: new(bool),
deprecationsCheck: new(bool),
},
}
c.fileOverrides.validChecks = map[string]bool{}
Expand All @@ -73,6 +75,7 @@ klog methods (Info, Infof, Error, Errorf, Warningf, etc).`)
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[verbosityZeroCheck], prefix+verbosityZeroCheck, true, `When true, logcheck will check whether the parameter for V() is 0.`)
logcheckFlags.BoolVar(c.enabled[deprecationsCheck], prefix+deprecationsCheck, true, `When true, logcheck will analyze the usage of deprecated Klog function calls.`)
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 @@ -149,6 +152,17 @@ func checkForFunctionExpr(fexpr *ast.CallExpr, pass *analysis.Pass, c *config) {
return
}

// Check for Deprecated function usage
if c.isEnabled(deprecationsCheck, filename) {
message, deprecatedUse := isDeprecatedContextualCall(fName)
if deprecatedUse {
pass.Report(analysis.Diagnostic{
Pos: fun.Pos(),
Message: message,
})
}
}

// Matching if any unstructured logging function is used.
if c.isEnabled(structuredCheck, filename) && isUnstructured(fName) {
pass.Report(analysis.Diagnostic{
Expand Down Expand Up @@ -301,6 +315,18 @@ func isUnstructured(fName string) bool {
return false
}

func isDeprecatedContextualCall(fName string) (message string, deprecatedUse bool) {
deprecatedContextualLogHelper := map[string]string{
"KObjs": "KObjSlice",
}
var replacementFunction string
if replacementFunction, deprecatedUse = deprecatedContextualLogHelper[fName]; deprecatedUse {
message = fmt.Sprintf(`Detect usage of deprecated helper "%s". Please switch to "%s" instead.`, fName, replacementFunction)
return
}
return
}

func isContextualCall(fName string) bool {
// List of klog functions we still want to use after migration to
// contextual logging. This is an allow list, so any new acceptable
Expand All @@ -315,6 +341,7 @@ func isContextualCall(fName string) bool {
"FromContext",
"KObj",
"KObjs",
"KObjSlice",
"KRef",
"LoggerWithName",
"LoggerWithValues",
Expand Down
5 changes: 5 additions & 0 deletions logcheck/testdata/src/k8s.io/klog/v2/klog.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ func KObjs(obj interface{}) interface{} {
return nil
}

// KObjSlice emulatess klog.KObjSlice
func KObjSlice(obj interface{}) interface{} {
return nil
}

func KRef(namespace, name string) interface{} {
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (
)

func doNotAlllowKlog() {
klog.InfoS("test log") // want `function "InfoS" should not be used, convert to contextual logging`
klog.ErrorS(nil, "test log") // want `function "ErrorS" should not be used, convert to contextual logging`
klog.V(1).Infof("test log") // want `function "V" should not be used, convert to contextual logging` `function "Infof" should not be used, convert to contextual logging`
klog.InfoS("test log") // want `function "InfoS" should not be used, convert to contextual logging`
klog.ErrorS(nil, "test log") // want `function "ErrorS" should not be used, convert to contextual logging`
klog.V(1).Infof("test log") // want `function "V" should not be used, convert to contextual logging` `function "Infof" should not be used, convert to contextual logging`
klog.KObjs(nil) // want `Detect usage of deprecated helper "KObjs". Please switch to "KObjSlice" instead.`
klog.InfoS("test log", "key", klog.KObjs(nil)) // want `function "InfoS" should not be used, convert to contextual logging` `Detect usage of deprecated helper "KObjs". Please switch to "KObjSlice" instead.`
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

func allowKlog() {
klog.KObj(nil)
klog.KObjs(nil)
klog.KObjSlice(nil)
klog.KRef("", "")
klog.FromContext(nil)
klog.TODO()
Expand Down

0 comments on commit 3a1631d

Please sign in to comment.