'zipkin-tracer' gem not thread-safe!?

[phrinx]

I ran into a problem where error was raised inside zipkin rack middleware - here the stack:

undefined method `sampled?' for nil:NilClass

Stacktrace (most recent call first):

  zipkin-tracer/zipkin_kafka_tracer.rb:38:in `set_rpc_name'
    return unless id.sampled?
  finagle-thrift/trace.rb:46:in `set_rpc_name'
    tracer.set_rpc_name(id, name) unless stack.empty?
  zipkin-tracer.rb:82:in `tracing_filter'
    ::Trace.set_rpc_name(env["REQUEST_METHOD"]) # get/post and all that jazz
  org/jruby/ext/thread/Mutex.java:149:in `synchronize'
  zipkin-tracer.rb:80:in `tracing_filter'
    @lock.synchronize do
  zipkin-tracer.rb:63:in `call'
    tracing_filter(id, env, whitelisted) { @app.call(env) }
  raven/integrations/rack.rb:61:in `call'
    response = @app.call(env)
  rack/graphite.rb:23:in `call'
    status, headers, body = @app.call(env)
  statsd.rb:46:in `timing'
    value = yield
  rack/graphite.rb:22:in `call'
    Statsd.instance.timing(metric) do

After talking to @Oscil8 it seems the 'finagle-thrift' middleware is not thread-safe (e.g. when used in jruby environment) since its using a global common stack (https://github.com/twitter/finagle/blob/master/finagle-thrift/src/main/ruby/lib/finagle-thrift/trace.rb#L6)

[Oscil8]

Yeah the upstream middleware is using a global stack, which is clearly not thread-safe. The bug needs to be fixed in the finagle-thrift gem

[jamescway]

do you guys think this will work? I'm also not sure if twitter will be willing to install the thread_safe gem.

module Trace extend self
  @mutex = Mutex.new
  def init
    @mutex.synchronize do
      @stack ||= ThreadSafe::Array.new
    end
  end
end

[Oscil8]

Nope, this is still storing the stack in a global.

You need to update the stack access method to pull from a thread-specific place -- NOT @stack.

module Trace extend self
  ..
  def stack
    Thread.current[:trace_stack] ||= []
  end
end

[jamescway]

Thanks for the help guys, here's the PR to address this. https://github.com/twitter/finagle/pull/363