Core messages and config options #1

Closed
jwittkoski opened this Issue Oct 25, 2012 · 4 comments

Comments

Projects
None yet
2 participants
Collaborator

jwittkoski commented Oct 25, 2012

  1. When doing a simple drop in replacement of Dancer's built in logger: console with Dancer::Logger::Log4perl, the first thing I noticed is that Dancer's core messages started appearing even though in Dancer's config I have log: debug (i.e. higher than core in normal Dancer semantics). With the built in logger: console I don't see that.

    Looking at the code I see that Dancer::Logger::Log4perl doesn't inherit from Dancer::Logger::Abstract at all. So the checks for Dancer's log config value (and therefore the possible filtering out of the core messages) are not done. I assume this is because that many of Logger::Log4perl features (like filtering messages below a certain logging level) are more powerful than Dancer's built in logging features so the intention was not to have two "layers" of such features to deal with?

    However, it wasn't clear from the Dancer::Logger::Log4perl documentation that Dancer's standard log and logger_format config options will be ignored.

  2. Why are Dancer's core messages mapped to info instead of a lower level such as debug? (I am assuming it is due to their status as informational messages that can be filtered by category if needed.)

    Would the correct Log4perl way to filter these (in production for example) be to filter out Dancer.Logger.Log4perl category messages? (That is the category I see for core messages.)

  3. Dancer::Logger::Log4perl ignoring logger_format is a little problematic. Even though many of the format options are duplicates with the ones provided by Log4perl, there are several Dancer related format placeholders that are not available. If logger_format was used to format the message that becomes Log4perl's %m all placeholders would be available.

Owner

polettix commented Oct 26, 2012

Hi! Glad to see that there's interest in the module.

Regarding Inheritance from Dancer::Logger::Abtract, I'm not sure that when I developed the module one year and a half ago it was available. Anyway it's highly probable that the blame is entirely on me and I just produced a duck class instead of going on the right path. This also implies the lack of "info" and your merge request I suspect.

My personal thought on the issue is that the class should be rewritten deriving from Dancer::Logger::Abstract, providing the _log() method, the only thing is to take care not to break anything. Looking at the amount of code it should be feasible :-)

On the other hand it remains to understand whether this approach honors logger_format but destroys all placeholders for Log4perl, which would be unfortunate. This has to be investigated.

I would also add that I did not have a precise idea when mapping the core level onto Log4perl ones, so I simply went for info. Again, this should be corrected if things go on the "right path".

Collaborator

jwittkoski commented Oct 29, 2012

Ah, ok, that makes sense! I agree that inheriting from D::L::Abstract is probably the right way forward.

Regarding logger_format, I was thinking that it could be left as D::L::Abstract uses it. Then it's "output" becomes the %m message within Log4perl. This would of course mean for example that the user could specify the time in both logger_format and within Log4perl's config, but that is easily adjustable with one or the other's config options. My reasoning being that trying to merge the two different template formats would be more confusing than simply "layering" them and letting the user set the appropriate config values.

Regarding core - as I think you mentioned on one of the mailing lists a while back it is really a category and not a level. If D::L::Log4perl mapped it this way then it could be filtered out separately from other info messages.

Collaborator

jwittkoski commented Nov 13, 2012

I created pull #4 as an attempt to refactor to use D::L::Abstract.

Collaborator

jwittkoski commented Apr 17, 2014

Closing this as @Casao merged the other issues about log levels and the refactored code.

jwittkoski closed this Apr 17, 2014

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