Add DD_TAGS environment variable

lib/ddtrace/correlation.rb

@@ -1,4 +1,4 @@
-require 'ddtrace/ext/environment'
+require 'ddtrace/environment'
 
 module Datadog
   # Contains behavior for managing correlations with tracing
@@ -10,7 +10,7 @@ def initialize(*args)
         super
         self.trace_id = trace_id || 0
         self.span_id = span_id || 0
-        self.env = env || ENV[Datadog::Ext::Environment::ENV_ENVIRONMENT]
+        self.env = env || Datadog::Environment.env
       end
 
       def to_s

lib/ddtrace/environment.rb

@@ -1,6 +1,27 @@
+require 'ddtrace/ext/environment'
+
 module Datadog
   # Namespace for handling application environment
   module Environment
+    # TODO: Extract to Datadog::Configuration::Settings
+    def self.env
+      ENV[Ext::Environment::ENV_ENVIRONMENT]
+    end
+
+    # TODO: Extract to Datadog::Configuration::Settings
+    def self.tags
+      tags = {}
+
+      env_to_list(Ext::Environment::ENV_TAGS).each do |tag|
+        pair = tag.split(':')
+        tags[pair.first] = pair.last if pair.length == 2
+      end
+
+      tags['env'] = env unless env.nil?
+
+      tags
+    end
+
     # Defines helper methods for environment
     module Helpers
       def env_to_bool(var, default = nil)

lib/ddtrace/ext/environment.rb

@@ -2,6 +2,7 @@ module Datadog
   module Ext
     module Environment
       ENV_ENVIRONMENT = 'DD_ENV'.freeze
+      ENV_TAGS = 'DD_TAGS'.freeze
     end
   end
 end

lib/ddtrace/metrics.rb

@@ -1,8 +1,8 @@
 require 'ddtrace/ext/metrics'
-require 'ddtrace/ext/environment'
 
 require 'set'
 require 'logger'
+require 'ddtrace/environment'
 require 'ddtrace/utils/time'
 require 'ddtrace/runtime/identity'
 
@@ -152,7 +152,7 @@ def default_metric_options
         # and defaults are unfrozen for mutation in Statsd.
         DEFAULT.dup.tap do |options|
           options[:tags] = options[:tags].dup
-          options[:tags] << "env:#{ENV[Ext::Environment::ENV_ENVIRONMENT]}" if ENV.key?(Ext::Environment::ENV_ENVIRONMENT)
+          options[:tags] << "env:#{Datadog::Environment.env}" unless Datadog::Environment.env.nil?
         end
       end
     end

lib/ddtrace/tracer.rb

@@ -3,8 +3,7 @@
 require 'logger'
 require 'pathname'
 
-require 'ddtrace/ext/environment'
-
+require 'ddtrace/environment'
 require 'ddtrace/span'
 require 'ddtrace/context'
 require 'ddtrace/logger'
@@ -85,9 +84,7 @@ def initialize(options = {})
                        end
 
       @mutex = Mutex.new
-      @tags = {}.tap do |tags|
-        tags[:env] = ENV[Ext::Environment::ENV_ENVIRONMENT] if ENV.key?(Ext::Environment::ENV_ENVIRONMENT)
-      end
+      @tags = Datadog::Environment.tags
 
       # Enable priority sampling by default
       activate_priority_sampling!(@sampler)
@@ -158,7 +155,8 @@ def default_service
     #
     #   tracer.set_tags('env' => 'prod', 'component' => 'core')
     def set_tags(tags)
-      @tags.update(tags)
+      string_tags = Hash[tags.collect { |k, v| [k.to_s, v] }]
+      @tags.update(string_tags)
     end
 
     # Guess context and parent from child_of entry.
@@ -210,8 +208,8 @@ def start_span(name, options = {})
         # child span
         span.parent = parent # sets service, trace_id, parent_id, sampled
       end
-      tags.each { |k, v| span.set_tag(k, v) } unless tags.empty?
       @tags.each { |k, v| span.set_tag(k, v) } unless @tags.empty?
+      tags.each { |k, v| span.set_tag(k, v) } unless tags.empty?
       span.start_time = start_time
 
       # this could at some point be optional (start_active_span vs start_manual_span)

