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

Change default level severity no (e.g. to match pino) #949

Open
cjol opened this issue Aug 24, 2023 · 3 comments
Open

Change default level severity no (e.g. to match pino) #949

cjol opened this issue Aug 24, 2023 · 3 comments
Labels
question Further information is requested

Comments

@cjol
Copy link

cjol commented Aug 24, 2023

We have consolidated logging across multiple services. In most (node) services, we use pino, which has established a precedent for logging severity -- in particular that warning=40 and error=50. This is different from loguru's default levels, which has warnings at 30 and errors at 40. This means that our standardised (external) log-processing tools, which look for logs where level>40 to escalate them don't catch loguru's error-level logs.

I have tried to change the default level severities with e.g. logger.level("WARNING", no=40), but I get TypeError: Level 'WARNING' already exists, you can't update its severity no. Is there an alternative approach? I suppose a workaround would be to define fully custom levels and log with logger.log('_WARNING', '..'), but then I'd have to make sure no one accidentally uses logger.warning`.

@Delgan
Copy link
Owner

Delgan commented Aug 27, 2023

How are logs parsed by your external tools? Which kind of handler are you using with Loguru?

I would suggest to adapt your sink or formatter so that it convert Loguru default severity to the one you're interested in.

Here is an example using a global patching function:

def add_pino_log_level(record):
    log_level_adapter = {
        "TRACE": 10,
        "DEBUG": 20,
        "INFO": 30,
        "WARNING": 40,
        "ERROR": 50,
        "CRITICAL": 60,
    }
    pino_level = log_level_adapter.get(record["level"].name, record["level"].no)
    record["extra"]["pino_level"] = pino_level

logger.configure(patcher=add_pino_log_level)

logger.add("file.log", format="[{time}][{extra[pino_level]}] {message}")

@Delgan Delgan added the question Further information is requested label Sep 1, 2023
@cjol
Copy link
Author

cjol commented Sep 4, 2023

We're outputting logs as JSON via stderr, which gets aggregated up in AWS Cloudwatch, from where we run queries like {$.level >= 40}

i will experiment to see if I can override the level in a patch function (your example shows adding a secondary property which would be confusing in the output).

Out of interest, is it a deliberate design decision to make the levels immutable, or is there an impediment in some implementation detail of the library? It feels a little arbitrary from here on the outside and a patch/custom formatter like a slightly odd workaround!

@Delgan
Copy link
Owner

Delgan commented Sep 5, 2023

I would not advise to patch the level value of the record dict. Because Loguru relies on these values internally, I can't guarantee it will produce the result you're expecting. In general, it's best practice to modify only the extra attribute.

Clearly differentiating Loguru's levels from those of Pino seems to me to be the best solution. How would you have done it using the standard logging library? As far as I know, it doesn't allow you to modify existing levels. These are two completely different frameworks, so rather than unifying them, having a conversion function seems easier.

Level severity was made immutable after discussions in #209. It's not a technical restriction. This is to maintain a coherent system. If a library expects a "DEBUG" log to be less severe than "INFO", the logs would no longer make sense if the two no were reversed. We don't want to allow "DEBUG" messages to be more important than "ERROR" messages. It would be confusing.

There is one small problem, however. If two libraries independently decide to create a custom level with the same name, then they become incompatible. A user importing both would face an error. This is something I need to fix, so perhaps the restriction on level mutation will be loosened. But I don't know what I'm going to do yet.

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

No branches or pull requests

2 participants