MDCInsertingServletFilterとSentryAppenderを組み合わせると一部のMDCのvalueがnullになっているせいで、処理途中でNPEになってしまいsentryにイベント通知できない

掲題の通りの事象に遭遇して、解決に手間取ったのでメモを残しておきます。

その時ぼやき半分につぶやいていたのはこちら。

MDCInsertingServletFilterからすると、valueにnullをセットするのは正当な気がしています。

問題が顕在化している箇所はSentryAppenderがMDCの値をコピーする箇所であり、具体的にはCollectionUtils#shallowCopyを呼び出していてその中でnew ConcurrentHashMap<>(map)とするときに、ConcurrentHashMapのコンストラクタ内のガード節でnullチェックをしており、nullの場合はNPEをthrowしてしまっています。

MDCのvalueがnullの場合は、空文字に変換すればよいのでは？と一瞬思いましたが、これはデータの改ざん（not加工）であるような気もしており、筋が悪そうです。

何らかの方法でConcurrentHashMapを辞めれれば良さそうな気がします。1

workaroundですがMDCInsertingServletFilterをコピペしてCustomMDCInsertingServletFilterという名前で作り、valueがnullになりうる箇所はnullの場合に空文字にするという対応にしました。

package sample

import ch.qos.logback.classic.ClassicConstants
import org.slf4j.MDC
import java.io.IOException
import javax.servlet.Filter
import javax.servlet.FilterChain
import javax.servlet.FilterConfig
import javax.servlet.ServletException
import javax.servlet.ServletRequest
import javax.servlet.ServletResponse
import javax.servlet.http.HttpServletRequest

// オリジナルはch.qos.logback.classic.helpers.MDCInsertingServletFilter
// 値がないときはMDCのvalueにnullをputするが、それだとSentryAppender#createEvent内で、
// CollectionUtils.shallowCopyを使ってMDCの値をコピーしようとしているが、
// ConcurrentHashMapがnullのvalueを受け付けないので、NPEを送出してしまう
// OSS側で直してほしい気もするが、正しい挙動がなく直しようがなさそう
open class CustomMDCInsertingServletFilter : Filter {

    override fun destroy() {
        // do nothing
    }

    @Throws(IOException::class, ServletException::class)
    override fun doFilter(request: ServletRequest, response: ServletResponse, chain: FilterChain) {
        insertIntoMDC(request)
        try {
            chain.doFilter(request, response)
        } finally {
            clearMDC()
        }
    }

    private fun insertIntoMDC(request: ServletRequest) {
        MDC.put(ClassicConstants.REQUEST_REMOTE_HOST_MDC_KEY, request.remoteHost)
        if (request is HttpServletRequest) {
            val httpServletRequest = request
            MDC.put(ClassicConstants.REQUEST_REQUEST_URI, httpServletRequest.requestURI)
            val requestURL = httpServletRequest.requestURL
            if (requestURL != null) {
                MDC.put(ClassicConstants.REQUEST_REQUEST_URL, requestURL.toString())
            }
            MDC.put(ClassicConstants.REQUEST_METHOD, httpServletRequest.method)
            MDC.put(ClassicConstants.REQUEST_QUERY_STRING, httpServletRequest.queryString ?: "") // nullになりうるのでnullだった場合は空文字に置換
            MDC.put(ClassicConstants.REQUEST_USER_AGENT_MDC_KEY, httpServletRequest.getHeader("User-Agent") ?: "") // nullになりうるのでnullだった場合は空文字に置換
            MDC.put(ClassicConstants.REQUEST_X_FORWARDED_FOR, httpServletRequest.getHeader("X-Forwarded-For") ?: "") // nullになりうるのでnullだった場合は空文字に置換
        }
    }

    private fun clearMDC() {
        MDC.remove(ClassicConstants.REQUEST_REMOTE_HOST_MDC_KEY)
        MDC.remove(ClassicConstants.REQUEST_REQUEST_URI)
        MDC.remove(ClassicConstants.REQUEST_QUERY_STRING)
        // removing possibly inexistent item is OK
        MDC.remove(ClassicConstants.REQUEST_REQUEST_URL)
        MDC.remove(ClassicConstants.REQUEST_METHOD)
        MDC.remove(ClassicConstants.REQUEST_USER_AGENT_MDC_KEY)
        MDC.remove(ClassicConstants.REQUEST_X_FORWARDED_FOR)
    }

    @Throws(ServletException::class)
    override fun init(arg0: FilterConfig) {
        // do nothing
    }
}

これググっても全くHITせず、OSS側のコードをデバッグしないと突き止められませんでした。

MDCInsertingServletFilterとSentryAppenderを組み合わせて使っている事例は多い気がするので同じように困ってる人の役にたつ情報になってくれれば幸いです。

2021/04/06追記

ちょうどこの記事の投稿の直後ぐらいに問題が報告されて修正が入ったようです。 @wreulickeさん情報提供頂きありがとうございます！

  1. 執筆時点ではまだソースコードを解析していないため、影響範囲が不明です。↩