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

Suggestion: Include motivation in README.md. #15

Open
Julian-O opened this issue Nov 6, 2018 · 6 comments
Open

Suggestion: Include motivation in README.md. #15

Julian-O opened this issue Nov 6, 2018 · 6 comments

Comments

@Julian-O
Copy link

Julian-O commented Nov 6, 2018

The README.md currently says:

One way to ensure consistency and rigor in logging is to always use extra to pass non-constant data and, therefore, to never use format strings, concatenation, or other similar techniques to construct a log string.

However, if doesn't explain why this rigor is important. It is easy to dismiss this: "A foolish consistency is the hobgoblin of little minds."

I would add a few paragraphs explaining why this practice should be adopted.

e.g.

It is important to avoid using format strings for two reasons:

  1. Performance: If a log message is turned off, it is unnecessary to perform the string interpolation and/or the string concatenation and the implicit calls to __str__() functions on the parameters. Keeping the parameter separate means that the logging system can skip such operations.
  2. Maintainability: The logging module is designed to provide a separation of concerns: the treatment of log messages from a module can be modified outside of the module. In particular, logging.Handler and logging.Filter classes can be customised to identify particular log messages and take particular actions, such as filter them, escalate them, replace them with a clearer log message or translate them to a different language. These manipulations are easier if access is given to the raw log message before the string interpolation is performed.
@fpuga
Copy link

fpuga commented Feb 15, 2019

It seems that the original motivation was explained in #5 but i agree that it will be nice have it in the README.

Also, i don't understand the example in the README. Afaik extra keyword is not used for variable data but for the logging.Formatter.

>>> import logging

>>> logging.error("Hello {world}", extra={"world":"Earth", "color": "blue"})                                                                                                                                                   
ERROR:root:Hello {world}

>>> import logging 
>>> logging.basicConfig(format="%(color)s - %(message)s") 
>>> logging.error("Hello {world}", extra={"world":"Earth", "color": "blue"})                                                                                                                                   
blue - Hello {world}

@afallou
Copy link
Contributor

afallou commented Feb 15, 2019

@fpuga that is correct.
We use two custom formatters. One we wrote for console output, that will format the log message using those extra arguments. The other from pythonjsonlogger will add the extra dict to the logged JSON; we use it for the logs we send to our logging aggregation system (we use Loggly).

I agree the README could be made clearer there.

@afallou
Copy link
Contributor

afallou commented Feb 16, 2019

@Julian-O a paragraph on motivation was added to the README in #19

@fpuga
Copy link

fpuga commented Feb 17, 2019

Thanks @afallou, it's more clear now.

@FanchenBao
Copy link

Since it is clear now that extra dict is only used for formatter, please also update the example. It could cause confusion.

@Dreamsorcerer
Copy link

Dreamsorcerer commented Jun 4, 2020

Has caused confusion. Just spent half an hour trying to figure out why the variables weren't being output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants