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

How to wrap an existing logger? #88

Closed
lamba92 opened this issue Aug 10, 2019 · 18 comments
Closed

How to wrap an existing logger? #88

lamba92 opened this issue Aug 10, 2019 · 18 comments

Comments

@lamba92
Copy link

lamba92 commented Aug 10, 2019

Say you already have an org.slf4j.Logger object and you want to wrap around it the capabilities of KLoggeg. Is it possible?

@oshai
Copy link
Owner

oshai commented Aug 10, 2019

I am not sure I understand what you mean. Can you give a concrete example?

@lamba92
Copy link
Author

lamba92 commented Aug 10, 2019

I have a slf4j logger from a Ktor application and I'd like to wrap it.

@oshai
Copy link
Owner

oshai commented Aug 10, 2019

The api to do that is internal (ie: it's not part of the api), you can look for the method of wrapJLogger to see the implementation. But if I understand how ktor works since it is using slf4j you can just use kotlin-logging as usual and it will work (ie: let kotlin-logging instantiate the logger for you.

@lamba92
Copy link
Author

lamba92 commented Aug 10, 2019

Yeah i digged inside the code and saw it. I have little knowledge on how to use properly those loggers and I wasn't sure if creating a new logger was a good idea or not.

@oshai
Copy link
Owner

oshai commented Aug 10, 2019

I don't think you can, those classes are internal.
How do you obtain the slf4j logger?

@corneil
Copy link
Contributor

corneil commented Aug 11, 2019

You would ideally want to write code as follows:

companion object {
    val logger= KotlinLogging.wrap(slf4jLogger)
// OR 
    val logger= KotlinLogging.wrap { code.to.obtain.logger() }
}

I notice there is a method: mu.internal.KLoggerFactory.wrapJLogger that does this work we just need to expose it in KotlinLogging

@oshai
Copy link
Owner

oshai commented Aug 11, 2019

@corneil yes, you can see I mentioned this method in one of the comments above. The thing that is not clear to me is why do you need that over just using KotlinLogging.logger {} which will have a similar effect assuming the logger is of the current class.

@lamba92
Copy link
Author

lamba92 commented Aug 11, 2019

How one should behave then? A simple

val MyContext.klog by lazy { 
    KotlinLogging.logger{} 
}

Is ok? If so why?

@oshai
Copy link
Owner

oshai commented Aug 12, 2019

how about:

private val logger = KotlinLogging.logger {} 

in the file, just outside of the class? you can see the example here: https://github.com/MicroUtils/kotlin-logging#getting-started

@oshai
Copy link
Owner

oshai commented Aug 15, 2019

@lamba92 was the issue resolved?

@lamba92
Copy link
Author

lamba92 commented Aug 15, 2019

I have not tried it yet but I guess it is! Still, a wrapper would have been better!

@oshai
Copy link
Owner

oshai commented Aug 16, 2019

@lamba92 How about adding the wrap method to the KotlinLogging? also added an extension method:

actual object KotlinLogging {
    /**
     * This method allow defining the logger in a file in the following way:
     * val logger = KotlinLogging.logger {}
     */
    actual fun logger(func: () -> Unit): KLogger = KLoggerFactory.logger(func)

    actual fun logger(name: String): KLogger = KLoggerFactory.logger(name)

    fun logger(underlyingLogger: Logger) = KLoggerFactory.wrapJLogger(underlyingLogger)
}

fun Logger.toKLogger() = KotlinLogging.logger(this)

see also PR #90

@lamba92
Copy link
Author

lamba92 commented Aug 16, 2019

Oh great that is great :)
One last thing. Say that I recover my logger from a specific context with property logger.

One could now create an extension property klogger as such:

val MyContext.klogger
    get() = KotlinLogging.logger(logger)

It is fine but every time you use it you are creating a new object.

What about adding a Boolean value to the function that allows to save the newly created KLogger into a map with the underlying logger as key? You would be now able to check the map if it already been created and fetch it or create a new one, save it and return it!

In any case I guess a lazy could work as:

val MyContext.klogger by lazy { 
    KotlinLogging.logger(logger) 
}

But if you have more then one context it is not fine at all!

@oshai
Copy link
Owner

oshai commented Aug 16, 2019

You can do the map solution, but I think adding it to the library is not that great. You could argue the same even without the new functionality but I think that map will make things more complicated and might lead to memory leaks.

@corneil
Copy link
Contributor

corneil commented Aug 16, 2019 via email

@lamba92
Copy link
Author

lamba92 commented Aug 16, 2019

You can do the map solution, but I think adding it to the library is not that great. You could argue the same even without the new functionality but I think that map will make things more complicated and might lead to memory leaks.

Oh yeah, that's memory leaks 101 :P Anyway, the wrapper is fine :) Thanks!

@lamba92 lamba92 closed this as completed Aug 16, 2019
@lamba92
Copy link
Author

lamba92 commented Aug 16, 2019

Misclick the close button

@oshai
Copy link
Owner

oshai commented Aug 19, 2019

fixed in 1.7.6

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