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

Spring Webflux: CoRouterFunctionDsl should expose the used CoroutineContext #27010

Closed
FrontierPsychiatrist opened this issue Jun 1, 2021 · 8 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Milestone

Comments

@FrontierPsychiatrist
Copy link

Affects: 5.3.7


I am using the CoRouterFunctionDsl to build a Spring Webflux application with suspend handlers in Kotlin. Recently I wanted to add some SLF4J MDC values (e.g. an AWS X-Ray Trace id) which requires the usage of https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-thread-context-element/. MDC is ThreadLocal based which does not play well with coroutines.

Adding such a ThreadContextElement can be done in a generic way (so it doesn't have to be repeated in each handler) in the private method asHandlerFunction in the class CoRouterFunctionDsl:

mono(Dispatchers.Unconfined) {
    init(it)
}

but there is no way of doing that. For now I copied the class and changed the function myself. The data I need there is based on the subscriber context of the Mono (coming from a WebFilter) and the ServerRequest, so it would be neat to inject something like coroutineContextProvider: (req: ServerRequest) -> Mono<CoroutineContext> into the DSL:

private val contextProvider = (req: ServerRequest) ->
  Mono.deferContextual { ctx ->
    // Extract data from req and ctx
    // Return a coroutine context
  }

coRouter(contextProvider) {
  GET("/info", ...)
}

// In CoRouterFunctionDsl

private fun asHandlerFunction(init: suspend (ServerRequest) -> ServerResponse) = HandlerFunction { req ->
    contextProvider(req)
      .flatMap { coroutineContext ->
              mono(coroutineContext) {
                  init(req)
              }
      }
}

I hope that makes sense. If there is an easier solution that I oversaw I am of course also open to that.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 1, 2021
@poutsma
Copy link
Contributor

poutsma commented Jun 2, 2021

@sdeleuze Could you take a look at this one, please?

@sdeleuze
Copy link
Contributor

@poutsma Do we provide a way to do the same thing on Reactive side via Reactor context? If not, what would that look like and would it b something we want to implement?

@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Nov 8, 2021
@sdeleuze sdeleuze added the theme: kotlin An issue related to Kotlin support label Jan 19, 2022
@FrontierPsychiatrist
Copy link
Author

@sdeleuze I think in a pure Reactor implementation this is easier since the call chain "stays inside the framework". It can be easily done by adding a WebFilter that uses Mono.contextWrite. Adding a filter in coRouter does not have the same effect as there will be another "switch" back to Reactor after the filter and the coroutine context will be "forgotten".

@sdeleuze sdeleuze removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 15, 2022
@sdeleuze sdeleuze added this to the 6.x Backlog milestone Mar 15, 2022
@sdeleuze
Copy link
Contributor

I am not sure how this should be implemented and exposed, but indeed getting access to the CoroutineContext seems a valid use case.

@sdeleuze
Copy link
Contributor

sdeleuze commented Aug 28, 2023

I am considering adding a fun context(provider: suspend (ServerRequest) -> CoroutineContext) method to the DSL in Spring Framework 6.1 to provide a custom CoroutineContext in various places of the DSL. It would allow to write code like:

coRouter {
  context { request ->
    delay(1) // Can invoke suspending functions
    CoroutineName(request.headers().firstHeader("Custom-Header")!!)
  }
  GET("/") {
    // ...
  }
}

@FrontierPsychiatrist
Copy link
Author

That seems like pretty much exactly what would help me! E.g. something like this is what I do at the moment)

coRouter {
  context { request ->
    MDC.put("traceId", req.header("traceId"))
    MDC.put("userId", req.awaitPrincipal().name)
    // Not sure if NonCancellable will work like this, in my tests the mono() creator did not accept it
    kotlinx.coroutines.slf4j.MDCContext() + NonCancellable
  }
  // mappings
}

@sdeleuze
Copy link
Contributor

Merged, please try 6.1.0-SNAPSHOT builds once created by the CI and provide a feedback if possible.

@FrontierPsychiatrist
Copy link
Author

I just tested it in a small application and it worked fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants