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

gh-92592: Allow logging filters to return a LogRecord. (GH-92591) #92591

Merged
merged 18 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions Doc/howto/logging-cookbook.rst
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,41 @@ which, when run, produces something like:
2010-09-06 22:38:15,301 d.e.f DEBUG IP: 123.231.231.123 User: fred A message at DEBUG level with 2 parameters
2010-09-06 22:38:15,301 d.e.f INFO IP: 123.231.231.123 User: fred A message at INFO level with 2 parameters

Imparting contextual in Handlers
Copy link
Member

Choose a reason for hiding this comment

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

Should be contextual information rather than just contextual. Also, no need to capitalise handlers here, as we're not talking about the class.

---------------------------------

Each :class:`~Handler` has it's own chain of Filters.
Copy link
Member

Choose a reason for hiding this comment

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

it's should be its - the former is only used as a contraction of it is. Also, no need to capitalise filters here.

If you want to add contextual information to a :class:`LogRecord` without leaking
this contextual information to other handlers, you can use a filter that returns
Copy link
Member

Choose a reason for hiding this comment

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

You could shorten this contextual information to just it.

a new :class:`~LogRecord` instead of modifying it in-place:

script::

import logging

def filter(record: logging.LogRecord):
vsajip marked this conversation as resolved.
Show resolved Hide resolved
record = logging.LogRecord(
name=record.name,
level=record.levelno,
pathname=record.pathname,
lineno=record.lineno,
msg=record.msg,
exc_info=record.exc_info,
args=(),
)
record.user = 'jim'
return record

if __name__ == '__main__':
logger = logging.getLogger()
logger.setLevel(logging.INFO)
handler = logging.StreamHandler()
formatter = logging.Formatter('%(message)s from %(user)-8s')
handler.setFormatter(formatter)
handler.addFilter(filter)
logger.addHandler(handler)

logger.info('A log message')

.. _multiple-processes:

Expand Down
11 changes: 9 additions & 2 deletions Doc/library/logging.rst
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,9 @@ empty string, all events are passed.
.. method:: filter(record)

Is the specified record to be logged? Returns zero for no, nonzero for
Copy link
Member

Choose a reason for hiding this comment

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

After the change, this should be `a 'falsy' value for no, and a 'truthy' value for yes.

yes. If deemed appropriate, the record may be modified in-place by this
method.
yes. Filters can also modify log records in-place or return a completely
Copy link
Member

Choose a reason for hiding this comment

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

also -> either

different record instance which will replace the original
log record without modifying it.
Copy link
Member

Choose a reason for hiding this comment

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

I think the without modifying it should be replaced by in any future processing of the event.


Note that filters attached to handlers are consulted before an event is
emitted by the handler, whereas filters attached to loggers are consulted
Expand All @@ -686,6 +687,12 @@ which has a ``filter`` method with the same semantics.
parameter. The returned value should conform to that returned by
:meth:`~Filter.filter`.

.. versionchanged:: 3.12
You can now return a :class:`LogRecord` instance from filters to replace
the log record without modifying it in place. This allows filters installed
Copy link
Member

Choose a reason for hiding this comment

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

installed on -> attached to

on a :class:`Handler` to modify the log record before it is emitted without
Copy link
Member

Choose a reason for hiding this comment

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

attached to a handler, not attached to on a handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, corrected

having side effects on other handlers.

Although filters are used primarily to filter records based on more
sophisticated criteria than levels, they get to see every record which is
processed by the handler or logger they're attached to: this can be useful if
Expand Down
41 changes: 31 additions & 10 deletions Lib/logging/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -817,23 +817,34 @@ def filter(self, record):
Determine if a record is loggable by consulting all the filters.

The default is to allow the record to be logged; any filter can veto
this and the record is then dropped. Returns a zero value if a record
is to be dropped, else non-zero.
this by returning a falsy value and the record is then dropped and
this method returns a falsy value.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this now repetition which isn't needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's worth differentiating between the return values of filters and the return value of the Filterer.filter function. The main difference is that filters may return a truthy value that is not a log record but Filterer.filter will now always return a log record (or falsy). I reworded it a bit to make these two different paragraphs.

Filters can return a log record, which case that log record
Copy link
Member

Choose a reason for hiding this comment

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

Change sentences to If a filter attached to a handler returns a log record instance, then that instance is used in place of the original log record in any further processing of the event by that handler. If a filter returns any other truthy value, the original log record is used in any further processing of the event by that handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you that does sound better

is used to call the next filter.
If filters return a truthy value that is not a log record the
next filter is called with the existing log record.

If none of the filters return falsy values, this method returns
a log record.

.. versionchanged:: 3.2

Allow filters to be just callables.

.. versionchanged:: 3.12
Allow filters to return a LogRecord instead of
modifying it in place.
"""
rv = True
for f in self.filters:
if hasattr(f, 'filter'):
result = f.filter(record)
else:
result = f(record) # assume callable - will raise if not
if not result:
rv = False
break
return rv
return False
if isinstance(result, LogRecord):
record = result
return record

#---------------------------------------------------------------------------
# Handler classes and functions
Expand Down Expand Up @@ -968,10 +979,14 @@ def handle(self, record):

Emission depends on filters which may have been added to the handler.
Wrap the actual emission of the record with acquisition/release of
the I/O thread lock. Returns whether the filter passed the record for
emission.
the I/O thread lock.

Returns an instance of the log record that was emitted
if it passed all filters, otherwise a falsy value is returned.
"""
rv = self.filter(record)
if isinstance(rv, LogRecord):
record = rv
if rv:
self.acquire()
try:
Expand Down Expand Up @@ -1640,8 +1655,14 @@ def handle(self, record):
This method is used for unpickled records received from a socket, as
well as those created locally. Logger-level filtering is applied.
"""
if (not self.disabled) and self.filter(record):
self.callHandlers(record)
if self.disabled:
return
maybe_record = self.filter(record)
if not maybe_record:
return
if isinstance(maybe_record, LogRecord):
record = maybe_record
self.callHandlers(record)

def addHandler(self, hdlr):
"""
Expand Down
59 changes: 59 additions & 0 deletions Lib/test/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,65 @@ def log_at_all_levels(self, logger):
for lvl in LEVEL_RANGE:
logger.log(lvl, self.next_message())

def test_handler_filter_replaces_record(self):
def replace_message(record: logging.LogRecord):
return logging.LogRecord(
Copy link
Member

Choose a reason for hiding this comment

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

Use copy.copy() here and elsewhere instead of this.

name=record.name,
level=record.levelno,
pathname=record.pathname,
lineno=record.lineno,
msg="new message!",
exc_info=record.exc_info,
args=(),
)

# Set up a logging hierarchy such that "child" and it's handler
# (and thus `replace_message()`) always get called before
# propagating up to "parent".
# Then we can confirm that `replace_message()` was able to
# replace the log record without having a side effect on
# other loggers or handlers.
parent = logging.getLogger("parent")
child = logging.getLogger("parent.child")
stream_1 = io.StringIO()
stream_2 = io.StringIO()
handler_1 = logging.StreamHandler(stream_1)
handler_2 = logging.StreamHandler(stream_2)
handler_2.addFilter(replace_message)
parent.addHandler(handler_1)
child.addHandler(handler_2)

child.info("original message")
handler_1.flush()
handler_2.flush()
self.assertEqual(stream_1.getvalue(), "original message\n")
self.assertEqual(stream_2.getvalue(), "new message!\n")

def test_logging_filter_replaces_record(self):
records = set()

class RecordingFilter(logging.Filter):
def filter(self, record: logging.LogRecord):
records.add(id(record))
return logging.LogRecord(
name=record.name,
level=record.levelno,
pathname=record.pathname,
lineno=record.lineno,
msg=record.msg,
exc_info=record.exc_info,
args=(),
)

logger = logging.getLogger("logger")
logger.setLevel(logging.INFO)
logger.addFilter(RecordingFilter())
logger.addFilter(RecordingFilter())

logger.info("msg")

self.assertEqual(2, len(records))

def test_logger_filter(self):
# Filter at logger level.
self.root_logger.setLevel(VERBOSE)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Let :mod:`logging` Filters to return a :class:`logging.LogRecord` instance
Copy link
Member

Choose a reason for hiding this comment

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

No need to capitalise filters and handlers here, and also either use Allow filters to return ... or Let filters return ..., as Let ... to ... would be grammatically incorrect. Also, strictly, it would be more accurate to say ... so that filters attached to handlers can enrich ...

so that :class:`logging.Handler`\ s can enrich records without side effects on
other Handlers.