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

Allow overriding the log level in debug mode. #3050

Merged

Conversation

timoreimann
Copy link
Contributor

@timoreimann timoreimann commented Mar 19, 2018

What does this PR do?

Allow overriding the log level in debug mode.

Motivation

Enabling the debug mode caused the log level to be hard-wired to DEBUG. We change the behavior so that an explicitly defined log level always overrides.

One primary motivation is to allow pprof-based benchmarking without excessive DEBUG logging distorting the results.

Additional Notes

We also remove a piece of code from SetEffectiveConfiguration which was never effective because the log level would already be set previously.

Fixes #2968.

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

Hello @timoreimann .

Many thanks for this very useful PR 👏

Can you update the documentation :

  • By precising that logLevel will be turned in DEBUG mode only if logLevel is not specified (in the sentence Additionally, the log level will be set to DEBUG.)
  • By replacing (--debug switch) by (--logLevel DEBUG switch) in If applicable, please paste the log output in debug mode (--debug switch)

Thanks!

@timoreimann
Copy link
Contributor Author

@nmengin good catch -- updated.

Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Enabling the debug mode caused the log level to be hard-wired to DEBUG.
We change the behavior so that an explicitly defined log level always
overrides. One primary motivation is to allow pprof-based benchmarking
without excessive DEBUG logging distorting the results.

We also remove a piece of code from SetEffectiveConfiguration which was
never effective because the log level would already be set previously.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logs kind/enhancement a new or improved feature. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants