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

Fix Default log level for automation log #17130

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Mar 9, 2018

This PR introduces

  • Default settings to set the log level for level_automation and level_policy (not needed, will explain below)
  • Adds lines to call Vmdb::Logger.apply_config on the $miq_ae_logger and $policy_log
  • Pedantic: (Mostly) Alphabetize the lists of loggers in three spots
    • config/settings.yml
    • Vmdb::Logger.apply_config
    • `Vmdb::Logger.create_loggers``

Regarding the "not needed" part, the method Vmdb::Logger.apply_config_value method can actually be called without the config value be present in the config and still would default to INFO, which is what we are looking for (this is already done with the $datawarehouse_log for example). I was unaware of this until doing the second commit in this PR, so I could see a case of not adding these settings for simplicity

Also, the second commit is completely pedantic and optional (I have no problem rebasing it out if that is prefered), but I figured that @bdunne would have eventually done it at some point anyway, so I figured I would get it out of the way. :trollface:

That said, I did leave the evm.log and rails.log at the top since I figured that those were of more importance in most cases.

Links

Steps for Testing/QA

If you run a automation task, the default log output should not show any [DEBUG] lines

Prior to a7ed2cb, all loggers would
have defaulted to `INFO` with or without the multicast logger.  The bug
that was fixed in that case is allowing the multicast logger to convert
the logger into `DEBUG` mode.

Unfortunately, two loggers weren't a part of the list of loggers that
then have user settings applied to them to change their default log
level:

- $miq_ae_logger
- $policy_log

Since both have been assumed to be `INFO` in the past, this change
makes that return to the previous behavior by default, and also has the
added bonus of allowing the user to change these settings via the
config.

Of note, the $policy_log was not the original target of the BZ that
prompted this change, but was added since it might also end up causing
issues down the road.

BZ link:  https://bugzilla.redhat.com/show_bug.cgi?id=1553813
@NickLaMuro
Copy link
Member Author

@miq-bot assign @gmcculloug
@miq-bot assign @Fryguy
@miq-bot add_labels bug, automation

@miq-bot
Copy link
Member

miq-bot commented Mar 9, 2018

@NickLaMuro Cannot apply the following label because they are not recognized: automation

@miq-bot miq-bot assigned Fryguy and unassigned gmcculloug Mar 9, 2018
@miq-bot miq-bot added the bug label Mar 9, 2018
@NickLaMuro
Copy link
Member Author

*shakes fist at @miq-bot *

Fine...

@miq-bot add_label automate

Better?

@miq-bot
Copy link
Member

miq-bot commented Mar 9, 2018

Checked commits NickLaMuro/manageiq@5fbb43e~...3626e94 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@gmcculloug
Copy link
Member

miq-bot labels Fine/yes 😆

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

👍 I approve this alphabetical change

@bdunne bdunne merged commit ca25c15 into ManageIQ:master Mar 9, 2018
@bdunne bdunne added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 9, 2018
@bdunne bdunne assigned bdunne and unassigned Fryguy Mar 9, 2018
@NickLaMuro
Copy link
Member Author

Wow... I hope @bdunne didn't get too excited about the alphabetical change and overlook the true intention of the PR 😏

@bdunne
Copy link
Member

bdunne commented Mar 9, 2018

No, I saw the full intent of the PR 😄

@simaishi simaishi mentioned this pull request Mar 9, 2018
simaishi pushed a commit that referenced this pull request Mar 12, 2018
…ation_logger

Fix Default log level for automation log
(cherry picked from commit ca25c15)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553924
@simaishi
Copy link
Contributor

Backported this PR + #17136.

Gaprindashvili backport details:

$ git log -1
commit 404be15527ad65ce169e7748803de1b11899d26b
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Fri Mar 9 12:16:50 2018 -0500

    Merge pull request #17130 from NickLaMuro/fix_default_debug_for_automation_logger
    
    Fix Default log level for automation log
    (cherry picked from commit ca25c1575905134e930be82959c7785560c9708f)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553924

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants