Use TraceFormatter instead of TraceOperation to set tags

TraceOperation is largely oblivious to the root span concept. The lack
of a concept of trace is part of the Trace API transport, therefore
TraceFormatter is more appropriate to carry and decide where to put the
trace-level values.

Trace tags explicitly do not carry to partially flushed traces in order
to avoid double processing.

lib/datadog/tracing/trace_operation.rb

@@ -63,7 +63,8 @@ def initialize(
         sample_rate: nil,
         sampled: nil,
         sampling_priority: nil,
-        service: nil
+        service: nil,
+        tags: nil
       )
         # Attributes
         @events = events || Events.new
@@ -84,6 +85,9 @@ def initialize(
         @sampling_priority = sampling_priority
         @service = service
 
+        # Generic tags
+        @tags = set_tags(tags)
+
         # State
         @root_span = nil
         @active_span = nil
@@ -105,10 +109,6 @@ def finished?
         @finished == true
       end
 
-      def started?
-        !@root_span.nil?
-      end
-
       def sampled?
         @sampled == true || (!@sampling_priority.nil? && @sampling_priority > 0)
       end
@@ -243,7 +243,7 @@ def to_digest
           trace_resource: @resource,
           trace_runtime_id: Core::Environment::Identity.id,
           trace_sampling_priority: @sampling_priority,
-          trace_service: @service
+          trace_service: @service,
         ).freeze
       end
 
@@ -265,7 +265,8 @@ def fork_clone
           sample_rate: @sample_rate,
           sampled: @sampled,
           sampling_priority: @sampling_priority,
-          service: (@service && @service.dup)
+          service: (@service && @service.dup),
+          tags: @tags.dup
         )
       end
 
@@ -322,11 +323,16 @@ def subscribe_deactivate_trace(&block)
         :events,
         :root_span
 
+      def tags
+        @tags ||= {}
+      end
+
       def set_tag(key, value = nil)
-        raise UnstartedError unless started?
-        raise FinishedError if finished?
+        tags[key] = value
+      end
 
-        @root_span.set_tag(key, value)
+      def set_tags(tags)
+        (tags || {}).each { |k, v| set_tag(k, v) }
       end
 
       def activate_span!(span_op)
@@ -415,23 +421,10 @@ def build_trace(spans, partial = false)
           name: @name,
           resource: @resource,
           service: @service,
+          tags: !partial ? @tags : nil,
           root_span_id: !partial ? @root_span && @root_span.id : nil
         )
       end
-
-      # Error when an action is attempted that needs an active trace
-      class UnstartedError < StandardError
-        def message
-          'Trace not started'.freeze
-        end
-      end
-
-      # Error when an action is attempted that needs an active trace
-      class FinishedError < StandardError
-        def message
-          'Trace finished'.freeze
-        end
-      end
     end
     # rubocop:enable Metrics/ClassLength
   end

lib/datadog/tracing/trace_segment.rb

@@ -49,12 +49,13 @@ def initialize(
         runtime_id: nil,
         sample_rate: nil,
         sampling_priority: nil,
-        service: nil
+        service: nil,
+        tags: nil
       )
         @id = id
         @root_span_id = root_span_id
         @spans = spans || []
-        @tags = {}
+        @tags = tags || {}
 
         # Set well-known tags
         self.agent_sample_rate = agent_sample_rate

lib/ddtrace/transport/trace_formatter.rb

@@ -32,6 +32,11 @@ def format!
         # trace metadata, we must put our trace
         # metadata on the root span. This "root span"
         # is needed by the agent/API to ingest the trace.
+
+        # Apply generic trace tags. Any more specific value will be overridden
+        # by the subsequent calls below.
+        set_trace_tags!
+
         set_resource!
 
         tag_agent_sample_rate!
@@ -59,6 +64,10 @@ def set_resource!
         root_span.resource = trace.resource
       end
 
+      def set_trace_tags!
+        root_span.set_tags(trace.tags)
+      end
+
       def tag_agent_sample_rate!
         return unless trace.agent_sample_rate
 

spec/datadog/tracing/trace_operation_spec.rb

@@ -776,23 +776,14 @@
   end
 
   describe '#set_tag' do
