Merge pull request #100 from solarwinds/NH-69750

NH-69750: change trigger_tracing_mode to symbol

.github/pull_request_template.md

@@ -0,0 +1,4 @@
+### Description
+
+
+### Test (if applicable)

.gitignore

@@ -47,6 +47,7 @@ appoptics.com
 # test script
 install_gem.sh
 push_to_packgecloud.sh
+push_to_github.sh
 sample_start.rb
 
 # github action

CONFIGURATION.md

@@ -116,9 +116,9 @@ Environment Variable | Config File Key | Description | Default
 `SW_APM_PROXY` | `:http_proxy` | Configure an HTTP proxy through which the library connects to the collector. | None
 `SW_APM_SERVICE_KEY` | `:service_key` | API token and service name in the form of _token:service name_, **required**. |
 `SW_APM_TAG_SQL` | `:tag_sql` | Enable/disable injecting trace context into supported SQL statements. Set to boolean true or (or string `true` in env var) to enable. | false
-`SW_APM_TRIGGER_TRACING_MODE` | N/A | Enable/disable trigger tracing for the service.  Setting to `disabled` may impact DEM visibility into the service. | `enabled`
+`SW_APM_TRIGGER_TRACING_MODE` | `:trigger_tracing_mode` | Enable/disable trigger tracing for the service.  Setting to `disabled` may impact DEM visibility into the service. | `enabled`
 `SW_APM_TRUSTEDPATH` | N/A | The library uses the host system's default trusted CA certificates to verify the TLS connection to the collector. To override the default, define the trusted certificate path configuration option with an absolute path to a specific trusted certificate file in PEM format. | None
 N/A | `:log_args` | Enable/disable the collection of URL query parameters, set to boolean false to disable. | true
 N/A | `:log_traceId` | Configure the insertion of trace context into application logs, setting `:traced` would include the available context fields such as trace_id, span_id into log messages. | `:never`
 N/A | `:tracing_mode` | Enable/disable the tracing mode for this service, setting `:disabled` would suppress all trace spans and metrics. | `:enabled`
-N/A | `:transaction_settings` | Configure tracing mode per transaction, aka transaction filtering. | None
+N/A | `:transaction_settings` | Configure tracing mode per transaction, aka transaction filtering. | None

lib/rails/generators/solarwinds_apm/templates/solarwinds_apm_initializer.rb

@@ -147,7 +147,7 @@
   # Trace options is a custom HTTP header X-Trace-Options that can be set on a request to carry additional information
   # to the agents, one such option being trigger-trace which we’ll call a trigger trace request.
   #
-  SolarWindsAPM::Config[:trigger_tracing_mode] = 'enabled'
+  SolarWindsAPM::Config[:trigger_tracing_mode] = :enabled
 
   #
   # Argument logging

lib/solarwinds_apm/api/current_trace_info.rb

@@ -51,7 +51,7 @@ def current_trace_info
       class TraceInfo
         attr_reader :tracestring, :trace_id, :span_id, :trace_flags, :do_log
 
-        REGEXP = /^(?<tracestring>(?<version>[a-f0-9]{2})-(?<trace_id>[a-f0-9]{32})-(?<span_id>[a-f0-9]{16})-(?<flags>[a-f0-9]{2}))$/.freeze
+        REGEXP = /^(?<tracestring>(?<version>[a-f0-9]{2})-(?<trace_id>[a-f0-9]{32})-(?<span_id>[a-f0-9]{16})-(?<flags>[a-f0-9]{2}))$/.freeze # rubocop:disable Style/RedundantFreeze
         private_constant :REGEXP
 
         def initialize

lib/solarwinds_apm/config.rb

@@ -12,6 +12,11 @@ module SolarWindsAPM
   # Use SolarWindsAPM::Config.show to view the entire nested hash.
   #
   module Config
+    LOGGER_LEVEL_MAPPING = {0 => ::Logger::FATAL, 1 => ::Logger::ERROR,
+                            2 => ::Logger::WARN, 3 => ::Logger::INFO,
+                            4 => ::Logger::DEBUG, 5 => ::Logger::DEBUG,
+                            6 => ::Logger::DEBUG}.freeze
+
     @@config = {}
     @@instrumentation = [:action_controller, :action_controller_api, :action_view,
                          :active_record, :bunnyclient, :bunnyconsumer, :curb,
@@ -44,19 +49,20 @@ def self.load_config_file
       config_files << config_file if File.exist?(config_file)
 
       # Check for file set by env variable
-      config_files << config_from_env if ENV.has_key?('SW_APM_CONFIG_RUBY')
+      config_files << config_file_from_env if ENV.has_key?('SW_APM_CONFIG_RUBY')
 
       # Check for default config file
       config_file = File.join(Dir.pwd, 'solarwinds_apm_config.rb')
       config_files << config_file if File.exist?(config_file)
 
+      SolarWindsAPM.logger.debug {"[#{name}/#{__method__}] Available config_files: #{config_files.join(', ')}" }
       SolarWindsAPM.logger.warn {"[#{name}/#{__method__}] Multiple configuration files configured, using the first one listed: #{config_files.join(', ')}"} if config_files.size > 1
       load(config_files[0]) if config_files.size > 0
 
       set_log_level        # sets SolarWindsAPM::Config[:debug_level], SolarWindsAPM.logger.level
     end
 
-    def self.config_from_env
+    def self.config_file_from_env
       if File.exist?(ENV['SW_APM_CONFIG_RUBY']) && !File.directory?(ENV['SW_APM_CONFIG_RUBY'])
         config_file = ENV['SW_APM_CONFIG_RUBY']
       elsif File.exist?(File.join(ENV['SW_APM_CONFIG_RUBY'], 'solarwinds_apm_config.rb'))
@@ -68,11 +74,37 @@ def self.config_from_env
     end
 
     def self.set_log_level
-      SolarWindsAPM::Config[:debug_level] = 3 unless (-1..6).cover?(SolarWindsAPM::Config[:debug_level])
+      log_level = (ENV['SW_APM_DEBUG_LEVEL'] || SolarWindsAPM::Config[:debug_level] || 3).to_i
+      SolarWindsAPM.logger.level = LOGGER_LEVEL_MAPPING[log_level] || ::Logger::INFO # default log level info
+    end
+
+    def self.enable_disable_config(env_var, key, value, default, bool: false)
+      env_value = ENV[env_var.to_s]&.downcase
+      valid_env_values = bool ? %w[true false] : %w[enabled disabled]
+
+      if env_var && valid_env_values.include?(env_value)
+        value = bool ? true?(env_value) : env_value.to_sym
+      elsif env_var && !env_value.to_s.empty?
+        SolarWindsAPM.logger.warn("[#{name}/#{__method__}] #{env_var} must be #{valid_env_values.join('/')} (current setting is #{ENV[env_var]}). Using default value: #{default}.")
+        return @@config[key.to_sym] = default
+      end
+
+      return @@config[key.to_sym] = value unless (bool && !boolean?(value)) || (!bool && !symbol?(value))
+
+      SolarWindsAPM.logger.warn("[#{name}/#{__method__}] :#{key} must be a #{valid_env_values.join('/')}. Using default value: #{default}.")
+      @@config[key.to_sym] = default
+    end
+
+    def self.true?(obj)
+      obj.to_s.casecmp("true").zero?
+    end
 
-      # let's find and use the equivalent debug level for ruby
-      debug_level = (ENV['SW_APM_DEBUG_LEVEL'] || SolarWindsAPM::Config[:debug_level] || 3).to_i
-      SolarWindsAPM.logger.level = debug_level < 0 ? 6 : [4 - debug_level, 0].max
+    def self.boolean?(obj)
+      [true, false].include?(obj)
+    end
+
+    def self.symbol?(obj)
+      [:enabled, :disabled].include?(obj)
     end
 
     ##
@@ -93,8 +125,9 @@ def self.print_config
     #
     # Initializer method to set everything up with a default configuration.
     # The defaults are read from the template configuration file.
+    # This will be called when require 'solarwinds_apm/config' happen
     #
-    def self.initialize(_data={})
+    def self.initialize
       # for config file backward compatibility
       @@instrumentation.each {|inst| @@config[inst] = {}}
       @@config[:transaction_name] = {}
@@ -123,6 +156,7 @@ def self.[](key)
     #
     # Config variable assignment method.  Here we validate and store the
     # assigned value(s) and trigger any secondary action needed.
+    # ENV always have higher precedence 
     #
     def self.[]=(key, value)
       key = key.to_sym
@@ -151,17 +185,14 @@ def self.[]=(key, value)
       when :transaction_settings
         compile_settings(value)
 
+      when :trigger_tracing_mode
+        enable_disable_config('SW_APM_TRIGGER_TRACING_MODE', key, value, :enabled)
+
       when :tracing_mode
-        # ALL TRACING COMMUNICATION TO OBOE IS NOW HANDLED BY TransactionSettings
-        # Make sure that the mode is stored as a symbol
-        @@config[key.to_sym] = value.to_sym
+        enable_disable_config(nil, key, value, :enabled)
 
       when :tag_sql
-        if ENV.has_key?('SW_APM_TAG_SQL')
-          @@config[key.to_sym] = (ENV['SW_APM_TAG_SQL'] == 'true')
-        else
-          @@config[key.to_sym] = value
-        end
+        enable_disable_config('SW_APM_TAG_SQL', key, value, false, bool: true)
 
       else
         @@config[key.to_sym] = value

lib/solarwinds_apm/oboe_init_options.rb

@@ -135,7 +135,6 @@ def read_and_validate_service_key
       token = match[1]
       service_name = match[3]
 
-      puts "validate_token(token): #{validate_token(token)}"
       return '' unless validate_token(token)   # return if token is not even valid
 
       if service_name.empty?

lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb

@@ -131,7 +131,7 @@ def shutdown(timeout: nil)
 
       # This span from inbound HTTP request if from a SERVER by some http.method
       def span_http?(span)
-        (span.kind == ::OpenTelemetry::Trace::SpanKind::SERVER && !span.attributes[HTTP_METHOD].nil?)
+        span.kind == ::OpenTelemetry::Trace::SpanKind::SERVER && !span.attributes[HTTP_METHOD].nil?
       end
 
       # Calculate if this span instance has_error

lib/solarwinds_apm/otel_config.rb

@@ -29,20 +29,12 @@ def self.validate_service_key
     end
 
     def self.resolve_sampler
-
-      resolve_sampler_config
+      sampler_config = {"trigger_trace" => SolarWindsAPM::Config[:trigger_tracing_mode]}
       @@config[:sampler] =
         ::OpenTelemetry::SDK::Trace::Samplers.parent_based(
-          root: SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(@@config[:sampler_config]),
-          remote_parent_sampled: SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(@@config[:sampler_config]),
-          remote_parent_not_sampled: SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(@@config[:sampler_config]))
-    end
-
-    def self.resolve_sampler_config
-      sampler_config = {}
-      sampler_config["trigger_trace"] = "enabled"
-      sampler_config["trigger_trace"] = nil if ENV["SW_APM_TRIGGER_TRACING_MODE"] == 'disabled'
-      @@config[:sampler_config] = sampler_config
+          root: SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(sampler_config),
+          remote_parent_sampled: SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(sampler_config),
+          remote_parent_not_sampled: SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(sampler_config))
     end
 
     #

lib/solarwinds_apm/support/oboe_tracing_mode.rb

@@ -17,15 +17,15 @@ class OboeTracingMode
 
       def self.get_oboe_trace_mode(tracing_mode)
         mode = OBOE_SETTINGS_UNSET
-        mode = OBOE_TRACE_ENABLED if tracing_mode == 'enabled'
-        mode = OBOE_TRACE_DISABLED if tracing_mode == 'disabled'
+        mode = OBOE_TRACE_ENABLED if tracing_mode == :enabled
+        mode = OBOE_TRACE_DISABLED if tracing_mode == :disabled
         mode
       end
 
       def self.get_oboe_trigger_trace_mode(trigger_trace_mode)
         mode = OBOE_SETTINGS_UNSET
-        mode = OBOE_TRIGGER_ENABLED if trigger_trace_mode == 'enabled'
-        mode = OBOE_TRIGGER_DISABLED if trigger_trace_mode == 'disabled'
+        mode = OBOE_TRIGGER_ENABLED if trigger_trace_mode == :enabled
+        mode = OBOE_TRIGGER_DISABLED if trigger_trace_mode == :disabled
         mode
       end
     end

lib/solarwinds_apm/support/swomarginalia/comment.rb

@@ -107,7 +107,7 @@ def self.sidekiq_job
         marginalia_job["class"] if marginalia_job.respond_to?(:[])
       end
 
-      DEFAULT_LINES_TO_IGNORE_REGEX = %r{\.rvm|/ruby/gems/|vendor/|marginalia|rbenv|monitor\.rb.*mon_synchronize}.freeze
+      DEFAULT_LINES_TO_IGNORE_REGEX = %r{\.rvm|/ruby/gems/|vendor/|marginalia|rbenv|monitor\.rb.*mon_synchronize}.freeze # rubocop:disable Style/RedundantFreeze
 
       def self.line
         SWOMarginalia::Comment.lines_to_ignore ||= DEFAULT_LINES_TO_IGNORE_REGEX

lib/solarwinds_apm/support/transaction_settings.rb

@@ -21,15 +21,11 @@ def initialize(url_path: '', name: '', kind: '')
     # calculate trace mode to set either 1 or 0 based on url_path and name+kind
     # first check if url_path match, if not match, then match the name+kind
     def calculate_trace_mode
-      tracing_mode_enabled? && tracing_enabled? ? SWO_TRACING_ENABLED : SWO_TRACING_DISABLED
+      SolarWindsAPM::Config[:tracing_mode] == :enabled && tracing_enabled? ? SWO_TRACING_ENABLED : SWO_TRACING_DISABLED
     end
 
     private
 
-    def tracing_mode_enabled?
-      SolarWindsAPM::Config[:tracing_mode] && ![:disabled, :never].include?(SolarWindsAPM::Config[:tracing_mode])
-    end
-
     def tracing_enabled?
       span_layer = "#{@kind}:#{@name}"
 

test/minitest_helper.rb

@@ -113,6 +113,8 @@ def self.createExit(_args)
     def self.isReady(_args)
       self
     end
+
+    def self.metadataString; end
   end
 end
 

test/opentelemetry/solarwinds_sampler_test.rb

@@ -15,7 +15,7 @@
   before do
 
     sampler_config = {}
-    sampler_config["trigger_trace"] =  "enabled"
+    sampler_config["trigger_trace"] =  :enabled
     @sampler = SolarWindsAPM::OpenTelemetry::SolarWindsSampler.new(sampler_config)
     @decision = {}
     @attributes_dict = {}