Opentracing bridge

[rdooley]

I believe this is ready for review again (and probably could do with me squashing work)

[fbogsany]

I'd remove the "shim" suffix everywhere.

[fbogsany]

Also, the suggested terminology here is "bridges"

[rdooley]

@fbogsany so something like opentracingbridge/lib/opentelemetry/opentracingbridge/span.rb ?

[fbogsany]

Either of the following:

opentelemetry-ruby/opencensus/lib/opentelemetry/opencensus/span.rb
opentelemetry-ruby/opencensus/opentelemetry-opencensus.gemspec
opentelemetry-ruby/opentracing/lib/opentelemetry/opentracing/span.rb
opentelemetry-ruby/opentracing/lib/opentelemetry-opentracing.gemspec

or

opentelemetry-ruby/bridges/opentracing/lib/opentelemetry/bridge/opentracing/span.rb
opentelemetry-ruby/bridges/opentracing/opentelemetry-bridge-opentracing.gemspec
...

I kinda like the second. I'm a little 🤷‍♂ about pluralizing the top-level directory, but leaning toward the plural. So:

api
sdk
bridges
exporters
adapters

at the top-level.

[rdooley]

The plural makes sense to me, on it

[rdooley]

Very weird jruby thing where if you bundle OTrace bridge before CI jaeger (or vise versa) whichever you bundled before the other one, fails its CI step with e.g.

File does not exist: thrift

[mwear]

👋 @rdooley I definitely owe you a review, which I'll try to get in tomorrow. Thanks for your patience 🙏!

[mwear]

I went through this and spotted a few things that are definitely issues. I took things a step further and tried this out on a small test app and it does not work. Mainly due to the fact that child_of can be an OpenTelemetry::Context. There are definitely more issues beyond those I mentioned. I'm going to paste in the sample app I tested with as I think it could be useful to help validate your work as you go.

The test app is a simple rack app. You can drop it in a config.ru file and run it with rackup. You can spin up two instances say, on ports 3000 and 3001 and it will make a request to the next instance. It has instrumentation for Rack and Faraday, so you should be able to get a distributed tracing linking up between both of them.

Gemfile

adjust paths for your local checkout of otel:

source 'https://rubygems.org'

gem 'rack'
gem 'faraday'
gem 'rack-tracer'
gem 'faraday-tracer'
gem 'opentelemetry-bridge-opentracing', path: '../../opentelemetry-ruby/bridges/opentracing'
gem 'opentelemetry-api', path: '../../opentelemetry-ruby/api'
gem 'opentelemetry-sdk', path: '../../opentelemetry-ruby/sdk'

app

drop this in a config.ru

require 'bundler'
Bundler.require

require 'opentelemetry/bridge/opentracing'

OpenTelemetry::SDK.configure
OpenTracing.global_tracer = OpenTelemetry::Bridge::OpenTracing::Tracer.new(OpenTelemetry.tracer_factory.tracer('default'))

class App
  class << self
    def call(env)
      call_next(env)
      [200, {}, ["Hello world!"]]
    end

    def call_next(env)
      puts "request: #{next_url(env)}"
      resp = http_conn.get(next_url(env))
      resp.body
    rescue => e
      puts e.inspect
    end

    def next_url(env)
      host = env['SERVER_NAME']
      port = env['SERVER_PORT'].to_i + 1
      "http://#{host}:#{port}"
    end

    def http_conn
      @http_conn ||= Faraday.new do |f|
        f.use Faraday::Tracer
        f.request :url_encoded
        f.adapter Faraday.default_adapter
      end
    end
  end
end

use Rack::Tracer

run App

• Note: your branch is out a date a bit with master. The tracer_factory is now a tracer_provider. Keep that in mind if you rebase.

Let me know if this helps get you moving in the right direction and reach out if you have any questions.

[mwear]

You need to be able to handle the case where child_of is an OpenTelemetry::Context. If child of is a Context you'll want to pass that as with_parent_context.

[mwear]

The same applies here.

[mwear]

If we want to interop with the OpenTelemetry Tracer, both inject and extract should use the global propagators, i.e. OpenTelemetry.propagation.inject and OpenTelemetry.propagation.extract.

[mwear]

Scope activation / deactivation (closing) needs to correspond with an OpenTelemetry::Context containing the active span being attached and detached. Attach makes that the current context, whereas detach restores the previous context. Manual Context#attach and Context#detach are somewhat dangerous, as each call to attach and detach need to match, however, I think the scope concept from OT should make this work out.

[fbogsany]

This is out of date and the spec has tightened up around OT compatibility for Otel. Closing this in favour of a new implementation.