six8 / logaware

Python Logger that is context aware
BSD 2-Clause "Simplified" License
2 stars 2 forks source link

AwareLogger does not respect stdlib's extra protocol. #6

Open symbolix opened 7 years ago

symbolix commented 7 years ago

Hi,

While investigating the mechanics of the stdlib's logging module, I had a chance to peak into how the internal _log() method is working and how the extra={} is handled.

I have noticed that the MetaAwareLogger that in return inherits from AwareLogger does not exactly respect the process of passing in an extra dictionary with the log message. I wonder if this is an issue or a feature?

meta.set_meta(context='cli')
log = MetaAwareLogger(lambda: meta.get_meta())

log.debug('Hello World?', extra={'event': 'buffer'})

Would fail with a formatter such as [%s(meta)s] [%s(event)s]

as internally the extra is stored inside a nested dictionary data {}. So there are two main dictionaries meta {} and data {}. meta {} makes sense, but to be able to access the contents of data {} one would need a custom Formatter to display data['extra'] with the above specified formatter template. To get his:

[{"contex":"cli"}] [{"event":"buffer"}]

Just checking. Thanks.

symbolix commented 7 years ago

To illustrate this with a simple example:

This code:

# Initialise logger.
log = logging.getLogger()
log.setLevel(logging.DEBUG)

# Define a format.
format_template = u'%(message)s [%(event)s]'
formatter = logging.Formatter(format_template)

# Create a handler.
ch = logging.StreamHandler()
ch.setFormatter(formatter)
ch.setLevel(logging.DEBUG)

# Add the handler to the logger
log.addHandler(ch)

log.warning(" Captured event is:", extra={'event':'buffer'})

Will produce this output:

Captured event is: [buffer]

But with logaware, this would fail as the extra {} bit is stored inside a dictionary called data {}. So it is not exactly compliant with the standard logging module. If it is a feature, please ignore me. Thanks.

six8 commented 7 years ago

I suppose it is a bug. Since logaware has metadata, extra might not be needed anymore.