Passing MDC context to newSuspendedTransaction

[marioalvial]

I`m trying to pass the MDC context to newSuspendedTransaction, but the function only accepts a Dispatcher, not CoroutineContext. So when I execute the following code:

override suspend fun save(event: LedgerEvent, atomicFunction: () -> Unit) {
        MDC.get("correlation_id") // 8d3b7750-8626-4390-8750-7f2eef401570
        newSuspendedTransaction {
            LedgerEventsTable.insert {
                it[id] = event.eventId
                it[bankAccountId] = event.bankAccountId
                it[revision] = event.revision
                it[type] = event.type
                it[deserializer] = event.deserializer()
                it[createdAt] = event.createdAt.toLocalDateTime()
                it[data] = entryEventsDbMapper.toData(event)
            }

            MDC.get("correlation_id") // null

            atomicFunction()
        }
    }

I got null value as the return of MDC.get("correlation_id") when it is executed inside coroutine scope.

How can I pass the current MDC Context to the new suspendedTransaction function?

[AlexeySoshin]

This is not directly related to Exposed, so not sure if it should be an issue.

But simply capturing the value before invoking newSuspendedTransaction should work:

val correlationId = MDC.get("correlation_id") 
newSuspendedTransaction {
    println(correlationId) // 8d3b7750-8626-4390-8750-7f2eef401570
}

[marioalvial]

Hi @AlexeySoshin thanks for the answer. And yes my question is not directly related to Exposed, but I think that applying current context to the new context created is something that exposed coroutine integration should do, shouldn`t it?

Capturing the value before invoking the newSuspendedTransaction is not scalable. If the function atomicFunction() needs to use values from mdc context I will need to pass one by one if I follow your suggestion.

Using coroutine we can pass the context like this:

MDC.put("kotlin", "rocks") // Put a value into the MDC context

launch(MDCContext()) {
    logger.info { "..." }   // The MDC context contains the mapping here
}

Shouldn't the function newSuspendedTransaction() also be able to pass context this way?

[AlexeySoshin]

There's a subtelty there, I think. newSuspendedTransaction does not launch a coroutine, unlike launch. So you don't have a coroutine context. It's a suspending block, but it's not a coroutine.

You can see the details here: https://github.com/JetBrains/Exposed/blob/e926b0cdc88edced464ab16e4a7088dfc55670fc/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/transactions/experimental/Suspended.kt#L91

Having said that, I may be missing something. If you would like to provide a test project to reproduce that issue (I'm particularly interested in your definition of MDCContext), I would be happy to take a look.

[marioalvial]

Hi @AlexeySoshin MDCContext is from implementation("org.jetbrains.kotlinx:kotlinx-coroutines-slf4j:1.6.0") lib.

/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines.slf4j

import kotlinx.coroutines.*
import org.slf4j.MDC
import kotlin.coroutines.AbstractCoroutineContextElement
import kotlin.coroutines.CoroutineContext

/**
 * The value of [MDC] context map.
 * See [MDC.getCopyOfContextMap].
 */
public typealias MDCContextMap = Map<String, String>?

/**
 * [MDC] context element for [CoroutineContext].
 *
 * Example:
 *
 * ```
 * MDC.put("kotlin", "rocks") // Put a value into the MDC context
 *
 * launch(MDCContext()) {
 *     logger.info { "..." }   // The MDC context contains the mapping here
 * }
 * ```
 *
 * Note that you cannot update MDC context from inside of the coroutine simply
 * using [MDC.put]. These updates are going to be lost on the next suspension and
 * reinstalled to the MDC context that was captured or explicitly specified in
 * [contextMap] when this object was created on the next resumption.
 * Use `withContext(MDCContext()) { ... }` to capture updated map of MDC keys and values
 * for the specified block of code.
 *
 * @param contextMap the value of [MDC] context map.
 * Default value is the copy of the current thread's context map that is acquired via
 * [MDC.getCopyOfContextMap].
 */
public class MDCContext(
    /**
     * The value of [MDC] context map.
     */
    @Suppress("MemberVisibilityCanBePrivate")
    public val contextMap: MDCContextMap = MDC.getCopyOfContextMap()
) : ThreadContextElement<MDCContextMap>, AbstractCoroutineContextElement(Key) {
    /**
     * Key of [MDCContext] in [CoroutineContext].
     */
    public companion object Key : CoroutineContext.Key<MDCContext>

    /** @suppress */
    override fun updateThreadContext(context: CoroutineContext): MDCContextMap {
        val oldState = MDC.getCopyOfContextMap()
        setCurrent(contextMap)
        return oldState
    }

    /** @suppress */
    override fun restoreThreadContext(context: CoroutineContext, oldState: MDCContextMap) {
        setCurrent(oldState)
    }

    private fun setCurrent(contextMap: MDCContextMap) {
        if (contextMap == null) {
            MDC.clear()
        } else {
            MDC.setContextMap(contextMap)
        }
    }
}

[AlexeySoshin]

Thanks, that's helpful. MDCContext() creates a new empty coroutine scope.

If you want to pass the entire MDC context, you can do it like that:

MDC.put("kotlin", "rocks")

newSuspendedTransaction(context = MDCContext(MDC.getCopyOfContextMap())) {
    println("Hello ${MDC.get("kotlin")}") 
}

[marioalvial]

Hi @AlexeySoshin you dont even need to pass the context map as a parameter because the default value of this parameter already is the result of the function MDC.getCopyOfContextMap().

But the problem is that the function newSuspendedTransaction does not accept the MDCContext instance (it highlights as a compile error) because the function accepts a CoroutineDispatcher and the MDCContext is a CoroutineContext. I think this PR fixed this issue: #1592

[AlexeySoshin]

The solution from above did compile when I tested it, but I'm testing it on Exposed directly, so maybe this fix was there, but not released yet, though.

Do you consider the issue resolved, or is there still a problem?

[Tapac]

@marioalvial after #1592 newSuspendedTransaction accepts CoroutineContext instead of dispatcher. Will this resolve the issue?

[bog-walk]

Closing as the 3 options, newSuspendedTransaction, withSuspendTransaction, and suspendedTransactionAsync, all accept CoroutineContext as the first argument.

Please consider reopening this issue on YouTrack if the problem persists beyond passing an MDCContext instance to the functions.