Merge pull request #3886 from DataDog/tonycthsu/avoid-telemetry-deadlock

Fix deadlock when sending telemetry logs

lib/datadog/appsec.rb

@@ -23,12 +23,12 @@ def processor
         appsec_component.processor if appsec_component
       end
 
-      def reconfigure(ruleset:, actions:)
+      def reconfigure(ruleset:, actions:, telemetry:)
         appsec_component = components.appsec
 
         return unless appsec_component
 
-        appsec_component.reconfigure(ruleset: ruleset, actions: actions)
+        appsec_component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
       end
 
       def reconfigure_lock(&block)

lib/datadog/appsec/component.rb

@@ -10,10 +10,10 @@ module AppSec
     # Core-pluggable component for AppSec
     class Component
       class << self
-        def build_appsec_component(settings)
+        def build_appsec_component(settings, telemetry:)
           return unless settings.respond_to?(:appsec) && settings.appsec.enabled
 
-          processor = create_processor(settings)
+          processor = create_processor(settings, telemetry)
 
           # We want to always instrument user events when AppSec is enabled.
           # There could be cases in which users use the DD_APPSEC_ENABLED Env variable to
@@ -28,8 +28,11 @@ def build_appsec_component(settings)
 
         private
 
-        def create_processor(settings)
-          rules = AppSec::Processor::RuleLoader.load_rules(ruleset: settings.appsec.ruleset)
+        def create_processor(settings, telemetry)
+          rules = AppSec::Processor::RuleLoader.load_rules(
+            telemetry: telemetry,
+            ruleset: settings.appsec.ruleset
+          )
           return nil unless rules
 
           actions = rules['actions']
@@ -47,9 +50,10 @@ def create_processor(settings)
             rules: [rules],
             data: data,
             exclusions: exclusions,
+            telemetry: telemetry
           )
 
-          processor = Processor.new(ruleset: ruleset)
+          processor = Processor.new(ruleset: ruleset, telemetry: telemetry)
           return nil unless processor.ready?
 
           processor
@@ -63,11 +67,11 @@ def initialize(processor:)
         @mutex = Mutex.new
       end
 
-      def reconfigure(ruleset:, actions:)
+      def reconfigure(ruleset:, actions:, telemetry:)
         @mutex.synchronize do
           AppSec::Processor::Actions.merge(actions)
 
-          new = Processor.new(ruleset: ruleset)
+          new = Processor.new(ruleset: ruleset, telemetry: telemetry)
 
           if new && new.ready?
             old = @processor

lib/datadog/appsec/processor.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require_relative '../core/telemetry/logging'
-
 module Datadog
   module AppSec
     # Processor integrates libddwaf into datadog/appsec
@@ -75,13 +73,15 @@ def extract_schema?
 
       attr_reader :diagnostics, :addresses
 
-      def initialize(ruleset:)
+      def initialize(ruleset:, telemetry:)
         @diagnostics = nil
         @addresses = []
         settings = Datadog.configuration.appsec
+        @telemetry = telemetry
 
-        unless load_libddwaf && create_waf_handle(settings, ruleset)
-          Datadog.logger.warn { 'AppSec is disabled, see logged errors above' }
+        # TODO: Refactor to make it easier to test
+        unless require_libddwaf && libddwaf_provides_waf? && create_waf_handle(settings, ruleset)
+          Datadog.logger.warn('AppSec is disabled, see logged errors above')
         end
       end
 
@@ -99,8 +99,27 @@ def finalize
 
       private
 
-      def load_libddwaf
-        Processor.require_libddwaf && Processor.libddwaf_provides_waf?
+      # libddwaf raises a LoadError on unsupported platforms; it may at some
+      # point succeed in being required yet not provide a specific needed feature.
+      def require_libddwaf
+        Datadog.logger.debug { "libddwaf platform: #{libddwaf_platform}" }
+
+        require 'libddwaf'
+
+        true
+      rescue LoadError => e
+        Datadog.logger.error do
+          'libddwaf failed to load,' \
+            "installed platform: #{libddwaf_platform} ruby platforms: #{ruby_platforms} error: #{e.inspect}"
+        end
+        @telemetry.report(e, description: 'libddwaf failed to load')
+
+        false
+      end
+
+      # check whether libddwaf is required *and* able to provide the needed feature
+      def libddwaf_provides_waf?
+        defined?(Datadog::AppSec::WAF) ? true : false
       end
 
       def create_waf_handle(settings, ruleset)
@@ -121,7 +140,7 @@ def create_waf_handle(settings, ruleset)
         Datadog.logger.error do
           "libddwaf failed to initialize, error: #{e.inspect}"
         end
-        Datadog::Core::Telemetry::Logging.report(e, level: :error, description: 'libddwaf failed to initialize')
+        @telemetry.report(e, description: 'libddwaf failed to initialize')
 
         @diagnostics = e.diagnostics if e.diagnostics
 
