asenchi / scrolls

Simple logging
MIT License
158 stars 26 forks source link

Achieve true thread local context #61

Closed rwdaigle closed 7 years ago

rwdaigle commented 10 years ago

In an environment using a Fiber pool (such as an evented Rack app), setting a Scrolls context in a Rack middleware will not then be available to all scopes within that request. This is because in Ruby 2.x Thread.current[]= behavior does not actually set a thread-local variable, but a Fiber-local one.

To achieve true thread locality you need to use thread_variable_get (and set). I believe this is how context should be implemented?

module Scrolls
  module Log

    def context=(h)
      Thread.current.thread_variable_set(:scrolls_context, h || {})
    end

    def context
      Thread.current.thread_variable_get(:scrolls_context)
    end
  end
end

If so, let me know and I can submit a proper PR. One up front question would be your preference in Ruby version detection.

asenchi commented 10 years ago

@rwdaigle I'll need to read up more, this is definitely interesting and unintended behavior. I'm curious what the effects are on Fiber based environments then? I've not kept up on what is using Fibers, or threads, is there an option here that satisfies each setup? I'm not well versed in the nuances in Threads vs. Fibers in Ruby.

As for version matching, I've always used ENV['RUBY_VERSION'], but am interested in what the current methods are. This has always been the easiest for me, but there may be a more common "practice".

asenchi commented 7 years ago

I've fixed this behavior in my refactor: https://github.com/asenchi/scrolls/pull/71