Various cleanups, add SSL/timeout configuration to HTTPJSON, properly log collector errors

.rubocop.yml

@@ -0,0 +1,48 @@
+AllCops:
+  Exclude:
+    - spec/**/*
+    - .bundle/**/*
+    - bin/**/*
+    - vendor/**/*
+    - tmp/**/*
+    - log/**/*
+    - Rakefile
+    - lightstep.gemspec
+
+Lint/AmbiguousOperator:
+  Enabled: false
+
+Metrics/ParameterLists:
+  Enabled: false
+
+Metrics/AbcSize:
+  Max: 147
+
+Metrics/BlockNesting:
+  Max: 4
+
+# Configuration parameters: CountComments.
+Metrics/ClassLength:
+  Max: 406
+
+Metrics/CyclomaticComplexity:
+  Max: 24
+
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes.
+# URISchemes: http, https
+Metrics/LineLength:
+  Enabled: false
+
+# Configuration parameters: CountComments.
+Metrics/MethodLength:
+  Max: 88
+
+# Configuration parameters: CountComments.
+Metrics/ModuleLength:
+  Max: 1000
+
+Performance/RedundantBlockCall:
+  Enabled: false
+
+Style/PercentLiteralDelimiters:
+  Enabled: false

Gemfile.lock

@@ -11,7 +11,7 @@ GEM
     bump (0.6.1)
     concurrent-ruby (1.0.5)
     diff-lcs (1.3)
-    docile (1.1.5)
+    docile (1.3.1)
     json (2.1.0)
     opentracing (0.4.1)
     rack (2.0.5)
@@ -29,8 +29,8 @@ GEM
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.7.0)
     rspec-support (3.7.1)
-    simplecov (0.12.0)
-      docile (~> 1.1.0)
+    simplecov (0.16.1)
+      docile (~> 1.1)
       json (>= 1.8, < 3)
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.2)
@@ -45,9 +45,8 @@ DEPENDENCIES
   rack (~> 2.0)
   rake (~> 11.3)
   rspec (~> 3.0)
-  simplecov (~> 0.12.0)
+  simplecov (~> 0.16)
   timecop (~> 0.8.0)
 
 BUNDLED WITH
-   1.16.2
-
+   1.16.4

example.rb

@@ -11,7 +11,7 @@ LightStep.configure(component_name: 'lightstep/ruby/example', access_token: acce
 puts 'Starting operation...'
 span = LightStep.start_span('my_span')
 thread1 = Thread.new do
-  for i in 1..10
+  (1..10).each do |i|
     sleep(0.15)
     puts "Logging event #{i}..."
     span.log(event: 'hello world', count: i)
@@ -19,7 +19,7 @@ thread1 = Thread.new do
 end
 thread2 = Thread.new do
   current = 1
-  for i in 1..16
+  (1..16).each do |i|
     child = LightStep.start_span('my_child', child_of: span.span_context)
     sleep(0.1)
     current *= 2

lib/lightstep/reporter.rb

@@ -57,7 +57,10 @@ module LightStep
 
       span_records = @span_records.slice!(0, @span_records.length)
       dropped_spans = 0
-      @dropped_spans.update{|old| dropped_spans = old; 0 }
+      @dropped_spans.update do |old|
+        dropped_spans = old
+        0
+      end
 
       report_request = {
         runtime: @runtime,
@@ -65,9 +68,10 @@ module LightStep
         youngest_micros: now,
         span_records: span_records,
         internal_metrics: {
-	  counts: [
-	    {name: "spans.dropped", int64_value: dropped_spans},
-	  ]
+          counts: [{
+            name: 'spans.dropped',
+            int64_value: dropped_spans
+          }]
         }
       }
 
@@ -75,9 +79,10 @@ module LightStep
 
       begin
         @transport.report(report_request)
-      rescue
-	# an error occurs, add the previous dropped_spans and count of spans
-	# that would have been recorded
+      rescue StandardError => e
+        LightStep.logger.error "LightStep error reporting to collector: #{e.message}"
+        # an error occurs, add the previous dropped_spans and count of spans
+        # that would have been recorded
         @dropped_spans.increment(dropped_spans + span_records.length)
       end
     end
@@ -103,8 +108,8 @@ module LightStep
             sleep(@period)
             flush
           end
-        rescue => ex
-          # TODO: internally log the exception
+        rescue StandardError => e
+          LightStep.logger.error "LightStep failed to report spans: #{e.message}"
         end
       end
     end

lib/lightstep/tracer.rb

@@ -66,7 +66,6 @@ module LightStep
     # @param tags [Hash] Tags to assign to the Span at start time
     # @return [Span]
     def start_span(operation_name, child_of: nil, references: [], start_time: nil, tags: nil)
-
       Span.new(
         tracer: self,
         operation_name: operation_name,
@@ -150,7 +149,7 @@ module LightStep
     protected
 
     def configure(component_name:, access_token: nil, transport: nil, tags: {})
-      raise ConfigurationError, "component_name must be a string" unless String === component_name
+      raise ConfigurationError, "component_name must be a string" unless component_name.is_a?(String)
       raise ConfigurationError, "component_name cannot be blank"  if component_name.empty?
 
       if transport.nil? and !access_token.nil?
@@ -234,7 +233,7 @@ module LightStep
     def extract_from_rack(env)
       extract_from_text_map(env.reduce({}){|memo, tuple|
         raw_header, value = tuple
-        header = raw_header.gsub(/^HTTP_/, '').gsub("_", "-").downcase
+        header = raw_header.gsub(/^HTTP_/, '').tr('_', '-').downcase
 
         memo[header] = value if header.start_with?(CARRIER_TRACER_STATE_PREFIX, CARRIER_BAGGAGE_PREFIX)
         memo

lib/lightstep/transport/base.rb

@@ -2,9 +2,6 @@ module LightStep
   module Transport
     # Base Transport type
     class Base
-      def initialize
-      end
-
       def report(_report)
         nil
       end

lib/lightstep/transport/http_json.rb

@@ -11,46 +11,102 @@ module LightStep
     # within the fork (and in the parent post-fork). See
     # `examples/fork_children/main.rb` for an example.
     class HTTPJSON < Base
-      LIGHTSTEP_HOST = "collector.lightstep.com"
+      LIGHTSTEP_HOST = 'collector.lightstep.com'.freeze
       LIGHTSTEP_PORT = 443
 
-      ENCRYPTION_TLS = 'tls'
-      ENCRYPTION_NONE = 'none'
+      ENCRYPTION_TLS = 'tls'.freeze
+      ENCRYPTION_NONE = 'none'.freeze
 
+      REPORTS_API_ENDPOINT = '/api/v0/reports'.freeze
+      HEADER_ACCESS_TOKEN = 'LightStep-Access-Token'.freeze
+
+      ##
       # Initialize the transport
-      # @param host [String] host of the domain to the endpoind to push data
+      #
+      # @param host [String] host of the domain to the endpoint to push data
       # @param port [Numeric] port on which to connect
       # @param verbose [Numeric] verbosity level. Right now 0-3 are supported
       # @param encryption [ENCRYPTION_TLS, ENCRYPTION_NONE] kind of encryption to use
       # @param access_token [String] access token for LightStep server
-      # @return [HTTPJSON]
-      def initialize(host: LIGHTSTEP_HOST, port: LIGHTSTEP_PORT, verbose: 0, encryption: ENCRYPTION_TLS, access_token:)
+      # @param ssl_verify_peer [Boolean]
+      # @param open_timeout [Integer]
+      # @param read_timeout [Integer]
+      # @param continue_timeout [Integer]
+      # @param keep_alive_timeout [Integer]
+      # @param logger [Logger]
+      #
+      def initialize(
+        host: LIGHTSTEP_HOST,
+        port: LIGHTSTEP_PORT,
+        verbose: 0,
+        encryption: ENCRYPTION_TLS,
+        access_token:,
+        ssl_verify_peer: true,
+        open_timeout: 20,
+        read_timeout: 20,
+        continue_timeout: nil,
+        keep_alive_timeout: 2,
+        logger: nil
+      )
         @host = host
         @port = port
         @verbose = verbose
         @encryption = encryption
+        @ssl_verify_peer = ssl_verify_peer
+        @open_timeout = open_timeout.to_i
+        @read_timeout = read_timeout.to_i
+        @continue_timeout = continue_timeout
+        @keep_alive_timeout = keep_alive_timeout.to_i
 
-        raise Tracer::ConfigurationError, "access_token must be a string" unless String === access_token
-        raise Tracer::ConfigurationError, "access_token cannot be blank"  if access_token.empty?
+        raise Tracer::ConfigurationError, 'access_token must be a string' unless access_token.is_a?(String)
+        raise Tracer::ConfigurationError, 'access_token cannot be blank'  if access_token.empty?
         @access_token = access_token
+        @logger = logger || LightStep.logger
       end
 
+      ##
       # Queue a report for sending
+      #
       def report(report)
-        p report if @verbose >= 3
+        @logger.info report if @verbose >= 3
 
-        https = Net::HTTP.new(@host, @port)
-        https.use_ssl = @encryption == ENCRYPTION_TLS
-        req = Net::HTTP::Post.new('/api/v0/reports')
-        req['LightStep-Access-Token'] = @access_token
+        req = build_request(report)
+        res = connection.request(req)
+
+        @logger.info res.to_s if @verbose >= 3
+
+        nil
+      end
+
+      private
+
+      ##
+      # @param [Hash] report
+      # @return [Net::HTTP::Post]
+      #
+      def build_request(report)
+        req = Net::HTTP::Post.new(REPORTS_API_ENDPOINT)
+        req[HEADER_ACCESS_TOKEN] = @access_token
         req['Content-Type'] = 'application/json'
         req['Connection'] = 'keep-alive'
         req.body = report.to_json
-        res = https.request(req)
+        req
+      end
 
-        puts res.to_s if @verbose >= 3
-
-        nil
+      ##
+      # @return [Net::HTTP]
+      #
+      def connection
+        unless @connection
+          @connection = ::Net::HTTP.new(@host, @port)
+          @connection.use_ssl = @encryption == ENCRYPTION_TLS
+          @connection.verify_mode = ::OpenSSL::SSL::VERIFY_NONE unless @ssl_verify_peer
+          @connection.open_timeout = @open_timeout
+          @connection.read_timeout = @read_timeout
+          @connection.continue_timeout = @continue_timeout
+          @connection.keep_alive_timeout = @keep_alive_timeout
+        end
+        @connection
       end
     end
   end

lightstep.gemspec

@@ -22,6 +22,6 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'rack', '~> 2.0'
   spec.add_development_dependency 'rspec', '~> 3.0'
   spec.add_development_dependency 'bump', '~> 0.5'
-  spec.add_development_dependency 'simplecov', '~> 0.12.0'
+  spec.add_development_dependency 'simplecov', '~> 0.16'
   spec.add_development_dependency 'timecop', '~> 0.8.0'
 end

spec/lightstep_spec.rb

@@ -6,7 +6,7 @@ describe LightStep do
   end
 
   def init_callback_tracer(callback)
-    tracer = LightStep::Tracer.new(
+    LightStep::Tracer.new(
       component_name: 'lightstep/ruby/spec',
       transport: LightStep::Transport::Callback.new(callback: callback)
     )
@@ -215,7 +215,7 @@ describe LightStep do
     file = File.open('./lib/lightstep.rb', 'r')
     data = [
       nil,
-      TRUE, FALSE,
+      true, false,
       0, -1, 1,
       0.0, -1.0, 1.0,
       '', 'a', 'a longer string',