Removes dependency on finagle and thrift (#133)

* Removes dependency on finagle and thrift

* Do not create data that can't be queried

* Use http.url instead of http.uri

.travis.yml

@@ -2,12 +2,10 @@ language: ruby
 jdk:
   - oraclejdk8
 rvm:
+  - 2.6.0
   - 2.5.3
   - 2.4.5
   - 2.3.8
-  - 2.2.10
-  - 2.1.10
-  - 2.0
   - jruby-9.1.6.0
 deploy:
   provider: rubygems
@@ -17,7 +15,7 @@ deploy:
   on:
     tags: true
     repo: openzipkin/zipkin-ruby
-    condition: "$TRAVIS_RUBY_VERSION == 2.5.3"
+    condition: "$TRAVIS_RUBY_VERSION == 2.6.0"
 notifications:
   webhooks:
     urls:

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 0.31.0
+* Remove dependency from finagle-thrift
+* Use http.url instead of http.uri
+
 # 0.30.0
 * Add 'http.method' to client annotations
 

Gemfile

@@ -14,10 +14,3 @@ platform :jruby do
   gem 'hermann', '~> 0.27.0'
 end
 
-platform :ruby_20 do
-  gem 'rack', '~> 1.0'
-end
-
-platform :ruby_21 do
-  gem 'rack', '~> 1.0'
-end

README.md

@@ -124,6 +124,8 @@ responds to #value!, it will be called (to block until completed).
 
 Caveat: Hermann is only usable from within Jruby, due to its implementation of zookeeper based broker discovery being JVM based.
 
+The Kafka transport send data using Thrift. Since version 0.31.0, Thrift is not a dependency, thus the gem 'finagle-thrift' needs to be added to the Gemfile also.
+
 ### Logger
 
 The simplest tracer that does something. It will log all your spans.
@@ -163,8 +165,8 @@ lambda do |span, env, status, response_headers, response_body|
   # string annotation
   span.record_tag('http.referrer', env['HTTP_REFERRER'])
   # integer annotation
-  span.record_tag('http.content_size', [env['CONTENT_SIZE']].pack('N'), Trace::BinaryAnnotation::Type::I32, ep)
-  span.record_tag('http.status', [status.to_i].pack('n'), Trace::BinaryAnnotation::Type::I16, ep)
+  span.record_tag('http.content_size', env['CONTENT_SIZE'].to_s)
+  span.record_tag('http.status', status)
 end
 ```
 

lib/zipkin-tracer.rb

@@ -1,5 +1,3 @@
-require 'finagle-thrift'
-require 'base64' #Bug in finagle. They should be requiring this: finagle-thrift-1.4.1/lib/finagle-thrift/tracer.rb:115
 require 'zipkin-tracer/trace'
 require 'zipkin-tracer/rack/zipkin-tracer'
 require 'zipkin-tracer/sidekiq/middleware'

lib/zipkin-tracer/excon/zipkin-tracer.rb

@@ -1,5 +1,3 @@
-require 'finagle-thrift/trace'
-require 'finagle-thrift/tracer'
 require 'uri'
 require 'excon'
 

lib/zipkin-tracer/faraday/zipkin-tracer.rb

@@ -1,6 +1,4 @@
 require 'faraday'
-require 'finagle-thrift/trace'
-require 'finagle-thrift/tracer'
 require 'uri'
 
 module ZipkinTracer

lib/zipkin-tracer/rack/zipkin-tracer.rb

@@ -1,6 +1,4 @@
 require 'rack'
-require 'finagle-thrift/trace'
-require 'finagle-thrift/tracer'
 require 'zipkin-tracer/config'
 require 'zipkin-tracer/tracer_factory'
 require 'zipkin-tracer/rack/zipkin_env'

lib/zipkin-tracer/rack/zipkin_env.rb

@@ -34,7 +34,7 @@ def retrieve_or_generate_ids
         trace_id, span_id = @env.values_at(*B3_REQUIRED_HEADERS)
         parent_span_id = @env['HTTP_X_B3_PARENTSPANID']
       else
-        span_id = Trace.generate_id
+        span_id = TraceGenerator.new.generate_id
         trace_id = TraceGenerator.new.generate_id_from_span_id(span_id)
         parent_span_id = nil
       end

lib/zipkin-tracer/trace.rb

@@ -1,21 +1,134 @@
-require 'finagle-thrift/trace'
 require 'zipkin-tracer/zipkin_tracer_base'
-# Module with a mix of functions and overwrites from the finagle implementation:
+# Most of this code is copied from Finagle
 # https://github.com/twitter/finagle/blob/finagle-6.39.0/finagle-thrift/src/main/ruby/lib/finagle-thrift/trace.rb
+# But moved and improved here.
 module Trace
+  # These methods and attr_accessor below are used as global configuration of this gem
+  # Most of these are set by the config class and then used around.
+  # TODO: Move this out of the Trace module , take out that extend self and be happier
+  extend self
   attr_accessor :trace_id_128bit
 
-  # We need this to access the tracer from the Faraday middleware.
   def self.tracer
     @tracer
   end
 
-  def sample_rate
+  def self.sample_rate
     @sample_rate
   end
 
+  def self.tracer=(tracer)
+    @tracer = tracer
+  end
+
+  def self.sample_rate=(sample_rate)
+    if sample_rate > 1 || sample_rate < 0
+      raise ArgumentError.new("sample rate must be [0,1]")
+    end
+    @sample_rate = sample_rate
+  end
+
+  def default_endpoint=(endpoint)
+    @default_endpoint = endpoint
+  end
+
+  def default_endpoint
+    @default_endpoint
+  end
+
+  # These classes all come from Finagle-thrift + some needed modifications (.to_h)
+  # Moved here as a first step, eventually move them out of the Trace module
+
+  class Annotation
+    CLIENT_SEND = "cs"
+    CLIENT_RECV = "cr"
+    SERVER_SEND = "ss"
+    SERVER_RECV = "sr"
+
+    attr_reader :value, :host, :timestamp
+    def initialize(value, host)
+      @timestamp = (Time.now.to_f * 1000 * 1000).to_i # micros
+      @value = value
+      @host = host
+    end
+
+    def to_h
+      {
+        value: @value,
+        timestamp: @timestamp,
+        endpoint: host.to_h
+      }
+    end
+  end
+
+  class BinaryAnnotation
+    SERVER_ADDRESS = 'sa'.freeze
+    URI = 'http.url'.freeze
+    METHOD = 'http.method'.freeze
+    PATH = 'http.path'.freeze
+    STATUS = 'http.status'.freeze
+    LOCAL_COMPONENT = 'lc'.freeze
+    ERROR = 'error'.freeze
+
+    module Type
+      BOOL = "BOOL"
+      STRING = "STRING"
+    end
+    attr_reader :key, :value, :host
+
+    def initialize(key, value, annotation_type, host)
+      @key = key
+      @value = value
+      @annotation_type = annotation_type
+      @host = host
+    end
+
+    def to_h
+      {
+        key: @key,
+        value: @value,
+        endpoint: host.to_h
+      }
+    end
+  end
+
+  class Flags
+    # no flags set
+    EMPTY = 0
+    # the debug flag is used to ensure we pass all the sampling layers and that the trace is stored
+    DEBUG = 1
+  end
+
+  class SpanId
+    HEX_REGEX = /^[a-f0-9]{16,32}$/i
+    MAX_SIGNED_I64 = 9223372036854775807
+    MASK = (2 ** 64) - 1
+
+    def self.from_value(v)
+      if v.is_a?(String) && v =~ HEX_REGEX
+        # drops any bits higher than 64 by selecting right-most 16 characters
+        new(v.length > 16 ? v[v.length - 16, 16].hex : v.hex)
+      elsif v.is_a?(Numeric)
+        new(v)
+      elsif v.is_a?(SpanId)
+        v
+      end
+    end
+
+    def initialize(value)
+      @value = value
+      @i64 = if @value > MAX_SIGNED_I64
+        -1 * ((@value ^ MASK) + 1)
+      else
+        @value
+      end
+    end
+
+    def to_s; "%016x" % @value; end
+    def to_i; @i64; end
+  end
+
   # A TraceId contains all the information of a given trace id
-  # This class is defined in finagle-thrift. We are overwriting it here
   class TraceId
     attr_reader :trace_id, :parent_id, :span_id, :sampled, :flags
 
@@ -27,8 +140,8 @@ def initialize(trace_id, parent_id, span_id, sampled, flags)
       @flags = flags
     end
 
-    def next_id
-      TraceId.new(@trace_id, @span_id, Trace.generate_id, @sampled, @flags)
+    def next_id(next_span_id)
+      TraceId.new(@trace_id, @span_id, next_span_id, @sampled, @flags)
     end
 
     # the debug flag is used to ensure the trace passes ALL samplers
@@ -45,8 +158,6 @@ def to_s
     end
   end
 
-  # This class is the 128-bit version of the SpanId class:
-  # https://github.com/twitter/finagle/blob/finagle-6.39.0/finagle-thrift/src/main/ruby/lib/finagle-thrift/trace.rb#L102
   class TraceId128Bit < SpanId
     HEX_REGEX_16 = /^[a-f0-9]{16}$/i
     HEX_REGEX_32 = /^[a-f0-9]{32}$/i
@@ -79,8 +190,8 @@ def to_i; @i128; end
   end
 
   # A span may contain many annotations
-  # This class is defined in finagle-thrift. We are adding extra methods here
   class Span
+    attr_accessor :name, :annotations, :binary_annotations, :debug
     def initialize(name, span_id)
       @name = name
       @span_id = span_id
@@ -136,51 +247,33 @@ def to_microseconds(time)
     end
   end
 
-  # This class is defined in finagle-thrift. We are adding extra methods here
-  class Annotation
-    def to_h
-      {
-        value: @value,
-        timestamp: @timestamp,
-        endpoint: host.to_h
-      }
-    end
-  end
-
-  # This class is defined in finagle-thrift. We are adding extra methods here
-  class BinaryAnnotation
-    SERVER_ADDRESS = 'sa'.freeze
-    URI = 'http.uri'.freeze
-    METHOD = 'http.method'.freeze
-    PATH = 'http.path'.freeze
-    STATUS = 'http.status'.freeze
-    LOCAL_COMPONENT = 'lc'.freeze
-    ERROR = 'error'.freeze
+  class Endpoint < Struct.new(:ipv4, :port, :service_name, :ip_format)
+    MAX_I32 = ((2 ** 31) - 1)
+    MASK = (2 ** 32) - 1
+    UNKNOWN_URL = 'unknown'.freeze
 
-    def to_h
-      {
-        key: @key,
-        value: @value,
-        endpoint: host.to_h
-      }
-    end
-  end
+    def self.host_to_i32(host)
+      unsigned_i32 = Socket.getaddrinfo(host, nil)[0][3].split(".").map do |i|
+        i.to_i
+      end.inject(0) { |a,e| (a << 8) + e }
 
-  # This class is defined in finagle-thrift. We are adding extra methods here
-  class Endpoint
-    UNKNOWN_URL = 'unknown'.freeze
+      signed_i32 = if unsigned_i32 > MAX_I32
+        -1 * ((unsigned_i32 ^ MASK) + 1)
+      else
+        unsigned_i32
+      end
 
-    # we cannot override the initializer to add an extra parameter so use a factory
-    attr_accessor :ip_format
+      signed_i32
+    end
 
     def self.local_endpoint(service_port, service_name, ip_format)
       hostname = Socket.gethostname
-      Endpoint.make_endpoint(hostname, service_port, service_name, ip_format)
+      Endpoint.new(hostname, service_port, service_name, ip_format)
     end
 
     def self.remote_endpoint(url, remote_service_name, ip_format)
       service_name = remote_service_name || url.host.split('.').first || UNKNOWN_URL # default to url-derived service name
-      Endpoint.make_endpoint(url.host, url.port, service_name, ip_format)
+      Endpoint.new(url.host, url.port, service_name, ip_format)
     end
 
     def to_h
@@ -191,12 +284,5 @@ def to_h
       }
     end
 
-    private
-    def self.make_endpoint(hostname, service_port, service_name, ip_format)
-      ep = Endpoint.new(hostname, service_port, service_name)
-      ep.ip_format = ip_format
-      ep
-    end
-
   end
 end

lib/zipkin-tracer/trace_generator.rb

@@ -4,7 +4,7 @@ class TraceGenerator
     # Next id, based on the current information in the container
     def next_trace_id
       if TraceContainer.tracing_information_set?
-        TraceContainer.current.next_id
+        TraceContainer.current.next_id(generate_id)
       else
         generate_trace_id
       end
@@ -23,12 +23,12 @@ def generate_id_from_span_id(span_id)
       Trace.trace_id_128bit ? generate_id_128bit(span_id) : span_id
     end
 
-    private
-
     def generate_id
       rand(TRACE_ID_UPPER_BOUND)
     end
 
+    private
+
     def generate_id_128bit(span_id)
       trace_id_low_64bit = '%016x' % span_id
       "#{trace_id_epoch_seconds}#{trace_id_high_32bit}#{trace_id_low_64bit}".hex

lib/zipkin-tracer/zipkin_kafka_tracer.rb

@@ -2,6 +2,7 @@
 begin
   require 'hermann/producer'
   require 'hermann/discovery/zookeeper'
+  require 'finagle-thrift'
 rescue LoadError => e
 end
 
@@ -26,7 +27,7 @@ def initialize(options = {})
       end
       super(options)
     end
-    
+
     def flush!
       resolved_spans = ::ZipkinTracer::HostnameResolver.new.spans_with_ips(spans)
       resolved_spans.each do |span|