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

KLogger does not properly populate SLF4J/Logback ILoggingEvent "message" field #449

Open
neeme-praks-sympower opened this issue Oct 14, 2024 · 15 comments

Comments

@neeme-praks-sympower
Copy link
Contributor

neeme-praks-sympower commented Oct 14, 2024

Context

SLF4J/Logback ILoggingEvent (source) has two important fields / getter methods:

  • message - contains the message template (e.g. "Hello {}!")
  • formattedMessage - contains the final formatted/merged log message (e.g. "Hello world!")

This distinction is useful -- for example, we use this to aggregate log messages.

Issue

KLogger relies on Kotlin string interpolation and as a result, the message field contains the same formatted message as formattedMessage, breaking the possibilities for aggregation.

Proposal

Introduce a new field in KLoggingEventBuilder that could contain the log message template (e.g. "Hello $variable!"):

This will make it easier for me to implement something on top of KLogger to fill that field properly (I'm thinking of implementing a Kotlin compiler plugin).

Copy link

Thank you for reporting an issue. See the wiki for documentation and slack for questions.

@oshai
Copy link
Owner

oshai commented Oct 16, 2024

Can you please provide some more info?
You mean that the un-formatted message will be added by the compiler plugin? if so, sounds like an interesting idea. In the past I thought of using a compiler plugin to add line numbers, file name, etc'. But never implemented it.

@neeme-praks-sympower
Copy link
Contributor Author

un-formatted message will be added by the compiler plugin?

Exactly.

In the past I thought of using a compiler plugin to add line numbers, file name, etc'

Also possible, yes.

@oshai
Copy link
Owner

oshai commented Oct 22, 2024

ok, would you like to send a PR for that or let me do the proposal?

@neeme-praks-sympower
Copy link
Contributor Author

I'm mostly interested in the unformatted message. In case you have broader scope in mind then it is probably better if you come up with a proposal.

@oshai
Copy link
Owner

oshai commented Oct 22, 2024

I suggest that you'll do the initial PR for the unformatted message (maybe call it rawMessage) and I will add on top another PR. Make sense?

@neeme-praks-sympower neeme-praks-sympower changed the title KLogger does not properly populate SLF4J ILoggingEvent "message" field KLogger does not properly populate SLF4J/Logback ILoggingEvent "message" field Oct 22, 2024
@neeme-praks-sympower
Copy link
Contributor Author

I just understood that the ILoggingEvent is actually part of Logback and SLF4J LoggingEvent also needs to support propagating the formatted message. So I submitted an issue for that: qos-ch/slf4j#437

@neeme-praks-sympower
Copy link
Contributor Author

One proposal from SLF4J developers (the original comment):

If they agree to extend KLoggingEventBuilder to accept an arguments parameter, you could write:

logger.atWarn { message = "foo $bar" }

and rewrite it via a compiler plugin into:

logger.atWarn {
  message = "foo {}"
  arguments = arrayOf(bar)
}

Since the fluent API of kotlin-logger ends up calling the fluent API of SLF4J, you would be able to have the correct LoggingEvent.getMessage() value.

What do you think about this change?

@oshai
Copy link
Owner

oshai commented Oct 23, 2024

Do you know of a way to make it accessible only to the plugin? Can the plugin work with private members?

@neeme-praks-sympower
Copy link
Contributor Author

Do you know of a way to make it accessible only to the plugin? Can the plugin work with private members?

The compiler plugin would be manipulating the (user-written) code that calls kotlin-logging API -- it does not manipulate the kotlin-logging code. So, the API that it would call would need to be public -- users can also call the same API themselves.

Some options, from the top of my head:

  • Extend the current API and allow SLF4J-style variables next to Kotlin-style variables (as suggested by SLF4J developers)
  • Introduce a new (internal and hidden) API, discourage users from using that.

I would prefer the first option.

@oshai
Copy link
Owner

oshai commented Oct 23, 2024

I would like to avoid the first option. It will also cause confusion for users in other platforms (not jvm).
How about calling logback directly not via slf4j? We have something similar for jul.

@neeme-praks-sympower
Copy link
Contributor Author

neeme-praks-sympower commented Oct 23, 2024

How about calling logback directly not via slf4j? We have something similar for jul.

That sounds interesting, I'll investigate that option. Any pointers to get me started faster?

EDIT: I think I found it: https://github.com/oshai/kotlin-logging/tree/master/src/jvmMain/kotlin/io/github/oshai/kotlinlogging/jul

@oshai
Copy link
Owner

oshai commented Oct 23, 2024

@neeme-praks-sympower
Copy link
Contributor Author

Here is my Logback backend for kotlin-logging and a proposal for how to pass in messageTemplate.
#452

@oshai
Copy link
Owner

oshai commented Dec 3, 2024

@neeme-praks-sympower anything left to do here?

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

2 participants