feat(span_stack): reaorder config read

Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>

Author: arjun-rajappa

lib/instana/config.rb

@@ -90,20 +90,26 @@ def []=(key, value)
       @config[key.to_sym] = value
     end
 
-    # Read stack trace configuration from YAML file, environment variables, or use defaults
-    # Priority: YAML file > Environment variables > Agent discovery > Defaults
+    # Read stack trace configuration from environment variables, YAML file, or use defaults
+    # Priority: Environment variables > YAML file > Agent discovery > Defaults
     def read_span_stack_config
-      # Try to load from YAML file first
-      yaml_config = read_span_stack_config_from_yaml
+      # Try environment variables first
+      if ENV['INSTANA_STACK_TRACE'] || ENV['INSTANA_STACK_TRACE_LENGTH']
+        read_span_stack_config_from_env
+        @config[:back_trace_technologies] = {}
+        return
+      end
 
+      # Try YAML file
+      yaml_config = read_span_stack_config_from_yaml
       if yaml_config
         @config[:back_trace] = yaml_config[:global]
         @config[:back_trace_technologies] = yaml_config[:technologies] || {}
-      else
-        # Fall back to environment variables or defaults
-        read_span_stack_config_from_env
-        @config[:back_trace_technologies] = {}
+        return
       end
+
+      # Use defaults
+      apply_default_stack_trace_config
     end
 
     # Read configuration from agent discovery response
@@ -171,14 +177,21 @@ def read_span_stack_config_from_yaml
 
     # Read stack trace configuration from environment variables
     def read_span_stack_config_from_env
-      stack_trace = ENV['INSTANA_STACK_TRACE']
-      stack_trace_length = ENV['INSTANA_STACK_TRACE_LENGTH']
-      config_source = stack_trace || stack_trace_length ? 'env' : 'default'
       @config[:back_trace] = {
-        stack_trace_level: stack_trace || 'error',
-        stack_trace_length: stack_trace_length ? stack_trace_length.to_i : 30,
-        config_source: config_source
+        stack_trace_level: ENV['INSTANA_STACK_TRACE'] || 'error',
+        stack_trace_length: ENV['INSTANA_STACK_TRACE_LENGTH']&.to_i || 30,
+        config_source: 'env'
+      }
+    end
+
+    # Apply default stack trace configuration
+    def apply_default_stack_trace_config
+      @config[:back_trace] = {
+        stack_trace_level: 'error',
+        stack_trace_length: 30,
+        config_source: 'default'
       }
+      @config[:back_trace_technologies] = {}
     end
 
     # Check if we should read configuration from agent

test/config_test.rb

@@ -94,7 +94,7 @@ def test_read_span_stack_config_from_env_with_no_env_vars
 
     assert_equal "error", subject[:back_trace][:stack_trace_level]
     assert_equal 30, subject[:back_trace][:stack_trace_length]
-    assert_equal 'default', subject[:back_trace][:config_source]
+    assert_equal 'env', subject[:back_trace][:config_source]
   end
 
   def test_read_span_stack_config_from_env_converts_length_to_integer
@@ -248,7 +248,7 @@ def test_read_span_stack_config_yaml_missing_global_section
     ENV.delete('INSTANA_CONFIG_PATH')
   end
 
-  def test_read_span_stack_config_yaml_takes_precedence_over_env
+  def test_read_span_stack_config_env_takes_precedence_over_yaml
     yaml_content = <<~YAML
       tracing:
         global:
@@ -263,10 +263,10 @@ def test_read_span_stack_config_yaml_takes_precedence_over_env
 
     subject = Instana::Config.new(logger: Logger.new('/dev/null'))
 
-    # YAML should take precedence
-    assert_equal 'all', subject[:back_trace][:stack_trace_level]
-    assert_equal 50, subject[:back_trace][:stack_trace_length]
-    assert_equal 'yaml', subject[:back_trace][:config_source]
+    # Environment variables should take precedence
+    assert_equal 'none', subject[:back_trace][:stack_trace_level]
+    assert_equal 5, subject[:back_trace][:stack_trace_length]
+    assert_equal 'env', subject[:back_trace][:config_source]
   ensure
     File.unlink('test_stack_config.yaml') if File.exist?('test_stack_config.yaml')
     ENV.delete('INSTANA_CONFIG_PATH')
@@ -376,9 +376,35 @@ def test_read_config_from_agent_with_nil_discovery
     assert_equal original_config, subject[:back_trace]
   end
 
-  # Tests for configuration priority: YAML > Env > Agent > Default
+  # Tests for configuration priority: Env > YAML > Agent > Default
+
+  def test_priority_env_over_yaml
+    yaml_content = <<~YAML
+      tracing:
+        global:
+          stack-trace: all
+          stack-trace-length: 50
+    YAML
+
+    File.write('test_stack_config.yaml', yaml_content)
+    ENV['INSTANA_CONFIG_PATH'] = 'test_stack_config.yaml'
+    ENV['INSTANA_STACK_TRACE'] = 'none'
+    ENV['INSTANA_STACK_TRACE_LENGTH'] = '10'
+
+    subject = Instana::Config.new(logger: Logger.new('/dev/null'))
+
+    # Environment variables should take precedence over YAML
+    assert_equal 'none', subject[:back_trace][:stack_trace_level]
+    assert_equal 10, subject[:back_trace][:stack_trace_length]
+    assert_equal 'env', subject[:back_trace][:config_source]
+  ensure
+    File.unlink('test_stack_config.yaml') if File.exist?('test_stack_config.yaml')
+    ENV.delete('INSTANA_CONFIG_PATH')
+    ENV.delete('INSTANA_STACK_TRACE')
+    ENV.delete('INSTANA_STACK_TRACE_LENGTH')
+  end
 
-  def test_priority_yaml_over_agent
+  def test_priority_yaml_over_agent_when_no_env
     yaml_content = <<~YAML
       tracing:
         global:
@@ -402,7 +428,7 @@ def test_priority_yaml_over_agent
     }
     subject.read_config_from_agent(discovery)
 
-    # YAML should take precedence
+    # YAML should take precedence over agent (when no env vars)
     assert_equal 'none', subject[:back_trace][:stack_trace_level]
     assert_equal 10, subject[:back_trace][:stack_trace_length]
     assert_equal 'yaml', subject[:back_trace][:config_source]
@@ -635,7 +661,7 @@ def test_read_config_from_agent_with_technology_specific_config
     assert_nil redis_config[:stack_trace_length]
   end
 
-  def test_yaml_technology_config_not_overridden_by_agent
+  def test_yaml_technology_config_not_overridden_by_agent_when_no_env
     yaml_content = <<~YAML
       tracing:
         global:
@@ -667,7 +693,7 @@ def test_yaml_technology_config_not_overridden_by_agent
     }
     subject.read_config_from_agent(discovery)
 
-    # YAML should take precedence for both global and technology-specific
+    # YAML should take precedence for both global and technology-specific (when no env vars)
     assert_equal 'none', subject[:back_trace][:stack_trace_level]
     assert_equal 10, subject[:back_trace][:stack_trace_length]
     assert_equal 'yaml', subject[:back_trace][:config_source]