madzak / python-json-logger

Json Formatter for the standard python logger
BSD 2-Clause "Simplified" License
1.74k stars 231 forks source link

Invalid format '(levelname) (name) (message)' for '%' style #86

Closed stchris closed 3 years ago

stchris commented 4 years ago

It seems like with Python 3.8 the format string needs to be specified differently.

I have this small test setup:

This is test.py:

import os
import logging
from pythonjsonlogger import jsonlogger

logger = logging.getLogger()
logHandler = logging.StreamHandler()
formatter = jsonlogger.JsonFormatter("(levelname) (name) (message)")
logHandler.setFormatter(formatter)
logger.handlers = [logHandler]
logger.setLevel(logging.INFO)

logger.info("Hello world")

and this Dockerfile:

FROM python:3.7
RUN pip install python-json-logger
COPY test.py .
CMD ["python", "test.py"]

If I run docker build -t test . && docker run -it test it works as expected, the output is {"levelname": "INFO", "name": "root", "message": "Hello world"}.

Under Python 3.8 this results in ValueError: Invalid format '(levelname) (name) (message)' for '%' style. I have to change the format line to read formatter = jsonlogger.JsonFormatter("%(levelname) %(name) %(message)"). I'm not sure if this is expected, but it might (at least) warrant an update of the documentation.

outime commented 4 years ago

@stchris hey thanks for this, just stumbled upon the same issue. It'd be great to put it in the docs indeed.

xmo-odoo commented 4 years ago

@stchris my understanding is jsonlogger intended to parse and extract fields from standard formatter patterns, which are composed of %(field)s and %(field)d. That just parenthesizing fields worked was is artefact of an overly lenient parser. 3.8's standard formatter apparently validates the format string more specifically against the format style but it doesn't require format strings to be specified differently so much as reject format strings which should never have been accepted in the first place.

The interaction between the format string and fields extraction would probably benefit from being documented (at all) though.

tirkarthi commented 3 years ago

Seems to be fixed with https://github.com/madzak/python-json-logger/pull/94

stchris commented 3 years ago

Indeed, this is fixed. Thanks, everyone! :+1: