NH-132825: minor improvement

.rubocop.yml

@@ -57,3 +57,5 @@ Style/StringLiterals:
     - 'Gemfile'
 Naming/PredicateMethod:
   Enabled: false
+Style/OneClassPerFile:
+  Enabled: false

lib/solarwinds_apm.rb

@@ -53,6 +53,6 @@
     end
   end
 rescue StandardError => e
-  warn "[solarwinds_apm/error] Problem loading: #{e.inspect}"
-  warn e.backtrace
+  SolarWindsAPM.logger.error { "[solarwinds_apm/error] Problem loading: #{e.inspect}" }
+  SolarWindsAPM.logger.error { e.backtrace&.join("\n") }
 end

lib/solarwinds_apm/api/transaction_name.rb

@@ -57,13 +57,15 @@ def set_transaction_name(custom_name = nil)
 
           if current_span.context.valid?
             current_trace_id = current_span.context.hex_trace_id
-            entry_span_id, trace_flags = solarwinds_processor.txn_manager.get_root_context_h(current_trace_id)&.split('-')
-            if entry_span_id.to_s.empty? || trace_flags.to_s.empty?
+            root_context = solarwinds_processor.txn_manager.get_root_context_h(current_trace_id)
+            parts = root_context&.split('-')
+            if parts.nil? || parts.length != 2
               SolarWindsAPM.logger.warn do
-                "[#{name}/#{__method__}] Set transaction name failed: record not found in the transaction manager."
+                "[#{name}/#{__method__}] Set transaction name failed: record not found or malformed in the transaction manager."
               end
               status = false
             else
+              entry_span_id, _trace_flags = parts
               solarwinds_processor.txn_manager.set("#{current_trace_id}-#{entry_span_id}", custom_name)
               SolarWindsAPM.logger.debug do
                 "[#{name}/#{__method__}] Cached custom transaction name for #{current_trace_id}-#{entry_span_id} as #{custom_name}"

lib/solarwinds_apm/config.rb

@@ -26,6 +26,16 @@ module Config
                              6 => { stdlib: ::Logger::DEBUG, otel: 'debug' } }.freeze
 
     @@config = {}
+    @@config_mutex = ::Mutex.new
+
+    # Type expectations for known configuration keys
+    CONFIG_TYPE_VALIDATORS = {
+      transaction_settings: [Array, NilClass],
+      debug_level: [Integer, NilClass],
+      tracing_mode: [Symbol, NilClass],
+      trigger_tracing_mode: [Symbol, NilClass],
+      tag_sql: [TrueClass, FalseClass, NilClass]
+    }.freeze
 
     ##
     # load_config_file
@@ -49,7 +59,8 @@ def self.load_config_file
       config_files << config_file if File.exist?(config_file)
 
       # Check for file set by env variable
-      config_files << config_file_from_env if ENV.key?('SW_APM_CONFIG_RUBY')
+      config_file_env = config_file_from_env if ENV.key?('SW_APM_CONFIG_RUBY')
+      config_files << config_file_env if config_file_env
 
       # Check for default config file
       config_file = File.join(Dir.pwd, 'solarwinds_apm_config.rb')
@@ -68,15 +79,15 @@ def self.load_config_file
 
     def self.config_file_from_env
       if File.exist?(ENV.fetch('SW_APM_CONFIG_RUBY', nil)) && !File.directory?(ENV.fetch('SW_APM_CONFIG_RUBY', nil))
-        config_file = ENV.fetch('SW_APM_CONFIG_RUBY', nil)
+        ENV.fetch('SW_APM_CONFIG_RUBY', nil)
       elsif File.exist?(File.join(ENV.fetch('SW_APM_CONFIG_RUBY', nil), 'solarwinds_apm_config.rb'))
-        config_file = File.join(ENV.fetch('SW_APM_CONFIG_RUBY', nil), 'solarwinds_apm_config.rb')
+        File.join(ENV.fetch('SW_APM_CONFIG_RUBY', nil), 'solarwinds_apm_config.rb')
       else
         SolarWindsAPM.logger.warn do
           "[#{name}/#{__method__}] Could not find the configuration file set by the SW_APM_CONFIG_RUBY environment variable:  #{ENV.fetch('SW_APM_CONFIG_RUBY', nil)}"
         end
+        nil
       end
-      config_file
     end
 
     def self.set_log_level
@@ -99,7 +110,7 @@ def self.enable_disable_config(env_var, key, value, default, bool: false)
         SolarWindsAPM.logger.warn do
           "[#{name}/#{__method__}] #{env_var} must be #{valid_env_values.join('/')} (current setting is #{raw_env_value}). Using default value: #{default}."
         end
-        return @@config[key] = default
+        return @@config_mutex.synchronize { @@config[key] = default }
       end
 
       # Validate final value efficiently
@@ -109,10 +120,10 @@ def self.enable_disable_config(env_var, key, value, default, bool: false)
         SolarWindsAPM.logger.warn do
           "[#{name}/#{__method__}] :#{key} must be #{valid_env_values.join('/')}. Using default value: #{default}."
         end
-        return @@config[key] = default
+        return @@config_mutex.synchronize { @@config[key] = default }
       end
 
-      @@config[key] = value
+      @@config_mutex.synchronize { @@config[key] = value }
     end
 
     def self.true?(obj)
@@ -134,10 +145,12 @@ def self.symbol?(obj)
     # to create an output similar to the content of the config file
     #
     def self.print_config
-      SolarWindsAPM.logger.debug { "[#{name}/#{__method__}] General configurations list blow:" }
-      @@config.each do |k, v|
-        SolarWindsAPM.logger.debug do
-          "[#{name}/#{__method__}] Config Key/Value: #{k}, #{v.inspect}"
+      SolarWindsAPM.logger.debug { "[#{name}/#{__method__}] General configurations listed below:" }
+      @@config_mutex.synchronize do
+        @@config.each do |k, v|
+          SolarWindsAPM.logger.debug do
+            "[#{name}/#{__method__}] Config Key/Value: #{k}, #{v.inspect}"
+          end
         end
       end
       nil
@@ -151,7 +164,7 @@ def self.print_config
     # This will be called when require 'solarwinds_apm/config' happen
     #
     def self.initialize
-      @@config[:transaction_name] = {}
+      @@config_mutex.synchronize { @@config[:transaction_name] = {} }
 
       # Always load the template, it has all the keys and defaults defined,
       # no guarantee of completeness in the user's config file
@@ -175,7 +188,7 @@ def self.merge!(data)
     end
 
     def self.[](key)
-      @@config[key.to_sym]
+      @@config_mutex.synchronize { @@config[key.to_sym] }
     end
 
     ##
@@ -187,7 +200,19 @@ def self.[](key)
     #
     def self.[]=(key, value)
       key = key.to_sym
-      @@config[key] = value
+
+      # Validate type for known configuration keys
+      if CONFIG_TYPE_VALIDATORS.key?(key)
+        allowed = CONFIG_TYPE_VALIDATORS[key]
+        unless allowed.any? { |type| value.is_a?(type) }
+          SolarWindsAPM.logger.warn do
+            "[#{name}/#{__method__}] Invalid type for :#{key}: expected #{allowed.map(&:name).join('/')}, got #{value.class}. Ignoring."
+          end
+          return
+        end
+      end
+
+      @@config_mutex.synchronize { @@config[key] = value }
 
       case key
       when :sampling_rate
@@ -225,7 +250,7 @@ def self.[]=(key, value)
         SolarWindsAPM.logger.warn { ':log_args is deprecated' }
 
       else
-        @@config[key.to_sym] = value
+        @@config_mutex.synchronize { @@config[key.to_sym] = value }
 
       end
     end

lib/solarwinds_apm/constants.rb

@@ -13,6 +13,9 @@ module Constants
     HTTP_ROUTE                       = 'http.route'
     HTTP_STATUS_CODE                 = 'http.status_code'
     HTTP_URL                         = 'http.url'
+    HTTP_REQUEST_METHOD              = 'http.request.method'
+    HTTP_RESPONSE_STATUS_CODE        = 'http.response.status_code'
+    INVALID_HTTP_STATUS_CODE         = 0
     INTL_SWO_AO_COLLECTOR            = 'collector.appoptics.com'
     INTL_SWO_AO_STG_COLLECTOR        = 'collector-stg.appoptics.com'
     INTL_SWO_COMMA                   = ','

lib/solarwinds_apm/opentelemetry/otlp_processor.rb

@@ -16,15 +16,15 @@ class OTLPProcessor
       SW_IS_ENTRY_SPAN    = 'sw.is_entry_span'
       SW_IS_ERROR         = 'sw.is_error'
 
-      HTTP_METHOD         = 'http.method'
-      HTTP_ROUTE          = 'http.route'
-      HTTP_STATUS_CODE    = 'http.status_code'
-      HTTP_URL            = 'http.url'
+      HTTP_METHOD         = SolarWindsAPM::Constants::HTTP_METHOD
+      HTTP_ROUTE          = SolarWindsAPM::Constants::HTTP_ROUTE
+      HTTP_STATUS_CODE    = SolarWindsAPM::Constants::HTTP_STATUS_CODE
+      HTTP_URL            = SolarWindsAPM::Constants::HTTP_URL
 
-      HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code'
-      HTTP_REQUEST_METHOD = 'http.request.method'
+      HTTP_RESPONSE_STATUS_CODE = SolarWindsAPM::Constants::HTTP_RESPONSE_STATUS_CODE
+      HTTP_REQUEST_METHOD = SolarWindsAPM::Constants::HTTP_REQUEST_METHOD
 
-      INVALID_HTTP_STATUS_CODE = 0
+      INVALID_HTTP_STATUS_CODE = SolarWindsAPM::Constants::INVALID_HTTP_STATUS_CODE
 
       def initialize(txn_manager)
         @txn_manager = txn_manager
@@ -155,17 +155,16 @@ def calculate_transaction_names(span)
       def record_request_metrics(span)
         meter_attrs = meter_attributes(span)
         span_time = calculate_span_time(start_time: span.start_timestamp, end_time: span.end_timestamp)
-        span_time = (span_time / 1e3).round
         SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] entry span, response_time: #{span_time}." }
         @metrics[:response_time].record(span_time, attributes: meter_attrs)
       end
 
-      # Calculate span time in microseconds (us) using start and end time
+      # Calculate span time in milliseconds (ms) using start and end time
       # in nanoseconds (ns). OTel span start/end_time are optional.
       def calculate_span_time(start_time: nil, end_time: nil)
         return 0 if start_time.nil? || end_time.nil?
 
-        ((end_time.to_i - start_time.to_i) / 1e3).round
+        ((end_time.to_i - start_time.to_i) / 1_000_000.0).round
       end
 
       # Calculate if this span instance has_error

lib/solarwinds_apm/opentelemetry/solarwinds_propagator.rb

@@ -35,7 +35,7 @@ class TextMapPropagator
         #   if extraction fails
         def extract(carrier, context: ::OpenTelemetry::Context.current,
                     getter: ::OpenTelemetry::Context::Propagation.text_map_getter)
-          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] extract context: #{context.inspect}" }
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] extract context: #{context.inspect}" } if SolarWindsAPM.logger.debug?
 
           context = ::OpenTelemetry::Context.new({}) if context.nil?
           context = inject_extracted_header(carrier, context, getter, XTRACEOPTIONS_HEADER_NAME, INTL_SWO_X_OPTIONS_KEY)
@@ -55,32 +55,24 @@ def extract(carrier, context: ::OpenTelemetry::Context.current,
         def inject(carrier, context: ::OpenTelemetry::Context.current,
                    setter: ::OpenTelemetry::Context::Propagation.text_map_setter)
           span_context = ::OpenTelemetry::Trace.current_span(context)&.context
-          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] span_context #{span_context.inspect}" }
           return unless span_context&.valid?
 
           trace_flag = span_context.trace_flags.sampled? ? 1 : 0
           sw_value   = "#{span_context.hex_span_id}-0#{trace_flag}"
-          trace_state_header = carrier[TRACESTATE_HEADER_NAME].nil? ? nil : carrier[TRACESTATE_HEADER_NAME]
-          SolarWindsAPM.logger.debug do
-            "[#{self.class}/#{__method__}] sw_value: #{sw_value}; trace_state_header: #{trace_state_header}"
-          end
+          trace_state_header = carrier[TRACESTATE_HEADER_NAME]
+
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] sw_value: #{sw_value}; trace_state_header: #{trace_state_header}" } if SolarWindsAPM.logger.debug?
 
           # prepare carrier with carrier's or new tracestate
           if trace_state_header.nil?
             # only create new trace state if valid span_id
             unless span_context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID
               trace_state = ::OpenTelemetry::Trace::Tracestate.create({ SolarWindsAPM::Constants::INTL_SWO_TRACESTATE_KEY => sw_value })
-              SolarWindsAPM.logger.debug do
-                "[#{self.class}/#{__method__}] creating new trace state: #{trace_state.inspect}"
-              end
               setter.set(carrier, TRACESTATE_HEADER_NAME, Utils.trace_state_header(trace_state))
             end
           else
             trace_state_from_string = ::OpenTelemetry::Trace::Tracestate.from_string(trace_state_header)
             trace_state = trace_state_from_string.set_value(SolarWindsAPM::Constants::INTL_SWO_TRACESTATE_KEY, sw_value)
-            SolarWindsAPM.logger.debug do
-              "[#{self.class}/#{__method__}] updating/adding trace state for injection #{trace_state.inspect}"
-            end
             setter.set(carrier, TRACESTATE_HEADER_NAME, Utils.trace_state_header(trace_state))
           end
         rescue StandardError => e
@@ -93,15 +85,14 @@ def inject(carrier, context: ::OpenTelemetry::Context.current,
         #
         # @return [Array<String>] a list of fields that will be used by this propagator.
         def fields
-          TRACESTATE_HEADER_NAME
+          [TRACESTATE_HEADER_NAME]
         end
 
         private
 
         def inject_extracted_header(carrier, context, getter, header, inject_key)
           extracted_header = getter.get(carrier, header)
           context = context.set_value(inject_key, extracted_header) if extracted_header
-          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] #{header}: #{inject_key} = #{extracted_header}" }
           context
         end
       end

lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb

@@ -60,17 +60,20 @@ def inject(carrier, context: ::OpenTelemetry::Context.current,
           SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Injection failed: #{e.message}" }
         end
 
-        private
+        W3C_UNSANITIZE_MAP = {
+          SolarWindsAPM::Constants::INTL_SWO_EQUALS_W3C_SANITIZED => SolarWindsAPM::Constants::INTL_SWO_EQUALS,
+          ':' => SolarWindsAPM::Constants::INTL_SWO_EQUALS,
+          SolarWindsAPM::Constants::INTL_SWO_COMMA_W3C_SANITIZED => SolarWindsAPM::Constants::INTL_SWO_COMMA
+        }.freeze
+
+        W3C_UNSANITIZE_PATTERN = Regexp.union(W3C_UNSANITIZE_MAP.keys).freeze
 
         # SW_XTRACEOPTIONS_RESPONSE_KEY -> xtrace_options_response
         def recover_response_from_tracestate(span_context)
           sanitized = span_context.tracestate.value(SW_XTRACEOPTIONS_RESPONSE_KEY)
-          sanitized = '' if sanitized.nil?
-          sanitized = sanitized.gsub(SolarWindsAPM::Constants::INTL_SWO_EQUALS_W3C_SANITIZED,
-                                     SolarWindsAPM::Constants::INTL_SWO_EQUALS)
-          sanitized = sanitized.gsub(':', SolarWindsAPM::Constants::INTL_SWO_EQUALS)
-          sanitized.gsub(SolarWindsAPM::Constants::INTL_SWO_COMMA_W3C_SANITIZED,
-                         SolarWindsAPM::Constants::INTL_SWO_COMMA)
+          return '' if sanitized.nil?
+
+          sanitized.gsub(W3C_UNSANITIZE_PATTERN, W3C_UNSANITIZE_MAP)
         end
       end
     end

lib/solarwinds_apm/otel_config.rb

@@ -17,12 +17,18 @@ module OTelConfig
     @@config           = {}
     @@config_map       = {}
     @@agent_enabled    = false
+    @@initialized      = false
 
     RESOURCE_ATTRIBUTES = 'RESOURCE_ATTRIBUTES'
 
     def self.initialize
       return unless defined?(::OpenTelemetry::SDK::Configurator)
 
+      if @@initialized
+        SolarWindsAPM.logger.warn { "[#{name}/#{__method__}] OTelConfig already initialized. Skipping re-initialization." }
+        return
+      end
+
       is_lambda = SolarWindsAPM::Utils.determine_lambda
 
       # add response propagator to rack instrumentation
@@ -39,7 +45,7 @@ def self.initialize
         return if otlp_endpoint.token.nil?
       end
 
-      ENV['OTEL_RESOURCE_ATTRIBUTES'] = "sw.apm.version=#{SolarWindsAPM::Version::STRING},sw.data.module=apm,service.name=#{ENV.fetch('OTEL_SERVICE_NAME', nil)}," + ENV['OTEL_RESOURCE_ATTRIBUTES'].to_s
+      ENV['OTEL_RESOURCE_ATTRIBUTES'] = "sw.apm.version=#{SolarWindsAPM::Version::STRING},sw.data.module=apm,service.name=#{ENV.fetch('OTEL_SERVICE_NAME', nil)}," + ENV['OTEL_RESOURCE_ATTRIBUTES'].to_s unless ENV['OTEL_RESOURCE_ATTRIBUTES'].to_s.include?('sw.apm.version=')
 
       # resource attributes
       mandatory_resource = SolarWindsAPM::ResourceDetector.detect
@@ -114,6 +120,7 @@ def self.initialize
       )
 
       @@agent_enabled = true
+      @@initialized = true
 
       nil
     end

lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils.rb

@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
-# Copyright The OpenTelemetry Authors
+# © 2023 SolarWinds Worldwide, LLC. All rights reserved.
 #
-# SPDX-License-Identifier: Apache-2.0
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at:http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
 
 module SolarWindsAPM
   module Patch

lib/solarwinds_apm/sampling/http_sampler.rb

@@ -99,9 +99,10 @@ def settings_request
       loop do
         response = fetch_with_timeout(@setting_url)
 
-        # Check for nil response from timeout
+        # Check for nil response from timeout or non-success response
         unless response.is_a?(Net::HTTPSuccess)
-          @logger.warn { "[#{self.class}/#{__method__}] Failed to retrieve settings due to timeout." }
+          @logger.warn { "[#{self.class}/#{__method__}] Failed to retrieve settings: #{response.nil? ? 'timeout' : "HTTP #{response.code}"}" }
+          sleep(sleep_duration)
           next
         end
 

lib/solarwinds_apm/sampling/oboe_sampler.rb

@@ -29,7 +29,7 @@ def initialize(logger)
       @counters = SolarWindsAPM::Metrics::Counter.new
       @buckets = {
         SolarWindsAPM::BucketType::DEFAULT =>
-          SolarWindsAPM::TokenBucket.new(SolarWindsAPM::TokenBucketSettings.new(nil, nil, 'DEFUALT')),
+          SolarWindsAPM::TokenBucket.new(SolarWindsAPM::TokenBucketSettings.new(nil, nil, 'DEFAULT')),
         SolarWindsAPM::BucketType::TRIGGER_RELAXED =>
           SolarWindsAPM::TokenBucket.new(SolarWindsAPM::TokenBucketSettings.new(nil, nil, 'TRIGGER_RELAXED')),
         SolarWindsAPM::BucketType::TRIGGER_STRICT =>