TwP
Add context inheritance via `prepend` instead of changing `Thread`
👋 Hello there!
I work at @Datadog on our open-source Ruby APM library, ddtrace.
One of our users has ran into an issue where logging's Thread inherit context behavior was conflicting with dd-trace's own extensions to Thread, leading to a stack level too deep (SystemStackError) due to both of our extensions conflicting.
We've seen this happen from time to time, e.g. https://github.com/rollbar/rollbar-gem/pull/1018 or https://github.com/MiniProfiler/rack-mini-profiler/pull/444. (See also https://github.com/rails/rails/pull/19434 and https://github.com/rails/rails/pull/19413 where rails moved to prepend as well)
Changing logging to use prepend makes it compatible with all libraries that have moved to use prepend.
Furthermore, while I was in the neighbourhood, I've added the tweaks necessary to not break Ruby 3.0 keyword arguments when used with threads, as well as a test to cover it.
I've tested these changes with Ruby 2.4, 2.7 and 3.0.
Hopefully this change is acceptable? Let me know if I can provide any more info or help, as we're really interested in solving this 😀
Note: As documented in the logging source code, setting the LOGGING_INHERIT_CONTEXT environment variable to false also avoids this issue, although at the cost of loosing that helpful feature.
~~Note: I'm experimenting with a hack in ddtrace that hopefully avoids triggering this issue, but older versions of ddtrace and other libraries using these monkey patches would still be affected, and thus I think it's still worth the fix.~~
Update: Experiment didn't work, either this patch or LOGGING_INHERIT_CONTEXT=false is still required.