Merge pull request #4 frompalazzem/rubyish-code-style

* update internals so that the code is a little bit more idiomatic
* fixed an issue related to span parenting

.rubocop.yml

@@ -20,3 +20,6 @@ Metrics/MethodLength:
 # honors this rule.
 Style/AccessorMethodName:
   Enabled: false
+
+Style/MethodCallParentheses:
+  Enabled: false

README.md

@@ -1,2 +1,17 @@
+# dd-trace-rb
 
-[![CircleCI](https://circleci.com/gh/DataDog/dd-trace-rb.svg?style=svg&circle-token=f9bf80ce9281bc638c6f7465512d65c96ddc075a)](https://circleci.com/gh/DataDog/dd-trace-rb)
+[![CircleCI](https://circleci.com/gh/DataDog/dd-trace-rb/tree/master.svg?style=svg&circle-token=b0bd5ef866ec7f7b018f48731bb495f2d1372cc1)](https://circleci.com/gh/DataDog/dd-trace-rb/tree/master)
+
+## Testing
+
+You can launch all tests using the following rake command:
+```
+  $ rake test
+```
+
+We also enforce the Ruby [community-driven style guide][1] through Rubocop. Simply launch:
+```
+  $ rake rubocop
+```
+
+[1]: https://github.com/bbatsov/ruby-style-guide

lib/ddtrace/local.rb

@@ -1,6 +1,7 @@
 require 'thread'
 
 module Datadog
+  # Buffer used to store active spans
   class SpanBuffer
     # Set the current active span.
     def set(span)
@@ -14,9 +15,9 @@ def get
 
     # Pop the current active span.
     def pop
-      s = get
+      span = get()
       set(nil)
-      s
+      span
     end
   end
 end

lib/ddtrace/span.rb

@@ -1,8 +1,13 @@
 require 'json'
 require 'time'
 
+# Datadog namespace
 module Datadog
+  # Span model that defines a logical unit of work that
+  # composes a complete trace.
   class Span
+    MAX_ID = 2**64 - 1
+
     attr_accessor :name, :service, :resource,
                   :start_time, :end_time,
                   :span_id, :trace_id, :parent_id,
@@ -13,12 +18,12 @@ def initialize(tracer, name, options = {})
       @tracer = tracer
 
       @name = name
-      @service = options[:service]
-      @resource = options[:resource] || name
+      @service = options.fetch(:service, nil)
+      @resource = options.fetch(:resource, name)
 
-      @span_id = Datadog.next_id
-      @parent_id = options[:parent_id] || 0
-      @trace_id = options[:trace_id] || @span_id
+      @span_id = Datadog.next_id()
+      @parent_id = options.fetch(:parent_id, 0)
+      @trace_id = options.fetch(:trace_id, @span_id)
 
       @meta = {}
       @status = 0
@@ -35,7 +40,7 @@ def trace
       set_error(e)
       raise
     ensure
-      finish
+      finish()
     end
 
     def set_tag(key, value)
@@ -49,12 +54,11 @@ def get_tag(key)
 
     # Mark the span with the given error.
     def set_error(e)
-      unless e.nil?
-        @status = 1
-        @meta['error.msg'] = e.message
-        @meta['error.type'] = e.class.to_s
-        @meta['error.stack'] = e.backtrace.join("\n")
-      end
+      return if e.nil?
+      @status = 1
+      @meta['error.msg'] = e.message
+      @meta['error.type'] = e.class.to_s
+      @meta['error.stack'] = e.backtrace.join("\n")
     end
 
     # Mark the span finished at the current time and submit it.
@@ -77,9 +81,14 @@ def to_s
     end
 
     # Set this span's parent, inheriting any properties not explicitly set.
+    # If the parent is nil, set the span zero values.
     def set_parent(parent)
       @parent = parent
-      unless parent.nil?
+
+      if parent.nil?
+        @trace_id = @span_id
+        @parent_id = 0
+      else
         @trace_id = parent.trace_id
         @parent_id = parent.span_id
         @service ||= parent.service
@@ -108,11 +117,9 @@ def to_hash
     end
   end
 
-  @@max_id = 2**64 - 1
-
-  # Return a span id.
+  # Return a span id
   def self.next_id
-    rand(@@max_id)
+    rand(Datadog::Span::MAX_ID)
   end
 
   # Encode the given set of spans.

lib/ddtrace/tracer.rb

@@ -3,44 +3,46 @@
 require 'ddtrace/writer'
 
 module Datadog
+  # Tracer class that records and creates spans related to a
+  # compositions of logical units of work.
   class Tracer
     attr_reader :writer
 
     def initialize(options = {})
       # buffers and sends completed traces.
-      @writer = options[:writer] || Datadog::Writer.new
+      @writer = options.fetch(:writer, Datadog::Writer.new())
 
       # store thes the active thread in the current span.
-      @buffer = Datadog::SpanBuffer.new
+      @buffer = Datadog::SpanBuffer.new()
       @spans = []
     end
 
     def trace(name, options = {})
       span = Span.new(self, name, options)
 
       # set up inheritance
-      parent = @buffer.get
+      parent = @buffer.get()
       span.set_parent(parent)
       @buffer.set(span)
 
       # now delete the called block to it
-      if block_given?
-        return span.trace(&Proc.new)
-      else
-        return span.trace
-      end
+      # TODO[manu]: this part must be refactored and merged with span.trace()
+      # span.trace call is mandatory and we should return the span when it's
+      # finished.
+      return span.trace(&Proc.new) if block_given?
+      span.trace()
+      span
     end
 
     def record(span)
       @spans << span
-
       parent = span.parent
       @buffer.set(parent)
-      if parent.nil?
-        spans = @spans
-        @spans = []
-        write(spans)
-      end
+
+      return unless parent.nil?
+      spans = @spans
+      @spans = []
+      write(spans)
     end
 
     def write(spans)

lib/ddtrace/transport.rb

@@ -1,6 +1,8 @@
 require 'net/http'
 
 module Datadog
+  # Transport class that handles the spans delivery to the
+  # local trace-agent
   class Transport
     def initialize(host, port)
       @http = Net::HTTP.new(host, port)

lib/ddtrace/writer.rb

@@ -4,7 +4,7 @@ module Datadog
   # Writer buffer and periodically sends traces to the server.
   class Writer
     def initialize(options = {})
-      @transport = options[:transport] || Datadog::Transport.new('localhost', '7777')
+      @transport = options.fetch(:transport, Datadog::Transport.new('localhost', '7777'))
       @trace_buffer = TraceBuffer.new(100)
 
       @flush_interval = 1
@@ -15,24 +15,23 @@ def initialize(options = {})
 
     # spawn will spawn a thread that will periodically flush to the server.
     def spawn(interval)
-      Thread.new do
+      Thread.new() do
         loop do
           sleep(interval)
-          flush
+          flush()
         end
       end
     end
 
     # flush will trigger a flush to the server.
     def flush
-      traces = @trace_buffer.pop
-      unless traces.empty?
-        spans = traces.flatten
-        # FIXME[matt] submit as an array of traces or a flat array of spans?
-        #
-        @transport.write(spans)          # FIXME matt: if there's an error, requeue
-        @traces_flushed += traces.length # FIXME matt: synchornize?
-      end
+      traces = @trace_buffer.pop()
+      return if traces.empty?
+
+      spans = traces.flatten
+      # FIXME[matt] submit as an array of traces or a flat array of spans?
+      @transport.write(spans)            # FIXME matt: if there's an error, requeue
+      @traces_flushed += traces.length() # FIXME matt: synchornize?
     end
 
     # write will queue the trace for submission to the api.
@@ -44,7 +43,7 @@ def write(trace)
     def stats
       {
         traces_flushed: @traces_flushed,
-        traces_buffered: @trace_buffer.length
+        traces_buffered: @trace_buffer.length()
       }
     end
   end
@@ -53,14 +52,14 @@ def stats
   # app.
   class TraceBuffer
     def initialize(max_size)
-      @mutex = Mutex.new
+      @mutex = Mutex.new()
       @traces = []
       @max_size = max_size
     end
 
     def push(trace)
       @mutex.synchronize do
-        len = @traces.length
+        len = @traces.length()
         if len < @max_size
           @traces << trace
         else
@@ -71,7 +70,7 @@ def push(trace)
 
     def length
       @mutex.synchronize do
-        return @traces.length
+        return @traces.length()
       end
     end
 

test/span_test.rb

@@ -29,6 +29,41 @@ def test_span_with_parent
     assert_equal(span.name, 'my.op')
   end
 
+  def test_span_set_parent
+    parent = Datadog::Span.new(nil, 'parent.span')
+    child = Datadog::Span.new(nil, 'child.span')
+
+    child.set_parent(parent)
+    assert_equal(child.parent, parent)
+    assert_equal(child.trace_id, parent.trace_id)
+    assert_equal(child.parent_id, parent.span_id)
+    assert_equal(child.service, parent.service)
+  end
+
+  def test_span_set_parent_keep_service
+    parent = Datadog::Span.new(nil, 'parent.span', service: 'webapp')
+    child = Datadog::Span.new(nil, 'child.span', service: 'defaultdb')
+
+    child.set_parent(parent)
+    assert_equal(child.parent, parent)
+    assert_equal(child.trace_id, parent.trace_id)
+    assert_equal(child.parent_id, parent.span_id)
+    refute_equal(child.service, 'webapp')
+    assert_equal(child.service, 'defaultdb')
+  end
+
+  def test_span_set_parent_nil
+    parent = Datadog::Span.new(nil, 'parent.span', service: 'webapp')
+    child = Datadog::Span.new(nil, 'child.span', service: 'defaultdb')
+
+    child.set_parent(parent)
+    child.set_parent(nil)
+    assert_equal(child.parent, nil)
+    assert_equal(child.trace_id, child.span_id)
+    assert_equal(child.parent_id, 0)
+    assert_equal(child.service, 'defaultdb')
+  end
+
   def test_span_block
     start = Time.now.utc
     span = nil