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 evaluated structured arguments #106

Closed
kschlesselmann opened this issue Feb 24, 2020 · 14 comments
Closed

Lazy evaluated structured arguments #106

kschlesselmann opened this issue Feb 24, 2020 · 14 comments

Comments

@kschlesselmann
Copy link

Is there a way to lazy evaluate log statements that use net.logstash.logback.argument.StructuredArgument?

Currently I can only use logger.trace("msg", kv("foo", bar())) and if bar takes some time to compute I have to wrap everything with logger.isTraceEnabled. It would be nice if I could somehow fall back to some variant of logger.trace { ??? } for this case.

@oshai
Copy link
Owner

oshai commented Feb 24, 2020

Is it used with the slf4j api? If it is, we can add that. Otherwise you can write such an extension method yourself pretty easily I guess.

@kschlesselmann
Copy link
Author

@oshai
Copy link
Owner

oshai commented Feb 25, 2020

@kschlesselmann if I get you correct you're trying to do something like:

logger.trace("my msg with placeholder: {}", kv("foo", bar()))

If so, you can simply replace it with:

logger.trace { "my msg without placeholder: ${kv("foo", bar())}" }

And it will work as you expect (no resolution of kv if level is disabled)

@kschlesselmann
Copy link
Author

@oshai This would work, yes. If you don't provide the kv in the message itself though it keeps the message clean (just my msg without placeholder) and appends the structured values to generated JSON (which is then displayable by kibana/cloudwatch logs insights etc).

So it would be great to have a variant where I do not have to put the additional information in the log string.

@oshai
Copy link
Owner

oshai commented Feb 25, 2020

oh, I got that. How do you think such method signature should be?

@kschlesselmann
Copy link
Author

kschlesselmann commented Feb 26, 2020

Good question. I too thought about that (tried to design my own extension) but I could not come up with a nice API. Do you have any suggestions?

@kschlesselmann
Copy link
Author

@kschlesselmann if I get you correct you're trying to do something like:

logger.trace("my msg with placeholder: {}", kv("foo", bar()))

If so, you can simply replace it with:

logger.trace { "my msg without placeholder: ${kv("foo", bar())}" }

And it will work as you expect (no resolution of kv if level is disabled)

Hi @oshai . I tried your suggestion today and sadly the kv isn't added to the generated JSON this way. So this does not work :-(

@kschlesselmann
Copy link
Author

kschlesselmann commented Feb 27, 2020

@oshai I thought a little bit about that and I've come up with something like that

internal fun KLogger.trace(vararg objs: () -> Pair<String, Any>, msg: () -> String) {
    if (this.isTraceEnabled) {
        val args = objs
                .map { it.invoke() }
                .map { kv(it.first, it.second.toString()) }
                .toTypedArray()

        this.trace(msg.invoke(), *args)
    }
}

fun foo() {
    logger.trace({ "foo" to "bar" }) { "lala" }
}

This impl is tied to the logstash encoder though to be a bit more "ideomatic". It's easy to use the SLF4J API though if you'd just use vararg objs: () -> Any and provide the kv yourself.

What do you think?

@oshai
Copy link
Owner

oshai commented Feb 27, 2020

Yes, I think it is better to stick with the slf4j api.
I am not sure how "pretty" is that format, but it sounds like it will be helpful for your case.
Do you want to create a PR for that?

@kschlesselmann
Copy link
Author

Yep sure, I'll see what I can do 👍

@newzubakhin
Copy link

Hi Gents. Is there any update on this feature? I'd like to see this in kotlin-logging.
Structured logging is a common and useful thing in a distributed environment.

@oshai
Copy link
Owner

oshai commented Jun 10, 2020

@kschlesselmann - is there any update on that?

@kschlesselmann
Copy link
Author

Sorry … nothing yet. I wasn't sure if using so many lambdas would lead to negative performance somewhere and had no time yet to investigate this any further :-(

We currently wrap our statements with

if (logger.isDebugEnabled) { … }

but I'd like to see this feature too ;-)

I'm happy to help but as it looks right now I won't have the time to drive this feature forward as main developer. Sorry guys.

@oshai
Copy link
Owner

oshai commented Aug 18, 2020

Thanks @kschlesselmann I am closing it for now, until someone decide to pick it up.

@oshai oshai closed this as completed Aug 18, 2020
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

3 participants