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

Introduce trace logging in Control Plane components #11132

Closed
siggy opened this issue Jul 19, 2023 · 0 comments · Fixed by #11147
Closed

Introduce trace logging in Control Plane components #11132

siggy opened this issue Jul 19, 2023 · 0 comments · Fixed by #11147

Comments

@siggy
Copy link
Member

siggy commented Jul 19, 2023

What problem are you trying to solve?

In the Go-based Control Plane components, the debug log level is quite verbose. This is the highest log level available in the Control Plane:

logLevel := cmd.String("log-level", log.InfoLevel.String(),
"log level, must be one of: panic, fatal, error, warn, info, debug")

Also, the debug log level sets klog to v=12, potentially exposing sensitive authorization tokens:

flag.Set("v", "12") // At 7 and higher, authorization tokens get logged.

How should the problem be solved?

Introduce a trace log level. Change debug to klog v=6, set trace at v=12:

diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go
index 8ce0c67f8..7df09f25d 100644
--- a/pkg/flags/flags.go
+++ b/pkg/flags/flags.go
@@ -34,7 +34,7 @@ func ConfigureAndParse(cmd *flag.FlagSet, args []string) {
        flag.Set("log_file", "/dev/null")
        flag.Set("v", "0")
        logLevel := cmd.String("log-level", log.InfoLevel.String(),
-               "log level, must be one of: panic, fatal, error, warn, info, debug")
+               "log level, must be one of: panic, fatal, error, warn, info, debug, trace")
        logFormat := cmd.String("log-format", "plain",
                "log format, must be one of: plain, json")
        printVersion := cmd.Bool("version", false, "print version and exit")
@@ -66,13 +66,17 @@ func setLogLevel(logLevel string) {
        }
        log.SetLevel(level)

-       if level == log.DebugLevel {
+       if level >= log.DebugLevel {
                flag.Set("stderrthreshold", "INFO")
                flag.Set("logtostderr", "true")
-               flag.Set("v", "12") // At 7 and higher, authorization tokens get logged.
+               flag.Set("v", "6")
                // pipe klog entries to logrus
                klog.SetOutput(log.StandardLogger().Writer())
        }
+
+       if level >= log.TraceLevel {
+               flag.Set("v", "12") // At 7 and higher, authorization tokens get logged.
+       }
 }

 func maybePrintVersionAndExit(printVersion bool) {

Any alternatives you've considered?

Don't use debug logs.

How would users interact with this feature?

Via helm and CLI flags.

Would you like to work on this feature?

maybe

alpeb pushed a commit that referenced this issue Jul 26, 2023
The `--log-level` flag did not support a `trace` level, despite the
underlying `logrus` library supporting it. Also, at `debug` level, the
Control Plane components were setting klog at v=12, which includes
sensitive data.

Introduce a `trace` log level. Keep klog at `v=12` for `trace`, change
it to `v=6` for `debug`.

Fixes #11132

Signed-off-by: Andrew Seigner <[email protected]>
hawkw pushed a commit that referenced this issue Aug 9, 2023
The `--log-level` flag did not support a `trace` level, despite the
underlying `logrus` library supporting it. Also, at `debug` level, the
Control Plane components were setting klog at v=12, which includes
sensitive data.

Introduce a `trace` log level. Keep klog at `v=12` for `trace`, change
it to `v=6` for `debug`.

Fixes #11132

Signed-off-by: Andrew Seigner <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants