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

feat(occ): Add a --debug option to output all log levels to the output #46115

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Jun 25, 2024

Related to #30989

Summary

Adds a universal option to pass to occ commands so that all levels of logs are printed in the output, even if the loglevel on the instance is higher.
This allows to inspect all the logging related to a command without having it mixed up with logs from other stuff going on in the server. Also allows to have logs and command output in the same place to see in which order things happen.
This is especially useful with user_ldap as all LDAP calls are logged in debug level.

TODO

  • Decide on the correct naming for the option, currently --debug bug maybe this will cause conflicts too easily. Changed to --debug-log
  • Decide whether to document the option in the usage output for classes based on OC\Core\Command\Base.
  • Determine whether the output format is the right one. (Do we want raw json instead?) Abandoned this idea, there is not much data to put in json anyway
  • Do we need to be able to do the same with an arbitrary loglevel, like --debug=1 to only show info and above? Added support for --debug-log-level when --debug-log is used
  • Should we activate the profiler when this option is used and output the profile? Not automatically
  • How can we make it output DB requests to the output as well? The profiler idea above would work. Left as an exercise for later

Checklist

@come-nc come-nc self-assigned this Jun 25, 2024
@come-nc come-nc force-pushed the enh/add-a-universal-debug-option-to-occ branch from 2a3a1ab to 808a2d3 Compare June 27, 2024 08:11
@come-nc
Copy link
Contributor Author

come-nc commented Jun 27, 2024

Hum, I think this is a duplicate of nextcloud/logreader#873 .
I’d prefer having this in core and as an option though, as setting an env var is not always the same syntax for customers, and logreader may be disabled. The point was to have a command we can pass to any user to get a detailed idea of what happens when they run a command.

@icewind1991
Copy link
Member

I’d prefer having this in core

Makes sense.

Decide on the correct naming for the option, currently --debug bug maybe this will cause conflicts too easily.

Maybe --debug-log (--debug is already conflicting with some apps)

Determine whether the output format is the right one. (Do we want raw json instead?)

We can do both by allowing something like --debug-log-format=json

Do we need to be able to do the same with an arbitrary loglevel, like --debug=1 to only show info and above?

Allowing a log level and defaulting to debug makes most sense to me (--debug-log=info)

Should we activate the profiler when this option is used and output the profile?

Should be a separate options

How can we make it output DB requests to the output as well? The profiler idea above would work.

Also would have that as a separate option.

--

It might make sense to have this logic detect if the logreader is available and delegate the logging to the logreader when possible, since that app already has cli log formatting. Keeping a "basic" log output in core.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
…n for level

Also changed option from --debug to --debug-log to avoid conflicts

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the enh/add-a-universal-debug-option-to-occ branch from 808a2d3 to 9baf8fe Compare July 2, 2024 14:35
@come-nc come-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 2, 2024
@come-nc come-nc added this to the Nextcloud 30 milestone Jul 2, 2024
@come-nc come-nc marked this pull request as ready for review July 2, 2024 15:28
$argv = $_SERVER['argv'];
$level = 0;
foreach ($argv as $key => $arg) {
if ($arg === '--debug-log') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put them into OCP somewhere so app devs have a list/track of "reserved arguments"?

…ow about them

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the enh/add-a-universal-debug-option-to-occ branch from 1ac84bf to 612088b Compare July 4, 2024 14:19
lib/base.php Outdated Show resolved Hide resolved
…ption

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc requested a review from artonge July 8, 2024 14:11
@come-nc come-nc merged commit a434dfb into master Jul 8, 2024
165 checks passed
@come-nc come-nc deleted the enh/add-a-universal-debug-option-to-occ branch July 8, 2024 15:15
@come-nc come-nc added the pending documentation This pull request needs an associated documentation update label Jul 8, 2024
/**
* @since 30.0.0
*/
final class ReservedOptions {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews feature: occ pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants