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

add extra to default text output format #1200

Open
trim21 opened this issue Sep 11, 2024 · 10 comments
Open

add extra to default text output format #1200

trim21 opened this issue Sep 11, 2024 · 10 comments
Labels
feature Request for adding a new feature

Comments

@trim21
Copy link
Contributor

trim21 commented Sep 11, 2024

This is a suggestion to change default loguru text output format, or a new option to append extra to non-serialized text format.

There are many structured logging will include extra to text output, but loguru just ignore them.

Hope loguru could support this. For example, append key1=value1 key2=value2 to default text output.

so a code snippet will output 2024-09-12 00:17:01.261 | INFO | __main__:<module>:5 - hello world task=main pid=1

from loguru import logger


with logger.contextualize(task="main", pid=1):
    logger.info("hello world")

golang stdlib log/slog:

2022/11/08 15:28:26 INFO hello count=3

zerolog(golang)
image

pino(nodejs)
image

structlog(python)
image

@Delgan Delgan added the feature Request for adding a new feature label Sep 22, 2024
@Delgan
Copy link
Owner

Delgan commented Sep 22, 2024

Thanks for the comparison with different frameworks. That's very helpful.

I'd already thought about it and I'm all for it. Structured logging is the way to go.

My plan is to add a new entry to the log record, for example a data dict. This would be filled by the keyword arguments of the logged message instead of extra currently. This is because it should be viewed as semantically different.

Then, add {data} to the current default format, which would be nicely formatted (if non-empty), e.g.:

logger.info("Hello world", foo="bar", baz=123)
# => 2024-09-22 19:39:58.381 | INFO     | __main__:<module>:1 - Hello world (foo="bar", baz=123)

@trim21
Copy link
Contributor Author

trim21 commented Sep 22, 2024

will kwargs from logger.contextualize be included?

@Delgan
Copy link
Owner

Delgan commented Sep 22, 2024

Initially, I believed it should not, but maybe I'm wrong. I haven't given it enough thought. I need to look into the current usage made of contextualize() in relation to bind().

@elvizlai
Copy link

elvizlai commented Dec 11, 2024

that should be really useful.

but for example :

logger.opt().bind(a=1).info("good {}, nice {b}", "morning", b="job", c="come on")

may be logout:
# => 2024-09-22 19:39:58.381 | INFO     | __main__:<module>:1 - good morning, nice job (c="come on")

should "a=1" included?

and I also read loguru source code, it can not clearly distinguish between 'bind' and 'format-placeholder', maybe we should add some new func with propagation to handler this.

boring but works like this

logger.opt().with(a=1, b="2").with(c=3).info("xxx")
# => - xxx (a=1, b="2", c=3)

@trim21
Copy link
Contributor Author

trim21 commented Dec 19, 2024

that should be really useful.

but for example :

logger.opt().bind(a=1).info("good {}, nice {b}", "morning", b="job", c="come on")

may be logout:
# => 2024-09-22 19:39:58.381 | INFO     | __main__:<module>:1 - good morning, nice job (c="come on")

should "a=1" included?

and I also read loguru source code, it can not clearly distinguish between 'bind' and 'format-placeholder', maybe we should add some new func with propagation to handler this.

boring but works like this

logger.opt().with(a=1, b="2").with(c=3).info("xxx")
# => - xxx (a=1, b="2", c=3)

I think it should

@Delgan
Copy link
Owner

Delgan commented Dec 30, 2024

@elvizlai Regarding the following example:

logger.bind(a=1).info("good {}, nice {b}", "morning", b="job", c="come on")

I think that the next version of Loguru must clearly distinguish between :

  1. Parameters intended for formatting ("morning" here).
  2. Parameters intended for contextual / structured logging (c="come on" here).
  3. Parameters intended to identify logger instances and specialize handlers (a=1 here).

Currently there is some undesirable overlap: keyword arguments end up both in the extra record dict and are also used for formatting. They have the same behavior as bind() and positional arguments. This design has led to a lot of problems and complications, and I think it's imperative that we rethink it.

I was planning to do is the following:

  1. Positional arguments will be reserved for formatting only, as it is currently.
  2. Keyword arguments will no longer be used for formatting. Instead, they will fill a new data entry in the record dict. The default format will also contains a new {data} field that will automatically format the values as discussed in this ticket.
  3. The bind() method will stay as is, but will be the only way to populate the extra entry in the record dict.

In such scenario, that means the bind() values and keyword arguments are not mixed, and therefore, the bind() values would not appear by default in the formatted {data}.

This is because, I believe keyword arguments to be contextual to the log line, and I don't want this to inadvertently override a value configured by bind(). For example:

from loguru import logger

# Imagine a filter or sink implements a specialization for this named logger.
named_logger = logger.bind(name="my-specific-logger")

named_logger.info("App initialized", count=123)
named_logger.info("New user connected", name=username)  # Oops!

I believe there should be a conceptual difference between bind() usage and keyword arguments. Do you see it otherwise @trim21?

@trim21
Copy link
Contributor Author

trim21 commented Dec 31, 2024

missing logger.contextualize

@Delgan
Copy link
Owner

Delgan commented Dec 31, 2024

missing logger.contextualize

Yes, I'm not sure yet if it falls more into the category of bind() or contextual keyword arguments. I remember we shortly discussed this before.

@trim21
Copy link
Contributor Author

trim21 commented Dec 31, 2024

I vote for .bind() because it's implicit like .bind()

@Delgan
Copy link
Owner

Delgan commented Dec 31, 2024

Yes, it's also what seems most natural to me.

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

No branches or pull requests

3 participants