spec/ddtrace/configuration/settings_spec.rb

@@ -77,8 +77,8 @@
         expect(Datadog::Logger.log).to eq(custom_log)
         expect(tracer.writer.transport.current_api.adapter.hostname).to eq('tracer.host.com')
         expect(tracer.writer.transport.current_api.adapter.port).to eq(1234)
-        expect(tracer.tags[:env]).to eq(:config_test)
-        expect(tracer.tags[:foo]).to eq(:bar)
+        expect(tracer.tags['env']).to eq(:config_test)
+        expect(tracer.tags['foo']).to eq(:bar)
       end
     end
 

spec/ddtrace/correlation_spec.rb

@@ -37,27 +37,17 @@
         expect(correlation_ids.to_s).to eq("dd.trace_id=#{trace_id} dd.span_id=#{span_id}")
       end
 
-      context "when #{Datadog::Ext::Environment::ENV_ENVIRONMENT}" do
-        context 'is not defined' do
-          around do |example|
-            ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => nil) do
-              example.run
-            end
-          end
+      context 'when Datadog::Environment.env' do
+        before { allow(Datadog::Environment).to receive(:env).and_return(environment) }
 
+        context 'is not defined' do
+          let(:environment) { nil }
           it { expect(correlation_ids.env).to be nil }
           it { expect(correlation_ids.to_s).to eq("dd.trace_id=#{trace_id} dd.span_id=#{span_id}") }
         end
 
         context 'is defined' do
           let(:environment) { 'my-env' }
-
-          around do |example|
-            ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => environment) do
-              example.run
-            end
-          end
-
           it { expect(correlation_ids.env).to eq environment }
           it { expect(correlation_ids.to_s).to eq("dd.trace_id=#{trace_id} dd.span_id=#{span_id} dd.env=#{environment}") }
         end

spec/ddtrace/environment_spec.rb

@@ -0,0 +1,100 @@
+require 'spec_helper'
+
+require 'ddtrace'
+require 'ddtrace/environment'
+
+RSpec.describe Datadog::Environment do
+  describe '::env' do
+    subject(:env) { described_class.env }
+    context "when #{Datadog::Ext::Environment::ENV_ENVIRONMENT}" do
+      around do |example|
+        ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => environment) do
+          example.run
+        end
+      end
+
+      context 'is not defined' do
+        let(:environment) { nil }
+        it { is_expected.to be nil }
+      end
+
+      context 'is defined' do
+        let(:environment) { 'env-value' }
+        it { is_expected.to eq(environment) }
+      end
+    end
+  end
+
+  describe '::tags' do
+    subject(:tags) { described_class.tags }
+
+    context "when #{Datadog::Ext::Environment::ENV_TAGS}" do
+      around do |example|
+        ClimateControl.modify(Datadog::Ext::Environment::ENV_TAGS => env_tags) do
+          example.run
+        end
+      end
+
+      context 'is not defined' do
+        let(:env_tags) { nil }
+        it { is_expected.to eq({}) }
+      end
+
+      context 'is defined' do
+        let(:env_tags) { 'a:1,b:2' }
+
+        it { is_expected.to include('a' => '1', 'b' => '2') }
+
+        context 'with an invalid tag' do
+          context do
+            let(:env_tags) { '' }
+            it { is_expected.to eq({}) }
+          end
+
+          context do
+            let(:env_tags) { 'a' }
+            it { is_expected.to eq({}) }
+          end
+
+          context do
+            let(:env_tags) { ':' }
+            it { is_expected.to eq({}) }
+          end
+
+          context do
+            let(:env_tags) { ',' }
+            it { is_expected.to eq({}) }
+          end
+
+          context do
+            let(:env_tags) { 'a:' }
+            it { is_expected.to eq({}) }
+          end
+        end
+
+        context 'and when ::env' do
+          context 'is set' do
+            before { allow(described_class).to receive(:env).and_return(nil) }
+            it { is_expected.to_not include('env') }
+          end
+
+          context 'is not set' do
+            let(:env_value) { 'env-value' }
+            before { allow(described_class).to receive(:env).and_return(env_value) }
+            it { is_expected.to include('env' => env_value) }
+          end
+        end
+      end
+
+      context 'conflicts with ::env' do
+        let(:env_tags) { "env:#{tag_env_value}" }
+        let(:tag_env_value) { 'tag-env-value' }
+        let(:env_value) { 'env-value' }
+
+        before { allow(described_class).to receive(:env).and_return(env_value) }
+
+        it { is_expected.to include('env' => env_value) }
+      end
+    end
+  end
+end

