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

Shain/client logger #23

Closed
wants to merge 3 commits into from
Closed

Shain/client logger #23

wants to merge 3 commits into from

Conversation

scotthain
Copy link
Contributor

🚧 Still some things not quite fleshed out, but a good starting place.

Basically, this proposes a new logging framework/style that retains backwards compatibility (if needed) and allows a simpler, more modular approach to creating and configuring loggers for the chef client. Also included would be some new loggers giving native support for syslog (finally!) and Windows (covered in a separate RFC)

@adamhjk
Copy link
Contributor

adamhjk commented Jul 18, 2014

How is this different from the output formatters and report handlers?

@sethvargo
Copy link

More on @adamhjk's comment - why do we need this? Is there something our current logging solution doesn't offer?

@scotthain
Copy link
Contributor Author

@adamhjk @sethvargo To the best of my knowledge, currently we only have one logging 'device' (the MonoLogger) which by default goes to STDOUT. You can also add the log_location to make it go to a file, and there's an override to make it do both if you want. This doesn't really enable multiple file, multiple output devices (other than these 2) or redirection (other than manually)

The formatters only apply to output that the 'user sees' which means they only get applied to things being logged to STDOUT. This doesn't change that concept or the functionality of that at all (it's pretty awesome as it is). What this would allow is for folks to create their own loggers to resolve things like this syslog issue - although this would be included in this implementation.

This does leverage a lot of our current solution (mixlib/log etc) but offers a more "pluggable" way to add a new logger in a well defined way.

@adamhjk
Copy link
Contributor

adamhjk commented Jul 18, 2014

If you were going to add this, it probably belongs in Mixlib::Log, and you
would want to keep backwards compat with the old interface.

A version of this that ignores the back-compat won't be acceptable to me,
given the number of folks who (warts and all) have worked around the
existing system (including with syslog).

On Fri, Jul 18, 2014 at 11:34 AM, Scott Hain notifications@github.com
wrote:

@adamhjk https://github.com/adamhjk @sethvargo
https://github.com/sethvargo To the best of my knowledge, currently we
only have one 'logging' device (the MonoLogger) which you by default goes
to STDOUT. You can also add the log_location to make it go to a file, and
there's an override to make it do both if you want.

The formatters only apply to output that the 'user sees' which means they
only get applied to things being logged to STDOUT. This doesn't change that
concept or the functionality of that at all (it's pretty awesome as it is).
What this would allow is for folks to create their own loggers to resolve
things like this syslog issue
https://tickets.opscode.com/browse/COOK-2326 - although this would be
included in this implementation.

This does leverage a lot of our current solution (mixlib/log etc) but
offers a more "pluggable" way to add a new logger in a well defined way.


Reply to this email directly or view it on GitHub
#23 (comment).

@scotthain
Copy link
Contributor Author

@adamhjk The idea is to have this be an 'opt-in' model that can mimic the functionality of the old style if configured correctly. It doesn't actually change the behavior of the current model at all, so if you never add a 'new' style logger, you'll never see the change. One of the big impetuses of this is that in the last logging refactor, we actually broke people's workarounds (as colorfully documented here). The other is the Windows logging stuff that @adamedx is working on right now - this would allow for multiple loggers to send different formats of output to different Windows log mechanisms without having to do crazy things.

The reason I don't think this belongs in mixlib/log itself, is mostly because mixlib/log (as far as I can tell) really defines the log levels, sets up a standard 'interface' to each log, and aggregates all the loggers you add to it. It doesn't actually have any log device functionality inside it.

@sersut
Copy link

sersut commented Jul 18, 2014

In fact I think it would be cool to give Mixlib::Log the ability to support SysLog or Windows Event Logs.

I don't think this changes the most things in this RFC. Chef::Loggers::* classes needs to be in mixlib-log with some additional configuration logic in chef to enable these loggers.

Totally 👍 on preserving 100% backwards compatibility.


### Configuration

#### (potentially) Deprecated Configurations
Copy link

Choose a reason for hiding this comment

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

We don't need to deprecate any of this functionality in order to make this work. So I'd replace this section with a section talking about how the backwards compat will be achieved.

@stevendanna
Copy link
Contributor

A couple of questions:

  • Am I misunderstanding, or is this essentially syntactic sugar and config cleanup around something like the following (which I believe works if you run it in something like a start handler, but have never had the need to test):
require 'path/to/my/logger'
Chef::Log.loggers << MyLogger.new(whatever_options)

That is not to say config cleanup and syntax helpers are a bad thing, I'm just making sure I have the scope of the project right.

  • How could we improve this to make it easier for users to provider their own loggers and formatters? Most of the times I've tried to ship customizations to either loggers or formatters as an end user, the most annoying part is figuring out where I can put my code such that Chef will load it and I can call it.


#### Chef::Loggers::WindowsEventLogger
TBD - name may change - see [Windows Logging RFC](https://github.com/opscode/chef-rfc/blob/adamed/windows-logging/rfc0002-windows-logging.md) (link may change)

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the number of distributions that are moving to systemd, it might be interesting to include a journald logger in this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually @stevendanna this works:
(adding syslog-logger to the Gemfile first)
require 'syslog-logger'
Chef::Log.use_log_devices( [ Logger::Syslog.new("chef-client", 8) ] )

@sethvargo
Copy link

I'm with @stevendanna

@coderanger
Copy link
Contributor

If part of the goal here is to simplify configuring multiple loggers, some prior art that might help: https://docs.djangoproject.com/en/1.7/topics/logging/#examples

@adamhjk
Copy link
Contributor

adamhjk commented Jul 18, 2014

You can also look at Log::Log4perl, which is a Log4j port, which is the
grand-daddy of configurable logging.

http://mschilli.github.io/log4perl/

On Fri, Jul 18, 2014 at 3:42 PM, Noah Kantrowitz notifications@github.com
wrote:

If part of the goal here is to simplify configuring multiple loggers, some
prior art that might help:
https://docs.djangoproject.com/en/1.7/topics/logging/#examples


Reply to this email directly or view it on GitHub
#23 (comment).

@scotthain
Copy link
Contributor Author

thanks @adamhjk, @coderanger - to be honest I based some of the concept on the way slf4j interacts with other Java loggers (logback, log4j, etc) since that's kinda my background. I'll definitely check out these two for more context/inspiration

also to be clear - my motivation for this is as follows (in order of importance)
a. create a syslog logger
b. show people how to enable/create a new logger if they want/need to (document the way it works)
c. clean up/simplify code/config if it will make either a. or b. easier.

@sethvargo
Copy link

I also wrote Logify

@scotthain
Copy link
Contributor Author

This will be revisited and a new RFC will be created following the new RFC guidelines.

@scotthain scotthain closed this Sep 5, 2014
@thommay thommay deleted the shain/client_logger branch January 23, 2017 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants