span todos

lib/lightstep/tracer/span.rb

@@ -1,6 +1,5 @@
 # TODO(ngauthier@gmail.com) Separate Span and SpanContext under a getter according
 # to the spec. Baggage moves to span context.
-
 module LightStep
   class Span
     # ----------------------------------------------------------------------------
@@ -16,14 +15,11 @@ module LightStep
       self
     end
 
-    # FIXME(ngauthier@gmail.com) accessor
     def set_baggage_item(key, value)
       @baggage[key] = value
       self
     end
 
-    # FIXME(ngauthier@gmail.com) accessor
-    # TODO(ngauthier@gmail.com) remove? Not in spec to get baggage.
     def get_baggage_item(key)
       @baggage[key]
     end
@@ -34,7 +30,7 @@ module LightStep
     end
 
     def log(fields)
-      record = { span_guid: @guid.to_s }
+      record = { span_guid: @guid }
 
       record[:stable_name] = fields[:event].to_s unless fields[:event].nil?
       unless fields[:timestamp].nil?
@@ -43,10 +39,9 @@ module LightStep
       @tracer.raw_log_record(record, fields[:payload])
     end
 
-    # FIXME(ngauthier@gmail.com) keyword arg?
     def finish(fields = nil)
       unless fields.nil?
-        set_end_micros(fields[:endTime] * 1000) unless fields[:endTime].nil?
+        self.end_micros = fields[:endTime] * 1000 unless fields[:endTime].nil?
       end
       @tracer._finish_span(self)
       self
@@ -57,20 +52,14 @@ module LightStep
     # ----------------------------------------------------------------------------
 
     def initialize(tracer)
-      @guid = ''
-      @operation = ''
-      @trace_guid = nil
       @tags = {}
       @baggage = {}
-      @start_micros = 0
-      @end_micros = 0
-      @error_flag = false
 
       @tracer = tracer
       @guid = tracer.generate_guid
     end
 
-    attr_reader :guid, :operation, :tags, :baggage, :start_micros, :end_micros, :error_flag
+    attr_reader :guid, :tags, :baggage
     attr_accessor :trace_guid
 
     def finalize
@@ -80,37 +69,32 @@ module LightStep
       end
     end
 
-    # FIXME(ngauthier@gmail.com) writer
-    def set_start_micros(start)
-      @start_micros = start
-      self
+    attr_writer :start_micros
+    def start_micros
+      @start_micros ||= 0
     end
 
-    # FIXME(ngauthier@gmail.com) writer
-    def set_end_micros(micros)
-      @end_micros = micros
-      self
+    attr_writer :end_micros
+    def end_micros
+      @end_micros ||= 0
     end
 
-    # FIXME(ngauthier@gmail.com) writer
-    def set_operation_name(name)
-      @operation = name
-      self
+    attr_writer :operation_name
+    def operation_name
+      @operation_name ||= ''
     end
 
     def parent_guid
       @tags[:parent_span_guid]
     end
 
-    # FIXME(ngauthier@gmail.com) writer
     def set_parent(span)
       set_tag(:parent_span_guid, span.guid)
       @trace_guid = span.trace_guid
       self
     end
 
-    # FIXME(ngauthier@gmail.com) to_h
-    def to_thrift
+    def to_h
       attributes = @tags.map do |key, value|
         {"Key" => key.to_s, "Value" => value.to_s}
       end
@@ -123,7 +107,8 @@ module LightStep
         "attributes" => attributes,
         "oldest_micros" => @start_micros.to_i,
         "youngest_micros" => @end_micros.to_i,
-        "error_flag" => @error_flag
+        # TODO(ngauthier@gmail.com) this wasn't used anywhere and was always false, can we remove?
+        "error_flag" => false
       }
     end
   end

lib/lightstep/tracer/tracer.rb

@@ -93,14 +93,14 @@ module LightStep
     # TODO(ngauthier@gmail.com) ability to provide a timestamp to be used other than now
     def start_span(operation_name, fields = nil)
       span = Span.new(self)
-      span.set_operation_name(operation_name)
-      span.set_start_micros(now_micros)
+      span.operation_name = operation_name
+      span.start_micros = now_micros
 
       unless fields.nil?
         span.set_parent(fields[:parent]) unless fields[:parent].nil?
         span.set_tags(fields[:tags]) unless fields[:tags].nil?
-        span.set_start_micros(fields[:startTime] * 1000) unless fields[:startTime].nil?
-        span.set_end_micros(fields[:endTime] * 1000) unless fields[:endTime].nil?
+        span.start_micros = fields[:startTime] * 1000 unless fields[:startTime].nil?
+        span.end_micros = fields[:endTime] * 1000 unless fields[:endTime].nil?
       end
 
       span.trace_guid = generate_guid if span.trace_guid.nil?
@@ -161,8 +161,8 @@ module LightStep
     def _finish_span(span)
       return unless enabled?
 
-      span.set_end_micros(now_micros) if span.end_micros === 0
-      full = push_with_max(@tracer_span_records, span.to_thrift, max_span_records)
+      span.end_micros = now_micros if span.end_micros === 0
+      full = push_with_max(@tracer_span_records, span.to_h, max_span_records)
       @tracer_counters[:dropped_spans] += 1 if full
       flush_if_needed
     end
@@ -312,8 +312,8 @@ module LightStep
 
     def join_from_text_map(operation_name, carrier)
       span = Span.new(self)
-      span.set_operation_name(operation_name)
-      span.set_start_micros(now_micros)
+      span.operation_name = operation_name
+      span.start_micros = now_micros
 
       parent_guid = carrier[CARRIER_TRACER_STATE_PREFIX + 'spanid']
       trace_guid = carrier[CARRIER_TRACER_STATE_PREFIX + 'traceid']

spec/lightstep_spec.rb

@@ -123,7 +123,7 @@ describe LightStep do
     parent2.finish
 
     (children1.concat children2).each do |child|
-      thrift_data = child.to_thrift
+      thrift_data = child.to_h
       expect(thrift_data['trace_guid']).to eq(child.trace_guid)
     end
   end