@@ -130,46 +149,21 @@ def create_waf_handle(settings, ruleset)
         Datadog.logger.error do
           "libddwaf failed to initialize, error: #{e.inspect}"
         end
-        Datadog::Core::Telemetry::Logging.report(e, level: :error, description: 'libddwaf failed to initialize')
+        @telemetry.report(e, description: 'libddwaf failed to initialize')
 
         false
       end
 
-      class << self
-        # check whether libddwaf is required *and* able to provide the needed feature
-        def libddwaf_provides_waf?
-          defined?(Datadog::AppSec::WAF) ? true : false
-        end
-
-        # libddwaf raises a LoadError on unsupported platforms; it may at some
-        # point succeed in being required yet not provide a specific needed feature.
-        def require_libddwaf
-          Datadog.logger.debug { "libddwaf platform: #{libddwaf_platform}" }
-
-          require 'libddwaf'
-
-          true
-        rescue LoadError => e
-          Datadog.logger.error do
-            'libddwaf failed to load,' \
-              "installed platform: #{libddwaf_platform} ruby platforms: #{ruby_platforms} error: #{e.inspect}"
-          end
-          Datadog::Core::Telemetry::Logging.report(e, level: :error, description: 'libddwaf failed to load')
-
-          false
-        end
-
-        def libddwaf_spec
-          Gem.loaded_specs['libddwaf']
-        end
-
-        def libddwaf_platform
-          libddwaf_spec ? libddwaf_spec.platform.to_s : 'unknown'
+      def libddwaf_platform
+        if Gem.loaded_specs['libddwaf']
+          Gem.loaded_specs['libddwaf'].platform.to_s
+        else
+          'unknown'
         end
+      end
 
-        def ruby_platforms
-          Gem.platforms.map(&:to_s)
-        end
+      def ruby_platforms
+        Gem.platforms.map(&:to_s)
       end
     end
   end

lib/datadog/appsec/processor/rule_loader.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative '../assets'
-require_relative '../../core/telemetry/logging'
 
 module Datadog
   module AppSec
@@ -10,7 +9,7 @@ class Processor
       # that load appsec rules and data from  settings
       module RuleLoader
         class << self
-          def load_rules(ruleset:)
+          def load_rules(ruleset:, telemetry:)
             begin
               case ruleset
               when :recommended, :strict
@@ -36,7 +35,7 @@ def load_rules(ruleset:)
                 "libddwaf ruleset failed to load, ruleset: #{ruleset.inspect} error: #{e.inspect}"
               end
 
-              Core::Telemetry::Logging.report(e, level: :error, description: 'libddwaf ruleset failed to load')
+              telemetry.report(e, description: 'libddwaf ruleset failed to load')
 
               nil
             end

lib/datadog/appsec/processor/rule_merger.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative '../assets'
-require_relative '../../core/telemetry/logging'
 
 module Datadog
   module AppSec
@@ -20,12 +19,34 @@ def initialize(version1, version2)
         end
 
         class << self
+          # TODO: `processors` and `scanners` are not provided by the caller, consider removing them
           def merge(
+            telemetry:,
             rules:, data: [], overrides: [], exclusions: [], custom_rules: [],
             processors: nil, scanners: nil
           )
-            processors ||= default_waf_processors
-            scanners   ||= default_waf_scanners
+            processors ||= begin
+              default_waf_processors
+            rescue StandardError => e
+              Datadog.logger.error("libddwaf rulemerger failed to parse default waf processors. Error: #{e.inspect}")
+              telemetry.report(
+                e,
+                description: 'libddwaf rulemerger failed to parse default waf processors'
+              )
+              []
+            end
+
+            scanners ||= begin
+              default_waf_scanners
+            rescue StandardError => e
+              Datadog.logger.error("libddwaf rulemerger failed to parse default waf scanners. Error: #{e.inspect}")
+              telemetry.report(
+                e,
+                description: 'libddwaf rulemerger failed to parse default waf scanners'
+              )
+              []
+            end
+
             combined_rules = combine_rules(rules)
 
             combined_data = combine_data(data) if data.any?
