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

Add meaningful logging to library and CLI applications #922

Closed
Tracked by #923
xavpaice opened this issue Dec 21, 2022 · 9 comments · Fixed by #1008
Closed
Tracked by #923

Add meaningful logging to library and CLI applications #922

xavpaice opened this issue Dec 21, 2022 · 9 comments · Fixed by #1008
Assignees
Labels
type::feature New feature or request

Comments

@xavpaice
Copy link
Member

Describe the rationale for the suggested feature.

When we run preflight or support-bundle (or, indeed, the other CLI components) there is a --debug switch, but it doesn't appear to do much.

Several times lately I have wanted to review what the process is doing, and where it is failing, but do not have the information without inserting print statements.

Describe the feature

When we specify --debug on the CLI, provide at least:

  • A log message in stdout for each collector, analyzer and redactor running with start/end times
  • same messages added to a log file in the support bundle
  • a copy of the support-bundle spec that ran (post-merge) included in the support-bundle. This may require redactors to run over the output.

Describe alternatives you've considered

Describe alternative solutions here. Include any workarounds you've considered.

TBA

Additional context

@xavpaice xavpaice added the type::feature New feature or request label Dec 21, 2022
@banjoh
Copy link
Member

banjoh commented Dec 22, 2022

Should we generalise this issue to have it address logging in general? Some points that need addressing are

  • Logging library of choice. We currently have references of go's log package and klog
  • Do we want to support log levels?
  • Log formatting?

@adamancini
Copy link
Member

we could go ahead and trigger a goroutine stack dump as well

@adamancini
Copy link
Member

relates to #926

@adamancini
Copy link
Member

the way I imagine it, it might be a shortcut to specifying the profiling flags, as well as setting --interactive=flase, and emitting logs at "debug" level (if we choose to support log levels) and emit everything to stdout

@xavpaice
Copy link
Member Author

Let's spec out a solution that first addresses the mixture of log and klog, and then works to implement debug levels so that when we specify --debug we get formatted debug logs on stderr (current location). This much would allow the tools to work as folks expect.

A second part of this would include some basic additions so that logs messages at debug level are emitted on various useful events, so that if we capture stdout/stderr and attach it to a support case, we get meaningful info to assist diagnose failing runs of support-bundle.

@xavpaice
Copy link
Member Author

@banjoh does #1010 cover the original issue here?

@banjoh
Copy link
Member

banjoh commented Feb 17, 2023

@banjoh does #1010 cover the original issue here?

Bullet 1 - Addressed by #1010

Bullet 2 - Will be covered by #1008

Bullet 3 - Not implemented. I created #1027 issue to address this bit

@banjoh banjoh changed the title --debug should provide more meaningful logs Add meaningful logging to library and CLI applications Feb 21, 2023
@banjoh banjoh reopened this Feb 24, 2023
@banjoh
Copy link
Member

banjoh commented Feb 24, 2023

Reopening cause we are still not able to store logs in the support bundle. This requires changes in kubernetes/klog#363 PR to be merged and released. Specifically, we nee to be able to tap into the stream of logs being emitted by klog as described here. We also get the capability to set the logs verbosity level programatically.

Note to self: Add a logger.ResetLogger() function or an equivalent to reset klog when troubleshoot is used as a library.

@xavpaice
Copy link
Member Author

PR was merged on March 2. #1028 captures the remaining work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type::feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants