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

Read logging configuration from config file #1293

Merged
merged 1 commit into from
Dec 17, 2018
Merged

Conversation

eht16
Copy link
Contributor

@eht16 eht16 commented Aug 16, 2017

Support reading a whole dict logging configuration from the config file
and configure the logging framework accordingly.
Move special logging configuration via command line options
(--verbose, --debug) into load_rules() as it needs to be set after the
logging framework has been reconfigured.

This should keep the previous behavior as close as possible when no
logging setup is provided in the configuration and mimic the desired log
level adjustments where appropriate even if a custom logging config
is given.

My main motivation was to add a custom log format, especially including
a timestamp (I don't like logs without timestamps).
Furthermore, with a full logging config, you can easily log your
ElastAlert logs into Logstash with https://github.com/eht16/python-logstash-async/, e.g. https://gist.github.com/eht16/b2183e1ce8373b8916b72dfa985a20c6.

@pataquets
Copy link

+1 if:

  • Command-line switches prevail if they contradict config file, to avoid unexpected behaviour changes.
  • Command-line switches are kept to avoid breaking anyone's setup.

Would love to get this in, since I would more easily enable debug-level logging (since --debug does not raise logger's level) and this will ease development & troubleshooting.

@pataquets
Copy link

Also, I'm not that deep in Python as of now, but ideally ES tracing should be as configurable as main logging, too.

@eht16
Copy link
Contributor Author

eht16 commented Feb 15, 2018

I'd be happy to update the PR if there are any chances that it get merged eventually.
Of course, the current behaviour should not be changed, I tried to ensure this but will re-check to be sure.

@eht16
Copy link
Contributor Author

eht16 commented Mar 21, 2018

I rebased my changes on current master, so everything is up to date and "mergeable" :).

Also I tested again the code for changes in current behaviour regarding --verbose and --debug.

There is only one change but on purpose:
If the log level in config is set to DEBUG and --verbose/--debug do not alter it to INFO.
Strictly it should be altered to INFO as this is what the previous behaviour did but only because the log level was never set lower than INFO.

So I personally think this is the right thing to do as it would seem strange to me if I put DEBUG in the config and --debug on the CLI and the resulting log level
is INFO.

@Qmando (or whoever else) any change to get this merged?

Support reading a whole dict logging configuration from the config file
and configure the logging framework accordingly.
Move special logging configuration via command line options
(--verbose, --debug) into load_rules() as it needs to be set after the
logging framework has been reconfigured.

This should keep the previous behavior as close as possible when no
logging setup is provided in the configuration and mimic the desired log
level adjustments where appropriate even if a custom logging config
is given.
@eht16
Copy link
Contributor Author

eht16 commented Dec 17, 2018

I rebased my changes on current master again, so everything is up to date and "mergeable" :).

@Qmando Qmando merged commit 96092f4 into Yelp:master Dec 17, 2018
@eht16
Copy link
Contributor Author

eht16 commented Dec 17, 2018

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants