Skip to content

Logging improvements #1004

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Logging improvements #1004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
985502c
logging: Support extra context for LogRecord.
alonbl Apr 30, 2025
e3992b8
logging: Move exc_info to common log.
alonbl Apr 30, 2025
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
3 changes: 3 additions & 0 deletions python-stdlib/logging/examples/example_logging_2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@
class MyHandler(logging.Handler):
def emit(self, record):
print("levelname=%(levelname)s name=%(name)s message=%(message)s" % record.__dict__)
print("extra1=%s" % record.__dict__.get("extra1"))


logging.getLogger().addHandler(MyHandler())
logging.getLogger().setLevel(logging.INFO)
logging.info("Test message7")
logging.info("Test message8", extra={"extra1": 99})
77 changes: 41 additions & 36 deletions python-stdlib/logging/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,19 @@


class LogRecord:
def set(self, name, level, message):
def __init__(self, name, level, message, extra=None):
Copy link

@Josverl Josverl May 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If the intend of extra is to provide the same functionality as CPythons LogRecord( ..., args, ...)
,then it makes sense to name it the same. That makes it simpler to learn.

Copy link
Contributor Author

@alonbl alonbl Oct 23, 2025

Choose a reason for hiding this comment

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

Hi,
There is a inconsistency between LogRecord[1] and Logger.makeRecord[2].
It is "unknown" how extra reaches the LogRecord once constructed.
I can make extra as property which is also not fully compatible.
I can have setExtra().
I do not think this is that critical how we pass the extra into the record, as the interface is clearly not compatible, for example, we do not have makeRecord or event population between loggers.
If you have a better notation, please let me know.
Thanks!

[1] https://docs.python.org/3/library/logging.html#logging.LogRecord
[2] https://docs.python.org/3/library/logging.html#logging.Logger.makeRecord

self.name = name
self.levelno = level
self.levelname = _level_dict[level]
self.message = message
self.ct = time.time()
self.msecs = int((self.ct - int(self.ct)) * 1000)
self.asctime = None
if extra is not None:
Copy link

@Josverl Josverl May 2, 2025

Choose a reason for hiding this comment

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

can be simplified to if extra:

Copy link
Contributor Author

@alonbl alonbl Oct 23, 2025

Choose a reason for hiding this comment

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

Correct, but if you look closely in this source the pattern is to test for explicit None without having empty additional path. If it is important I will replace.

for key in extra:
if (key in ["message", "asctime"]) or (key in self.__dict__):
raise KeyError("Attempt to overwrite %r in LogRecord" % key)
setattr(self, key, extra[key])


class Handler:
Expand Down Expand Up @@ -110,7 +115,6 @@ def __init__(self, name, level=NOTSET):
self.name = name
self.level = level
self.handlers = []
self.record = LogRecord()

def setLevel(self, level):
self.level = level
Expand All @@ -121,36 +125,19 @@ def isEnabledFor(self, level):
def getEffectiveLevel(self):
return self.level or getLogger().level or _DEFAULT_LEVEL

def log(self, level, msg, *args):
def log(self, level, msg, *args, exc_info=False, extra=None):
if self.isEnabledFor(level):
if args:
if isinstance(args[0], dict):
args = args[0]
msg = msg % args
self.record.set(self.name, level, msg)
record = LogRecord(self.name, level, msg, extra)
handlers = self.handlers
if not handlers:
handlers = getLogger().handlers
for h in handlers:
h.emit(self.record)
h.emit(record)

def debug(self, msg, *args):
self.log(DEBUG, msg, *args)

def info(self, msg, *args):
self.log(INFO, msg, *args)

def warning(self, msg, *args):
self.log(WARNING, msg, *args)

def error(self, msg, *args):
self.log(ERROR, msg, *args)

def critical(self, msg, *args):
self.log(CRITICAL, msg, *args)

def exception(self, msg, *args, exc_info=True):
self.log(ERROR, msg, *args)
tb = None
if isinstance(exc_info, BaseException):
tb = exc_info
Expand All @@ -161,6 +148,24 @@ def exception(self, msg, *args, exc_info=True):
sys.print_exception(tb, buf)
self.log(ERROR, buf.getvalue())

def debug(self, msg, *args, **kwargs):
self.log(DEBUG, msg, *args, **kwargs)

def info(self, msg, *args, **kwargs):
self.log(INFO, msg, *args, **kwargs)

def warning(self, msg, *args, **kwargs):
self.log(WARNING, msg, *args, **kwargs)

def error(self, msg, *args, **kwargs):
self.log(ERROR, msg, *args, **kwargs)

def critical(self, msg, *args, **kwargs):
self.log(CRITICAL, msg, *args, **kwargs)

def exception(self, msg, *args, **kwargs):
self.log(ERROR, msg, *args, **kwargs)

def addHandler(self, handler):
self.handlers.append(handler)

Expand All @@ -178,32 +183,32 @@ def getLogger(name=None):
return _loggers[name]


def log(level, msg, *args):
getLogger().log(level, msg, *args)
def log(level, msg, *args, **kwargs):
getLogger().log(level, msg, *args, **kwarg)


def debug(msg, *args):
getLogger().debug(msg, *args)
def debug(msg, *args, **kwargs):
getLogger().debug(msg, *args, **kwargs)


def info(msg, *args):
getLogger().info(msg, *args)
def info(msg, *args, **kwargs):
getLogger().info(msg, *args, **kwargs)


def warning(msg, *args):
getLogger().warning(msg, *args)
def warning(msg, *args, **kwargs):
getLogger().warning(msg, *args, **kwargs)


def error(msg, *args):
getLogger().error(msg, *args)
def error(msg, *args, **kwargs):
getLogger().error(msg, *args, **kwargs)


def critical(msg, *args):
getLogger().critical(msg, *args)
def critical(msg, *args, **kwargs):
getLogger().critical(msg, *args, **kwargs)


def exception(msg, *args, exc_info=True):
getLogger().exception(msg, *args, exc_info=exc_info)
def exception(msg, *args, exc_info=True, **kwargs):
getLogger().exception(msg, *args, exc_info=exc_info, **kwargs)


def shutdown():
Expand Down
Loading