@@ -44,26 +65,10 @@ def merge(
 
           def default_waf_processors
             @default_waf_processors ||= JSON.parse(Datadog::AppSec::Assets.waf_processors)
-          rescue StandardError => e
-            Datadog.logger.error("libddwaf rulemerger failed to parse default waf processors. Error: #{e.inspect}")
-            Datadog::Core::Telemetry::Logging.report(
-              e,
-              level: :error,
-              description: 'libddwaf rulemerger failed to parse default waf processors'
-            )
-            []
           end
 
           def default_waf_scanners
             @default_waf_scanners ||= JSON.parse(Datadog::AppSec::Assets.waf_scanners)
-          rescue StandardError => e
-            Datadog.logger.error("libddwaf rulemerger failed to parse default waf scanners. Error: #{e.inspect}")
-            Datadog::Core::Telemetry::Logging.report(
-              e,
-              level: :error,
-              description: 'libddwaf rulemerger failed to parse default waf scanners'
-            )
-            []
           end
 
           private

lib/datadog/appsec/remote.rb

@@ -53,7 +53,7 @@ def products
         end
 
         # rubocop:disable Metrics/MethodLength
-        def receivers
+        def receivers(telemetry)
           return [] unless remote_features_enabled?
 
           matcher = Core::Remote::Dispatcher::Matcher::Product.new(ASM_PRODUCTS)
@@ -86,7 +86,10 @@ def receivers
             end
 
             if rules.empty?
-              settings_rules = AppSec::Processor::RuleLoader.load_rules(ruleset: Datadog.configuration.appsec.ruleset)
+              settings_rules = AppSec::Processor::RuleLoader.load_rules(
+                telemetry: telemetry,
+                ruleset: Datadog.configuration.appsec.ruleset
+              )
 
               raise NoRulesError, 'no default rules available' unless settings_rules
 
@@ -99,9 +102,10 @@ def receivers
               overrides: overrides,
               exclusions: exclusions,
               custom_rules: custom_rules,
+              telemetry: telemetry
             )
 
-            Datadog::AppSec.reconfigure(ruleset: ruleset, actions: actions)
+            Datadog::AppSec.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry)
           end
 
           [receiver]

lib/datadog/core/configuration/components.rb

@@ -94,7 +94,9 @@ def initialize(settings)
           # the Core resolver from within your product/component's namespace.
           agent_settings = AgentSettingsResolver.call(settings, logger: @logger)
 
-          @remote = Remote::Component.build(settings, agent_settings)
+          @telemetry = self.class.build_telemetry(settings, agent_settings, @logger)
+
+          @remote = Remote::Component.build(settings, agent_settings, telemetry: telemetry)
           @tracer = self.class.build_tracer(settings, agent_settings, logger: @logger)
           @crashtracker = self.class.build_crashtracker(settings, agent_settings, logger: @logger)
 
@@ -107,8 +109,7 @@ def initialize(settings)
 
           @runtime_metrics = self.class.build_runtime_metrics_worker(settings)
           @health_metrics = self.class.build_health_metrics(settings)
-          @telemetry = self.class.build_telemetry(settings, agent_settings, logger)
-          @appsec = Datadog::AppSec::Component.build_appsec_component(settings)
+          @appsec = Datadog::AppSec::Component.build_appsec_component(settings, telemetry: telemetry)
 
           self.class.configure_tracing(settings)
         end

lib/datadog/core/remote/client/capabilities.rb

@@ -12,10 +12,11 @@ class Client
         class Capabilities
           attr_reader :products, :capabilities, :receivers, :base64_capabilities
 
-          def initialize(settings)
+          def initialize(settings, telemetry)
             @capabilities = []
             @products = []
             @receivers = []
+            @telemetry = telemetry
 
             register(settings)
 
@@ -28,12 +29,12 @@ def register(settings)
             if settings.respond_to?(:appsec) && settings.appsec.enabled
               register_capabilities(Datadog::AppSec::Remote.capabilities)
               register_products(Datadog::AppSec::Remote.products)
-              register_receivers(Datadog::AppSec::Remote.receivers)
+              register_receivers(Datadog::AppSec::Remote.receivers(@telemetry))
             end
 
             register_capabilities(Datadog::Tracing::Remote.capabilities)
             register_products(Datadog::Tracing::Remote.products)
-            register_receivers(Datadog::Tracing::Remote.receivers)
+            register_receivers(Datadog::Tracing::Remote.receivers(@telemetry))
           end
 
           def register_capabilities(capabilities)

lib/datadog/core/remote/component.rb

@@ -150,10 +150,10 @@ class << self
           #
           # Those checks are instead performed inside the worker loop.
           # This allows users to upgrade their agent while keeping their application running.
-          def build(settings, agent_settings)
+          def build(settings, agent_settings, telemetry:)
             return unless settings.remote.enabled
 
-            new(settings, Client::Capabilities.new(settings), agent_settings)
+            new(settings, Client::Capabilities.new(settings, telemetry), agent_settings)
           end
         end
       end

lib/datadog/core/telemetry/component.rb

@@ -5,6 +5,7 @@
 require_relative 'http/transport'
 require_relative 'metrics_manager'
 require_relative 'worker'
+require_relative 'logging'
 
 require_relative '../configuration/ext'
 require_relative '../utils/forking'
@@ -17,6 +18,7 @@ class Component
         attr_reader :enabled
 
         include Core::Utils::Forking
+        include Telemetry::Logging
 
         def self.build(settings, agent_settings, logger)
           enabled = settings.telemetry.enabled