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

Lazy log message formatting #1001

Closed
TheJJ opened this issue Apr 22, 2018 · 8 comments
Closed

Lazy log message formatting #1001

TheJJ opened this issue Apr 22, 2018 · 8 comments
Labels
improvement Enhancement of an existing component just do it You can start working on this, there should be nothing left to discuss lang: c++ Done in C++ code lang: python Done in Python code

Comments

@TheJJ
Copy link
Member

TheJJ commented Apr 22, 2018

Our logging system does not support lazy message formatting, like the python logging module.

from log import info

# current version: always performs string formatting
var = "with formatting"
info("some message %s" % var)

# new version: should only format the string if the message is actually emitted
var = "lazyness"
info("everything is better™ with ", var)

The functions are in openage/log/__init__.py and then relay the messages to libopenage.
The formatting of messages should thus happen (either in C++ or the python part, not sure) only if the log level of the message is higher than the minimum log level of a sink that gets the message.

Possible approach:

  • Use the python standard logging module so it logs to our C++ as backend
  • Notify the logging module about log levels from C++
  • Let all the magic be done by the standard logging module then, our functions can be aliases, or we just only use logging all over the python codebase.

This is an optimization, so no real priority 😉

@TheJJ TheJJ added improvement Enhancement of an existing component lang: c++ Done in C++ code lang: python Done in Python code labels Apr 22, 2018
@TheJJ TheJJ added the just do it You can start working on this, there should be nothing left to discuss label May 2, 2018
@kev946
Copy link
Contributor

kev946 commented May 18, 2018

Are we just trying to do lazy formatting on the messages sent from the Python side?

@TheJJ
Copy link
Member Author

TheJJ commented May 18, 2018

We can do both, in C++ and Python. The C++ operator << could be abused for that, and we would have to store references to the message parts, to merge them later if the message has to be formatted.

We could add that C++ (which controls the loglevel) notifies Python of the current level, so messages are not even transported C++ if the message is not important enough.

@TheJJ
Copy link
Member Author

TheJJ commented May 23, 2018

I added a possible approach section above, with the main idea to create a backend for our C++ logger for the python logging module.

@kev946
Copy link
Contributor

kev946 commented May 27, 2018

For the Python side, maybe we could create a CppHandler class derived from Handler. We override setLevel and emit to call into the C++ side. Does this sound good?

@TheJJ
Copy link
Member Author

TheJJ commented May 27, 2018

Yes, except that we have to call setLevel from the C++ side so the C++-level is set in the python world.

kev946 added a commit to kev946/openage that referenced this issue Jun 4, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 4, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 4, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 7, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 7, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 8, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 23, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 23, 2018
kev946 added a commit to kev946/openage that referenced this issue Jun 26, 2018
@TheJJ
Copy link
Member Author

TheJJ commented Nov 5, 2018

C++ side was done in #1029.

@TheJJ
Copy link
Member Author

TheJJ commented Nov 5, 2018

Python side in #1025.

@TheJJ TheJJ closed this as completed Nov 5, 2018
@TheJJ
Copy link
Member Author

TheJJ commented Nov 5, 2018

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Enhancement of an existing component just do it You can start working on this, there should be nothing left to discuss lang: c++ Done in C++ code lang: python Done in Python code
Projects
None yet
Development

No branches or pull requests

2 participants