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

Configurable contextItems for Klogger #80

Closed
skotsj opened this issue Jun 10, 2022 · 13 comments
Closed

Configurable contextItems for Klogger #80

skotsj opened this issue Jun 10, 2022 · 13 comments
Labels
enhancement New feature or request

Comments

@skotsj
Copy link

skotsj commented Jun 10, 2022

It would be great if extraction of contextItems could be configurable, ability to provide a lambda etc.

This way I could implement my own function to extract fields from for example ReactorContext (kotlinx.coroutines.reactor.ReactorContext) to avoid adding klogging context and mapping fields there on all log invocations.

I suppose I could implement my own Klogger for this, but base support seems nice to have.

@mjstrasser
Copy link
Member

Hi @skotsj

Thanks for checking out Klogging! Your suggestion sounds good but I am not sure I understand it.

Do you mean have selected fields from other elements in the coroutine context automatically included in log messages? I think that is a great idea, and using lambdas sounds like a good mechanism for implementing it.

Here‘s how it might work.

  • There is some way of configuring Klogging to find and use each lambda.
  • When one or lambdas are specified, KloggerImpl.contextItems() calls them and merges the returned maps with information explicitly added to LogContext.

Did I get that right?

@skotsj
Copy link
Author

skotsj commented Jun 12, 2022

That is correct.

@mjstrasser mjstrasser added the enhancement New feature or request label Jun 14, 2022
@mjstrasser
Copy link
Member

mjstrasser commented Jun 14, 2022

@skotsj The newest 0.5.0-SNAPSHOT builds of Klogging include code that may do what you want.

This code enables Klogging to call a lambda on a CoroutineContext.Element instance in the current context. There are tests in OtherContextTest.kt.

You configure it with the configuration DSL like this:

val reactorContextItems: (ReactorContext) -> Map<String, Any?> = { context ->
    // Items to be included in log events
}

loggingConfiguration {
    // Other configuration using DSL
    addContextItemExtractor(ReactorContext, reactorContextItems)
}

If you configure Klogging using JSON, you can add to that configuration by specifying loggingConfiguration(append = true). But you must do that in code after declaring a logger, because the JSON configuration is lazily loaded only when a logger is declared. Something like this:

class LoggingSetup {

    private val logger = logger<LoggingSetup>() // Loads JSON configuration

    init {
        val reactorContextItems: (ReactorContext) -> Map<String, Any?> = { context ->
            // Items to be included in log events
        }
        loggingConfiguration(append = true) {
            addContextItemExtractor(ReactorContext, reactorContextItems)
        }
    }
}

@skotsj
Copy link
Author

skotsj commented Jun 14, 2022

Looks good, can't really test as I'm currently working in a proxied environment without snapshots.

@mjstrasser
Copy link
Member

I will include the functionality into a release soon (possibly within a day).

I realised that using the existing Klogging configuration is the wrong model. I will provide an object to manage this kind of configuration.

I am also thinking of defining the lambda as an extension function on CoroutineContext.Element, thus:

// `EventItems` is a type alias for `Map<String, Any?>`
val reactorContextItems: ReactorContext.() -> EventItems {
    // Items to include in log events
}

Do you prefer that to (ReactorContext) -> EventItems?

@mjstrasser
Copy link
Member

I have made the following changes, which are available in klogging-jvm:0.4.7 (also via slfj-klogging:0.2.6 and klogging-spring-boot-starter:0.2.3):

  • Context item extractor is an extension function on CoroutineContext.Element.

  • Use the object Context to add a context item extractor instead of via other configuration, e.g.:

    val reactorContextItems: ReactorContext.() -> EventItems {
        // Items to include in log events
    }
    
    Context.addContextItemExtractor(ReactorContext, reactorContextItems)
  • You can specify constant context items to be included in all log events, for example:

    Context.addBaseContext("app" to "SpecialApp")

I will document these in more detail soon.

@skotsj
Copy link
Author

skotsj commented Jun 15, 2022

Oh, I didn't notice/forgot, but the extractor needs to be a coroutine, accessing context var is suspended. Shouldn't be an issue to fix I guess seeing as contextITems is already a coroutine.

@mjstrasser
Copy link
Member

Of course, I should have thought of that. My tests were a bit dumb.

@mjstrasser
Copy link
Member

I have made two changes to the extraction function/lambda in klogging-jvm:0.4.8:

  • It is a suspend function
  • It takes CoroutineContext.Element as a parameter instead of as receiver (again: this made declaration and testing easier)

I changed OtherContextTest to define a real function and use it. I found that the call to Context.addContextItemExtractor() needed an unsafe cast for the function argument.

This is a tricky generic type problem I have not solved yet.

@skotsj
Copy link
Author

skotsj commented Jun 15, 2022

Tested 0.4.8. Works like a charm, much appreciate the quick implementation ;)

@mjstrasser
Copy link
Member

That is good news. It made my day 😄

I would like to know how Klogging is being used.

Are you able to share some code? Are you using Spring Boot or Ktor? Are you logging to stdout and collecting logs or sending direct to an aggregator?

@mjstrasser
Copy link
Member

I have released klogging-jvm:0.4.9 that moves the unsafe cast into library code.

@skotsj
Copy link
Author

skotsj commented Jun 16, 2022

Can't share any code unfortunately, I'm using spring boot with webflux using coroutines and logging json to stdout on k8s gathered by elk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants