seanmonstar / intel

I need more intel!
http://seanmonstar.github.io/intel/
Mozilla Public License 2.0
202 stars 26 forks source link

Silencing default logger #14

Closed unveiled closed 10 years ago

unveiled commented 10 years ago

Has to be a stupid problem. I could not for the life of me figure out how to keep the 'root' Logger from writing to the console. Ended up using intel.config to assign an intel.NULL handler to the 'root' logger. Seemed unnecessarily complex. Is there a better way?

seanmonstar commented 10 years ago

In all my apps, I usually have a different named root logger, like intel.getLogger('bid'). In config, I personally always make that logger have propagate: false.

unveiled commented 10 years ago

Got it. It's not clear from the readme that the default Logger is the baked in parent of any 'top level' named Logger and that failing to set my 'top level' named Logger as propagate: false would result in my messages being logged by both my named Loggers and the built in default Logger. Is this intuitive? In my mind, the default Logger exists only for basic use cases, especially the way it's use is demonstrated in the readme. Once I've started creating named loggers, I wouldn't expect the default Logger to come into play anymore. My problem is solved though. Thanks for you help.

As an aside, I really appreciate the work you've put into this library. I was surprised how long it took me to find a reasonable logger utility for node. Your api is simple and clean while meeting all of my use cases, and the hierarchical idea is great.

seanmonstar commented 10 years ago

I welcome advice on fixing this. My thought was, if you start doing intel.getLogger('foo.bar'), but otherwise never setup any handlers, the top level logger will still output to stdout.

unveiled commented 10 years ago

I understand now why everything implicitly propagates to root. It's a clean way of addressing a couple of basic use cases, but it makes a couple of others less intuitive. As I see it there are three api concerns:

  1. require('intel').info('blah') should just work.
  2. intel.getLogger('foo.bar').info('blah') without setting up a handler for 'foo.bar' should work the same as 1 with 'foo.bar' as the name.
  3. intel.config ... shouldn't invisibly/automatically propagate top level named loggers to an implicit 'root' Logger.

Honestly, 2 was also a hiccup in my grokking of intel. Calling intel.getLogger('foo.bar') returns a Logger that already has some sort of handler attached to it. It wasn't clear to me how I was supposed to modify the settings of this default handler. Was there a name for it, so I could remove it? If I added another console handler would it override the default? I couldn't easily resolve either question, so I avoided that pattern. I'm assuming now that intel.getLogger('foo.bar') just returns a Logger with no handlers and the automatic propagation to 'root' results in root's handler processing the message. So does adding a handler to 'foo.bar' result in both foo.bar's handler and root's handler being called?

I would propose that intel.getLogger('foo.bar') should return a Logger named 'foo.bar' with 'foo' as the root Logger (no implicit default root Logger). Logging to 'foo.bar' propagates by default to 'foo' but no further. If both 'foo' and 'foo.bar' have no explicitly defined handlers (either through intel.addHandler or intel.config), then the handler defined on 'root' is used as a sane default. Defining a handler on 'foo.bar' (not 'foo') will override the default handler for 'foo.bar', but the default handler would still be called by 'foo' when the message propagates.

I'm not particularly happy with this proposal. While I think it addresses some of the issues, it also results in much more special casing. And the behavior remains unintuitive. If everything worked as I described, then there would still be the case where someone does intel.getLogger('foo.bar') and adds a console handler for 'foo.bar', but does not add a handler for 'foo'. Now everything is getting logged twice, once by foo.bar with the chosen formatting and once by foo with the default. This is the same unintuitive result I encountered. It could be addressed with even more special casing: if any Logger defines a console handler, then don't invoke the default handler for messages that propagate to parents of that Logger which have not had handlers defined for them. Now things are really getting ugly.

I think this is just an irreducible complexity arising out of hierarchical loggers. The best solution is probably to change nothing about the code and instead make some comments in the readme about the implicit root logger and how to easily silence it. Or maybe add an api call to silence it:

intel.getLogger('foo.bar').silenceRoot().addHandler ...

intel.getLogger('foo.bar').noDefault().addHandler ...

intel.getLogger('foo.bar').setRoot('foo').addHandler ...

intel.getLogger('foo').propagation(false) intel.getLogger('foo.bar').addHandler ...

Having an implicit root is not a bad design. I just wasn't expecting it after reading the readme a few times.

seanmonstar commented 10 years ago

I've toyed with the idea that if you call intel.config() with a handlers object, then loggers whose parent is ROOT would be implicitly set to propagates: false, unless you explicitly passed true.

// ROOT still handles also
intel.config({
  'loggers': {
    'foo': { level: 'INFO' }
  }
});
// foo would be set to not propagate
intel.config({
  'handlers': {
    // some handlers
  },
  'loggers': {
    'foo': { level: 'INFO' },
    'foo.bar': { level: 'WARN' } // parent is foo, so we default is propagate:true
  }
});
// foo explicitly propagates, so keep on
intel.config({
  'handlers': {
    // some handlers
  },
  'loggers': {
    'foo': { level: 'INFO', propagate: true },
    'foo.bar': { level: 'WARN' }
  }
});

This keeps the most sane settings for what people would I expect, I think...

unveiled commented 10 years ago

I like that. Single special case that is easy to explain. There aren't many hierarchical logging libraries (at least that I've seen), so I don't know how many expectations people bring to the table before reading the docs.

seanmonstar commented 10 years ago

I borrowed heavily from the Python's standard logging library

seanmonstar commented 10 years ago

Just a notice, this is now available on npm in v0.5: https://github.com/seanmonstar/intel/releases/tag/v0.5.0