-    it 'needs a started trace' do
-      expect { trace_op.send(:set_tag, 'foo', 'bar') }.to raise_error(Datadog::Tracing::TraceOperation::UnstartedError)
-    end
-
-    it 'needs an unfinished trace' do
-      trace_op.measure('top') {}
-      expect { trace_op.send(:set_tag, 'foo', 'bar') }.to raise_error(Datadog::Tracing::TraceOperation::FinishedError)
-    end
-
     it 'sets tag on trace from a measurement' do
       trace_op.measure('top') do
         trace_op.send(:set_tag, 'foo', 'bar')
       end
 
       trace = trace_op.flush!
 
-      expect(trace.spans.find { |span| span.name == 'top' }.meta).to include('foo' => 'bar')
+      expect(trace.tags).to include('foo' => 'bar')
     end
 
     it 'sets tag on trace from a nested measurement' do
@@ -806,7 +797,7 @@
 
       expect(trace.spans).to have(2).items
       expect(trace.spans.map(&:name)).to include('parent')
-      expect(trace.spans.find { |span| span.name == 'grandparent' }.meta).to include('foo' => 'bar')
+      expect(trace.tags).to include('foo' => 'bar')
     end
 
     context 'with partial flushing' do
@@ -823,12 +814,12 @@
 
         expect(trace.spans).to have(1).items
         expect(trace.spans.map(&:name)).to include('parent')
-        expect(trace.spans.map(&:meta)).to_not include('foo' => 'bar')
+        expect(trace.tags).to_not include('foo' => 'bar')
 
         final_flush = trace_op.flush!
         expect(final_flush.spans).to have(1).items
         expect(final_flush.spans.map(&:name)).to include('grandparent')
-        expect(final_flush.spans.map(&:meta)).to include('foo' => 'bar')
+        expect(final_flush.tags).to include('foo' => 'bar')
       end
     end
   end

spec/ddtrace/transport/trace_formatter_spec.rb

@@ -16,6 +16,10 @@
   let(:trace_options) { {} }
 
   shared_context 'trace metadata' do
+    let(:trace_tags) do
+      nil
+    end
+
     let(:trace_options) do
       {
         resource: resource,
@@ -28,7 +32,8 @@
         rule_sample_rate: rule_sample_rate,
         runtime_id: runtime_id,
         sample_rate: sample_rate,
-        sampling_priority: sampling_priority
+        sampling_priority: sampling_priority,
+        tags: trace_tags
       }
     end
 
@@ -45,6 +50,17 @@
     let(:sampling_priority) { Datadog::Tracing::Sampling::Ext::Priority::USER_KEEP }
   end
 
+  shared_context 'trace metadata with tags' do
+    include_context 'trace metadata'
+
+    let(:trace_tags) do
+      {
+        'foo' => 'bar',
+        'baz' => 42
+      }
+    end
+  end
+
   shared_context 'no root span' do
     let(:trace) { Datadog::Tracing::TraceSegment.new(spans, **trace_options) }
     let(:spans) { Array.new(3) { Datadog::Tracing::Span.new('my.job') } }
@@ -139,6 +155,16 @@
         end
       end
 
+      shared_examples 'root span with generic tags' do
+        context 'metrics' do
+          it { expect(root_span.metrics).to include({ 'baz' => 42 }) }
+        end
+
+        context 'meta' do
+          it { expect(root_span.meta).to include({ 'foo' => 'bar' }) }
+        end
+      end
+
       context 'with no root span' do
         include_context 'no root span'
 
@@ -159,6 +185,16 @@
 
           it_behaves_like 'root span with tags'
         end
+
+        context 'when trace has metadata set with generic tags' do
+          include_context 'trace metadata with tags'
+
+          it { is_expected.to be(trace) }
+          it { expect(root_span.resource).to eq('my.job') }
+
+          it_behaves_like 'root span with tags'
+          it_behaves_like 'root span with generic tags'
+        end
       end
 
       context 'with missing root span' do
@@ -181,6 +217,16 @@
 
           it_behaves_like 'root span with tags'
         end
+
+        context 'when trace has metadata set with generic tags' do
+          include_context 'trace metadata with tags'
+
+          it { is_expected.to be(trace) }
+          it { expect(root_span.resource).to eq('my.job') }
+
+          it_behaves_like 'root span with tags'
+          it_behaves_like 'root span with generic tags'
+        end
       end
 
       context 'with a root span' do