spec/ddtrace/metrics_spec.rb

@@ -638,26 +638,16 @@
           )
         end
 
-        context "when #{Datadog::Ext::Environment::ENV_ENVIRONMENT}" do
-          context 'is not defined' do
-            around do |example|
-              ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => nil) do
-                example.run
-              end
-            end
+        context 'when Datadog::Environment.env' do
+          before { allow(Datadog::Environment).to receive(:env).and_return(environment) }
 
+          context 'is not defined' do
+            let(:environment) { nil }
             it { is_expected.to_not include(/env:/) }
           end
 
           context 'is defined' do
             let(:environment) { 'my-env' }
-
-            around do |example|
-              ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => environment) do
-                example.run
-              end
-            end
-
             it { is_expected.to include("env:#{environment}") }
           end
         end

spec/ddtrace/tracer_spec.rb

@@ -21,30 +21,79 @@
 
   describe '#tags' do
     subject(:tags) { tracer.tags }
+    let(:env_tags) { {} }
 
-    it { is_expected.to be_a_kind_of(Hash) }
+    before { allow(Datadog::Environment).to receive(:tags).and_return(env_tags) }
 
-    context "when #{Datadog::Ext::Environment::ENV_ENVIRONMENT}" do
-      context 'is not defined' do
-        around do |example|
-          ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => nil) do
-            example.run
-          end
+    context 'by default' do
+      it { is_expected.to be env_tags }
+    end
+
+    context 'when equivalent String and Symbols are added' do
+      shared_examples 'equivalent tags' do
+        it 'retains the tag only as a String' do
+          is_expected.to include('host')
+          is_expected.to_not include(:host)
         end
 
-        it { is_expected.to_not include(:env) }
+        it 'retains only the last value' do
+          is_expected.to include('host' => 'b')
+        end
       end
 
-      context 'is defined' do
-        let(:environment) { 'my-env' }
+      context 'with #set_tags' do
+        it_behaves_like 'equivalent tags' do
+          before do
+            tracer.set_tags('host' => 'a')
+            tracer.set_tags(host: 'b')
+          end
+        end
 
-        around do |example|
-          ClimateControl.modify(Datadog::Ext::Environment::ENV_ENVIRONMENT => environment) do
-            example.run
+        it_behaves_like 'equivalent tags' do
+          before do
+            tracer.set_tags(host: 'a')
+            tracer.set_tags('host' => 'b')
           end
         end
+      end
+    end
+  end
+
+  describe '#start_span' do
+    subject(:start_span) { tracer.start_span(name, options) }
+    let(:span) { start_span }
+    let(:name) { 'span.name' }
+    let(:options) { {} }
+
+    it { is_expected.to be_a_kind_of(Datadog::Span) }
+
+    context 'when :tags are given' do
+      let(:options) { super().merge(tags: tags) }
+      let(:tags) { { tag_name => tag_value } }
+      let(:tag_name) { 'my-tag' }
+      let(:tag_value) { 'my-value' }
+
+      it { expect(span.get_tag(tag_name)).to eq(tag_value) }
+
+      context 'and default tags are set on the tracer' do
+        let(:default_tags) { { default_tag_name => default_tag_value } }
+        let(:default_tag_name) { 'default_tag' }
+        let(:default_tag_value) { 'default_value' }
+
+        before { tracer.set_tags(default_tags) }
+
+        it 'includes both :tags and default tags' do
+          expect(span.get_tag(default_tag_name)).to eq(default_tag_value)
+          expect(span.get_tag(tag_name)).to eq(tag_value)
+        end
+
+        context 'which conflicts with :tags' do
+          let(:tag_name) { default_tag_name }
 
-        it { is_expected.to include(env: environment) }
+          it 'uses the tag from :tags' do
+            expect(span.get_tag(tag_name)).to eq(tag_value)
+          end
+        end
       end
     end
   end