-
Notifications
You must be signed in to change notification settings - Fork 529
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
Add loguru
integration
#1994
Add loguru
integration
#1994
Conversation
Actually, this is the solution in comments under #653 adapted to codebase and tested as well. #653 (comment) I also changed `logging` integration to use methods instead of functions in handlers, as in that way we can easily overwrite parts that are different in `loguru` integration. It shouldn't be a problem, as those methods are private and used only in that file. Please let me know if I missed something, or you have more ideas how to test this.
So those will not be referenced in docs
@antonpirker - want to take a look at this? |
I am a bit swamped right now, so this could take some time. |
Mypy will give ` Cannot find implementation for "loguru"` if not doing this.
I do not really understand the error. This doesn't look like my problem. |
Sometimes our tests are a bit flaky. I have updated your branch with the current master, so the tests will run again. |
Hey @PerchunPak I finally had some time to play with this integration. Nice work! I create a small test project for testing: from loguru import logger as loguru_logger
import sentry_sdk
import sentry_sdk.integrations.loguru
sentry_sdk.init(
dsn="...",
integrations=[
sentry_sdk.integrations.loguru.LoguruIntegration(),
],
traces_sample_rate=1.0,
debug=True,
)
def main():
loguru_logger.debug("Guru: I am ignored")
loguru_logger.info("Guru: I am a breadcrumb")
loguru_logger.error("Guru: I am an event", extra=dict(bar=43))
loguru_logger.exception("Guru: An exception happened")
if __name__ == "__main__":
main() So the The error message contains all the formatted Loguru message with all the information. This is not the best developer experience. Could you please change this, to only contain the message from the call to "loguru_logger.exception()"? One other thing I noticed is, that Is this just how Loguru works, or am I doing something wrong? |
Unfortunately, looks like no. Loguru parses the message before it is passed to our handler, so we get only a formatted message. We can somehow cut unnecessary parts (e.g. regex or build the unnecessary parts of message by ourselves to cut it) or we can change the format of messages to contain only plain messages (
Both, you don't catch any exception, so loguru doesn't provide it. This code works, and also doesn't send formatted messages by loguru in the report name. try:
1 / 0
except ZeroDivisionError:
loguru_logger.exception("Guru: An exception happened") If there is no exception, loguru prints >>> from loguru import logger
>>> logger.exception("Guru: An exception happened")
2023-04-26 21:29:02.780 | ERROR | __main__:<module>:1 - Guru: An exception happened
NoneType: None |
We should not do this. We should never change the behavior of the users code, just add functionality on top of it. Can we "build the unnecessary parts of message" with the formatting the users has set so it always works, no matter what the configuration of the user looks like? |
Or one other option: can we hook into something that is run before our handler, where we have access to the raw message? |
loguru.add("logs.txt", format="some custom format") # and maybe some rotation, doesn't matter So if we will monkeypatch logger.info("hi!")
# calls all handlers:
# 0 - default (stderr logger)
# 1 - what we added upper (log into file)
# 2 - sentry's breadcrumb handler # only if INFO >= logging_level > ERROR
# 3 - sentry's event handler # only if ERROR >= logging_level All of them can have custom format, so we can't catch what the user actually sees in the terminal. BUT we can provide configuration options that we will send into Also, as I understand, all info that provides
I did this via appending this code to the end of our loguru integration module import loguru._simple_sinks
class PatchedStandardSink(loguru._simple_sinks.StandardSink):
def write(self, message):
parsed_record = None
actual_message = message.record["message"]
def handler_hande(record):
nonlocal parsed_record
parsed_record = record
original_handler_handle = self._handler.handle
self._handler.handle = handler_hande
original_write(self, message)
parsed_record.message = actual_message
parsed_record.msg = actual_message
original_handler_handle(parsed_record)
original_write = loguru._simple_sinks.StandardSink.write
loguru._simple_sinks.StandardSink.write = PatchedStandardSink.write But this gives absolutely the same result as just setting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. It does what it should and the mentioned problems we can not fix because of the nature of loguru, so this is good to be merged.
Hey @PerchunPak ! Again: Great work, thanks again for this contribution! We now need also some documentation for this. Could you please create a PR in our docs repo. You can just copy the Redis docs and change it to fit Loguru: https://github.com/getsentry/sentry-docs/blob/master/src/platforms/python/common/configuration/integrations/redis.mdx#L19 Thanks! And if you want to have e small thank you gift please send me your shipping address and t-shirt size to anton.pirker@sentry.io |
Thank you for the merge!
It is already done here. |
Hey @PerchunPak ! Your contribution has been released: https://github.com/getsentry/sentry-python/releases/tag/1.23.0 If you want a little thank you for your contribution please send me your shipping address (and tshirt size) to anton.pirker@sentry.io! |
To include getsentry/sentry-python#1994.
Actually, this is the solution in comments under #653 adapted to codebase and tested as well.
#653 (comment)
I also changed
logging
integration to use methods instead of functions in handlers, as in that way we can easily overwrite parts that are different inloguru
integration. It shouldn't be a problem, as those methods are private and used only in that file.Please let me know if I missed something, or you have more ideas how to test this.
Fixes #653.