From a4d99a1d6d33e365fdb6d010190068dbcae2c482 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Mon, 2 Sep 2024 15:49:12 +0200 Subject: [PATCH 1/4] Inject telemetry component --- lib/datadog/appsec.rb | 4 +- lib/datadog/appsec/component.rb | 18 +- lib/datadog/appsec/processor.rb | 76 +++--- lib/datadog/appsec/processor/rule_loader.rb | 4 +- lib/datadog/appsec/processor/rule_merger.rb | 44 ++-- lib/datadog/appsec/remote.rb | 10 +- lib/datadog/core/configuration/components.rb | 8 +- .../core/remote/client/capabilities.rb | 7 +- lib/datadog/core/remote/component.rb | 4 +- lib/datadog/core/telemetry.rb | 24 ++ lib/datadog/core/telemetry/component.rb | 2 + lib/datadog/core/telemetry/logging.rb | 15 +- lib/datadog/tracing/remote.rb | 2 +- sig/datadog/appsec/processor.rbs | 13 +- sig/datadog/appsec/processor/rule_loader.rbs | 6 +- sig/datadog/appsec/processor/rule_merger.rbs | 11 +- sig/datadog/appsec/remote.rbs | 2 +- .../core/remote/client/capabilities.rbs | 6 +- sig/datadog/core/remote/component.rbs | 6 +- sig/datadog/core/telemetry/logging.rbs | 2 +- sig/datadog/tracing/remote.rbs | 2 +- spec/datadog/appsec/component_spec.rb | 32 ++- .../appsec/processor/rule_loader_spec.rb | 8 +- .../appsec/processor/rule_merger_spec.rb | 117 ++++----- spec/datadog/appsec/processor_spec.rb | 236 +++++++++++++----- spec/datadog/appsec/remote_spec.rb | 41 ++- spec/datadog/appsec/scope_spec.rb | 5 +- .../core/remote/client/capabilities_spec.rb | 3 +- spec/datadog/core/remote/client_spec.rb | 5 +- spec/datadog/core/remote/component_spec.rb | 10 +- spec/datadog/tracing/remote_spec.rb | 4 +- 31 files changed, 463 insertions(+), 264 deletions(-) create mode 100644 lib/datadog/core/telemetry.rb diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index 1418acbf92b..c19c54770d9 100644 --- a/lib/datadog/appsec.rb +++ b/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) diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 893436c5d45..2663f594b06 100644 --- a/lib/datadog/appsec/component.rb +++ b/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 diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index 85e3dbb7a83..cd2587514f9 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -75,13 +75,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 +101,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, level: :error, 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 +142,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, level: :error, description: 'libddwaf failed to initialize') @diagnostics = e.diagnostics if e.diagnostics @@ -130,46 +151,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, level: :error, 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 diff --git a/lib/datadog/appsec/processor/rule_loader.rb b/lib/datadog/appsec/processor/rule_loader.rb index 14f1ef5bd66..27a76fe1c52 100644 --- a/lib/datadog/appsec/processor/rule_loader.rb +++ b/lib/datadog/appsec/processor/rule_loader.rb @@ -10,7 +10,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 +36,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, level: :error, description: 'libddwaf ruleset failed to load') nil end diff --git a/lib/datadog/appsec/processor/rule_merger.rb b/lib/datadog/appsec/processor/rule_merger.rb index 515e72db5da..a53c7be7ee3 100644 --- a/lib/datadog/appsec/processor/rule_merger.rb +++ b/lib/datadog/appsec/processor/rule_merger.rb @@ -20,12 +20,36 @@ 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, + level: :error, + 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, + level: :error, + 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 +68,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 diff --git a/lib/datadog/appsec/remote.rb b/lib/datadog/appsec/remote.rb index a10a0bd77e6..29ef57bbd3f 100644 --- a/lib/datadog/appsec/remote.rb +++ b/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] diff --git a/lib/datadog/core/configuration/components.rb b/lib/datadog/core/configuration/components.rb index 254b0165147..40500c0eda8 100644 --- a/lib/datadog/core/configuration/components.rb +++ b/lib/datadog/core/configuration/components.rb @@ -6,6 +6,7 @@ require_relative '../diagnostics/health' require_relative '../logger' require_relative '../runtime/metrics' +require_relative '../telemetry' require_relative '../telemetry/component' require_relative '../workers/runtime_metrics' @@ -94,7 +95,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 +110,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 diff --git a/lib/datadog/core/remote/client/capabilities.rb b/lib/datadog/core/remote/client/capabilities.rb index 00f4b7322a6..6578d6af325 100644 --- a/lib/datadog/core/remote/client/capabilities.rb +++ b/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) diff --git a/lib/datadog/core/remote/component.rb b/lib/datadog/core/remote/component.rb index 096c60708c6..b468ee9dbdc 100644 --- a/lib/datadog/core/remote/component.rb +++ b/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 diff --git a/lib/datadog/core/telemetry.rb b/lib/datadog/core/telemetry.rb new file mode 100644 index 00000000000..20631d5cc79 --- /dev/null +++ b/lib/datadog/core/telemetry.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Datadog + module Core + # Telemetry module for sending telemetry events + module Telemetry + class << self + def report(exception, level: :error, description: nil) + instance&.report(exception, level: level, description: description) + end + + def error(description) + instance&.error(description) + end + + private + + def instance + Datadog.send(:components).telemetry + end + end + end + end +end diff --git a/lib/datadog/core/telemetry/component.rb b/lib/datadog/core/telemetry/component.rb index a1d1786273d..6959977a2ee 100644 --- a/lib/datadog/core/telemetry/component.rb +++ b/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 diff --git a/lib/datadog/core/telemetry/logging.rb b/lib/datadog/core/telemetry/logging.rb index eba7e87719c..4d8ec75c74d 100644 --- a/lib/datadog/core/telemetry/logging.rb +++ b/lib/datadog/core/telemetry/logging.rb @@ -21,7 +21,6 @@ module Telemetry # - What information needed to make it actionable? # module Logging - extend self # Extract datadog stack trace from the exception module DatadogStackTrace @@ -59,23 +58,13 @@ def report(exception, level:, description: nil) stack_trace: DatadogStackTrace.from(exception) ) - dispatch(event) + log!(event) end def error(description) event = Event::Log.new(message: description, level: :error) - dispatch(event) - end - - private - - def dispatch(event) - if (telemetry = Datadog.send(:components).telemetry) - telemetry.log!(event) - else - Datadog.logger.debug { 'Attempting to send telemetry log when telemetry component is not ready' } - end + log!(event) end end end diff --git a/lib/datadog/tracing/remote.rb b/lib/datadog/tracing/remote.rb index ebef54cbf31..adb1ef02a76 100644 --- a/lib/datadog/tracing/remote.rb +++ b/lib/datadog/tracing/remote.rb @@ -45,7 +45,7 @@ def process_config(config, content) content.errored("#{e.class.name} #{e.message}: #{Array(e.backtrace).join("\n")}") end - def receivers + def receivers(_telemetry) receiver do |repository, _changes| # DEV: Filter our by product. Given it will be very common # DEV: we can filter this out before we receive the data in this method. diff --git a/sig/datadog/appsec/processor.rbs b/sig/datadog/appsec/processor.rbs index 0f2be11a9b3..5fc907cb2ac 100644 --- a/sig/datadog/appsec/processor.rbs +++ b/sig/datadog/appsec/processor.rbs @@ -34,7 +34,7 @@ module Datadog @ruleset: ::Hash[::String, untyped] @addresses: ::Array[::String] - def initialize: (ruleset: ::Hash[untyped, untyped]) -> void + def initialize: (ruleset: ::Hash[untyped, untyped], telemetry: Datadog::Core::Telemetry::Component) -> void def ready?: () -> bool def finalize: () -> void @@ -42,14 +42,11 @@ module Datadog private - def load_libddwaf: () -> bool + def require_libddwaf: () -> bool + def libddwaf_provides_waf?: () -> bool def create_waf_handle: (Datadog::Core::Configuration::Settings::_AppSec settings, ::Hash[String, untyped] ruleset) -> bool - - def self.libddwaf_provides_waf?: () -> bool - def self.require_libddwaf: () -> bool - def self.libddwaf_spec: () -> ::Gem::BasicSpecification - def self.libddwaf_platform: () -> ::String - def self.ruby_platforms: () -> ::Array[::String] + def libddwaf_platform: () -> ::String + def ruby_platforms: () -> ::Array[::String] end end end diff --git a/sig/datadog/appsec/processor/rule_loader.rbs b/sig/datadog/appsec/processor/rule_loader.rbs index 4f8fe74a7ba..857caa2d818 100644 --- a/sig/datadog/appsec/processor/rule_loader.rbs +++ b/sig/datadog/appsec/processor/rule_loader.rbs @@ -3,7 +3,11 @@ module Datadog class Processor module RuleLoader type ruleset = Symbol | String | File | StringIO | Hash[String, untyped] - def self.load_rules: (ruleset: ruleset) -> ::Hash[untyped, untyped]? + + def self.load_rules: ( + telemetry: Datadog::Core::Telemetry::Component, + ruleset: ruleset + ) -> ::Hash[untyped, untyped]? def self.load_data: (?ip_denylist: Array[String], ?user_id_denylist: Array[String]) -> Array[Hash[String, untyped]] diff --git a/sig/datadog/appsec/processor/rule_merger.rbs b/sig/datadog/appsec/processor/rule_merger.rbs index a8b71690126..cf4c498fc63 100644 --- a/sig/datadog/appsec/processor/rule_merger.rbs +++ b/sig/datadog/appsec/processor/rule_merger.rbs @@ -14,7 +14,16 @@ module Datadog type processors = ::Array[::Hash[::String, untyped]] type scanners = ::Array[::Hash[::String, untyped]] - def self.merge: (rules: ::Array[rules], ?data: ::Array[data], ?overrides: overrides, ?exclusions: exclusions, ?custom_rules: custom_rules, ?processors: processors?, ?scanners: scanners?) -> rules + def self.merge: ( + telemetry: Datadog::Core::Telemetry::Component, + rules: ::Array[rules], + ?data: ::Array[data], + ?overrides: overrides, + ?exclusions: exclusions, + ?custom_rules: custom_rules, + ?processors: processors?, + ?scanners: scanners? + ) -> rules private diff --git a/sig/datadog/appsec/remote.rbs b/sig/datadog/appsec/remote.rbs index 4639d99cca2..8b1e54b7af2 100644 --- a/sig/datadog/appsec/remote.rbs +++ b/sig/datadog/appsec/remote.rbs @@ -37,7 +37,7 @@ module Datadog def self.products: () -> ::Array[String] - def self.receivers: () -> ::Array[Core::Remote::Dispatcher::Receiver] + def self.receivers: (Datadog::Core::Telemetry::Component telemetry) -> ::Array[Core::Remote::Dispatcher::Receiver] def self.remote_features_enabled?: () -> bool diff --git a/sig/datadog/core/remote/client/capabilities.rbs b/sig/datadog/core/remote/client/capabilities.rbs index f74590664dd..6bd98582e6c 100644 --- a/sig/datadog/core/remote/client/capabilities.rbs +++ b/sig/datadog/core/remote/client/capabilities.rbs @@ -11,8 +11,12 @@ module Datadog attr_reader base64_capabilities: String - def initialize: (Datadog::Core::Configuration::Settings settings) -> void + @telemetry: Datadog::Core::Telemetry::Component + def initialize: ( + Datadog::Core::Configuration::Settings settings, + Datadog::Core::Telemetry::Component telemetry + ) -> void private diff --git a/sig/datadog/core/remote/component.rbs b/sig/datadog/core/remote/component.rbs index 73a01709572..7f15b40de45 100644 --- a/sig/datadog/core/remote/component.rbs +++ b/sig/datadog/core/remote/component.rbs @@ -22,7 +22,11 @@ module Datadog def shutdown!: () -> void - def self.build: (untyped settings, Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings) -> Datadog::Core::Remote::Component? + def self.build: ( + untyped settings, + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings, + telemetry: Datadog::Core::Telemetry::Component + ) -> Datadog::Core::Remote::Component? class Barrier type timeout_s = ::Integer | ::Float diff --git a/sig/datadog/core/telemetry/logging.rbs b/sig/datadog/core/telemetry/logging.rbs index 57cd4ae8d37..75ef035f062 100644 --- a/sig/datadog/core/telemetry/logging.rbs +++ b/sig/datadog/core/telemetry/logging.rbs @@ -16,7 +16,7 @@ module Datadog private - def dispatch: (Datadog::Core::Telemetry::Event::Log event) -> void + def telemetry_component: () -> Datadog::Core::Telemetry::Component end end end diff --git a/sig/datadog/tracing/remote.rbs b/sig/datadog/tracing/remote.rbs index d1d7bcd8235..0c745606125 100644 --- a/sig/datadog/tracing/remote.rbs +++ b/sig/datadog/tracing/remote.rbs @@ -13,7 +13,7 @@ module Datadog def self.process_config: (Hash[String, untyped] config, Core::Remote::Configuration::Content content) -> void - def self.receivers: () -> ::Array[Core::Remote::Dispatcher::Receiver] + def self.receivers: (Datadog::Core::Telemetry::Component OpenTelemetry) -> ::Array[Core::Remote::Dispatcher::Receiver] def self.receiver: (?::Array[String] products) { (Core::Remote::Configuration::Repository repository, Array[Core::Remote::Configuration::Repository::change] changes) -> void } -> ::Array[Core::Remote::Dispatcher::Receiver] diff --git a/spec/datadog/appsec/component_spec.rb b/spec/datadog/appsec/component_spec.rb index 055353c960d..75e0c9fe1cb 100644 --- a/spec/datadog/appsec/component_spec.rb +++ b/spec/datadog/appsec/component_spec.rb @@ -9,18 +9,20 @@ settings end + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + context 'when appsec is enabled' do let(:appsec_enabled) { true } it 'returns a Datadog::AppSec::Component instance' do - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component).to be_a(described_class) end context 'when processor is ready' do it 'returns a Datadog::AppSec::Component with a processor instance' do expect_any_instance_of(Datadog::AppSec::Processor).to receive(:ready?).and_return(true) - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component.processor).to be_a(Datadog::AppSec::Processor) end @@ -29,7 +31,7 @@ context 'when processor fail to instanciate' do it 'returns a Datadog::AppSec::Component with a nil processor' do expect_any_instance_of(Datadog::AppSec::Processor).to receive(:ready?).and_return(false) - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component.processor).to be_nil end @@ -39,7 +41,7 @@ it 'returns a Datadog::AppSec::Component with a nil processor' do expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(nil) - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component.processor).to be_nil end @@ -85,7 +87,7 @@ expect(Datadog::AppSec::Processor::Actions).to receive(:merge).with(actions) expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(ruleset) - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component.processor).to be_a(Datadog::AppSec::Processor) end @@ -119,7 +121,7 @@ expect(Datadog::AppSec::Processor::Actions).to_not receive(:merge) expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).and_return(ruleset) - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component.processor).to be_a(Datadog::AppSec::Processor) end @@ -130,20 +132,24 @@ let(:appsec_enabled) { false } it 'returns nil' do - component = described_class.build_appsec_component(settings) + component = described_class.build_appsec_component(settings, telemetry: telemetry) expect(component).to be_nil end end context 'when appsec is not active' do it 'returns nil' do - component = described_class.build_appsec_component(double(Datadog::Core::Configuration::Settings)) + component = described_class.build_appsec_component( + double(Datadog::Core::Configuration::Settings), + telemetry: telemetry + ) expect(component).to be_nil end end end describe '#reconfigure' do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component, report: nil) } let(:ruleset) do { 'exclusions' => [{ @@ -196,7 +202,7 @@ component = described_class.new(processor: processor) component.instance_variable_set(:@mutex, mutex) expect(mutex).to receive(:synchronize) - component.reconfigure(ruleset: {}, actions: actions) + component.reconfigure(ruleset: {}, actions: actions, telemetry: telemetry) end end @@ -207,7 +213,7 @@ component = described_class.new(processor: processor) expect(Datadog::AppSec::Processor::Actions).to receive(:merge).with(actions) - component.reconfigure(ruleset: ruleset, actions: actions) + component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry) end end @@ -219,7 +225,7 @@ old_processor = component.processor expect(old_processor).to receive(:finalize) - component.reconfigure(ruleset: ruleset, actions: actions) + component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry) new_processor = component.processor expect(new_processor).to_not eq(old_processor) new_processor.finalize @@ -234,7 +240,7 @@ old_processor = component.processor expect(old_processor).to_not receive(:finalize) - component.reconfigure(ruleset: ruleset, actions: actions) + component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry) new_processor = component.processor expect(new_processor).to_not eq(old_processor) new_processor.finalize @@ -251,7 +257,7 @@ ruleset = { 'invalid_one' => true } expect(old_processor).to_not receive(:finalize) - component.reconfigure(ruleset: ruleset, actions: actions) + component.reconfigure(ruleset: ruleset, actions: actions, telemetry: telemetry) expect(component.processor).to eq(old_processor) end end diff --git a/spec/datadog/appsec/processor/rule_loader_spec.rb b/spec/datadog/appsec/processor/rule_loader_spec.rb index 143e5f81412..2ba437698ff 100644 --- a/spec/datadog/appsec/processor/rule_loader_spec.rb +++ b/spec/datadog/appsec/processor/rule_loader_spec.rb @@ -21,8 +21,8 @@ end let(:recommended) { JSON.parse(Datadog::AppSec::Assets.waf_rules(:recommended)) } let(:strict) { JSON.parse(Datadog::AppSec::Assets.waf_rules(:strict)) } - - subject(:rules) { described_class.load_rules(ruleset: ruleset) } + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + subject(:rules) { described_class.load_rules(ruleset: ruleset, telemetry: telemetry) } context 'when ruleset is :recommended' do let(:ruleset) { :recommended } @@ -61,7 +61,7 @@ let(:ruleset) { '/does/not/exist' } it 'returns `nil`' do - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + expect(telemetry).to receive(:report).with( an_instance_of(Errno::ENOENT), level: :error, description: 'libddwaf ruleset failed to load' @@ -93,7 +93,7 @@ let(:ruleset) { StringIO.new('this is not json') } it 'returns `nil`' do - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + expect(telemetry).to receive(:report).with( an_instance_of(JSON::ParserError), level: :error, description: 'libddwaf ruleset failed to load' diff --git a/spec/datadog/appsec/processor/rule_merger_spec.rb b/spec/datadog/appsec/processor/rule_merger_spec.rb index 67081912023..b233aa20835 100644 --- a/spec/datadog/appsec/processor/rule_merger_spec.rb +++ b/spec/datadog/appsec/processor/rule_merger_spec.rb @@ -2,6 +2,15 @@ require 'datadog/appsec/processor/rule_merger' RSpec.describe Datadog::AppSec::Processor::RuleMerger do + around do |example| + described_class.instance_variable_set(:@default_waf_processors, nil) + described_class.instance_variable_set(:@default_waf_scanners, nil) + example.run + described_class.instance_variable_set(:@default_waf_processors, nil) + described_class.instance_variable_set(:@default_waf_scanners, nil) + end + + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } let(:rules) do [ { @@ -150,7 +159,7 @@ }, ] - result = described_class.merge(rules: rules_dup.freeze) + result = described_class.merge(rules: rules_dup.freeze, telemetry: telemetry) expect(result).to include('rules' => expected_result) end @@ -202,7 +211,7 @@ }.freeze expect do - described_class.merge(rules: rules_dup.freeze) + described_class.merge(rules: rules_dup.freeze, telemetry: telemetry) end.to raise_error(described_class::RuleVersionMismatchError) end end @@ -214,7 +223,7 @@ it 'does not merge rules_overrides or exclusions' do expected_result = rules[0] - result = described_class.merge(rules: rules) + result = described_class.merge(rules: rules, telemetry: telemetry) expect(result).to include(expected_result) end end @@ -236,7 +245,7 @@ ] ] - result = described_class.merge(rules: rules, overrides: rules_overrides) + result = described_class.merge(rules: rules, overrides: rules_overrides, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('rules_override' => rules_overrides.flatten) end @@ -311,7 +320,7 @@ ], ] - result = described_class.merge(rules: rules, exclusions: exclusions) + result = described_class.merge(rules: rules, exclusions: exclusions, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('exclusions' => exclusions.flatten) end @@ -385,7 +394,7 @@ }, ] - result = described_class.merge(rules: rules, data: rules_data) + result = described_class.merge(rules: rules, data: rules_data, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('rules_data' => expected_result) end @@ -439,7 +448,7 @@ } ] - result = described_class.merge(rules: rules, data: rules_data) + result = described_class.merge(rules: rules, data: rules_data, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('rules_data' => expected_result) end @@ -486,7 +495,7 @@ } ] - result = described_class.merge(rules: rules, data: rules_data) + result = described_class.merge(rules: rules, data: rules_data, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('rules_data' => expected_result) end @@ -530,7 +539,7 @@ } ] - result = described_class.merge(rules: rules, data: rules_data) + result = described_class.merge(rules: rules, data: rules_data, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('rules_data' => expected_result) end @@ -591,7 +600,7 @@ ] ] - result = described_class.merge(rules: rules, custom_rules: custom_rules) + result = described_class.merge(rules: rules, custom_rules: custom_rules, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('custom_rules' => custom_rules.flatten) end @@ -676,7 +685,13 @@ } ] - result = described_class.merge(rules: rules, data: rules_data, overrides: rules_overrides, exclusions: exclusions) + result = described_class.merge( + rules: rules, + data: rules_data, + overrides: rules_overrides, + exclusions: exclusions, + telemetry: telemetry + ) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('rules_data' => expect_rules_data) expect(result).to include('exclusions' => exclusions.flatten) @@ -687,75 +702,61 @@ context 'processors' do it 'merges default processors' do - result = described_class.merge(rules: rules) + result = described_class.merge(rules: rules, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('processors' => described_class.default_waf_processors) end it 'merges the provided processors' do - result = described_class.merge(rules: rules, processors: 'hello') + result = described_class.merge(rules: rules, processors: 'hello', telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('processors' => 'hello') end - end - - describe '.default_waf_processors' do - around do |example| - described_class.instance_variable_set(:@default_waf_processors, nil) - example.run - described_class.instance_variable_set(:@default_waf_processors, nil) - end - it 'returns default waf processors' do - expect(described_class.default_waf_processors).not_to be_empty - end - - it 'returns []' do - allow(Datadog::AppSec::Assets).to receive(:waf_processors).and_raise - expect(Datadog.logger).to receive(:error).with(/libddwaf rulemerger failed to parse default waf processors/) - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( - a_kind_of(StandardError), - level: :error, - description: 'libddwaf rulemerger failed to parse default waf processors' - ) - expect(described_class.default_waf_processors).to be_empty + context 'when fail to parse default waf processors' do + it 'returns []' do + allow(Datadog::AppSec::Assets).to receive(:waf_processors).and_raise + expect(Datadog.logger).to receive(:error).with(/libddwaf rulemerger failed to parse default waf processors/) + expect(telemetry).to receive(:report).with( + a_kind_of(StandardError), + level: :error, + description: 'libddwaf rulemerger failed to parse default waf processors' + ) + + result = described_class.merge(rules: rules, telemetry: telemetry) + expect(result).to include('rules' => rules[0]['rules']) + expect(result).to include('processors' => []) + end end end context 'scanners' do it 'merges default scanners' do - result = described_class.merge(rules: rules) + result = described_class.merge(rules: rules, telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('scanners' => described_class.default_waf_scanners) end - it 'merges the provided processors' do - result = described_class.merge(rules: rules, scanners: 'hello') + it 'merges the provided scanners' do + result = described_class.merge(rules: rules, scanners: 'hello', telemetry: telemetry) expect(result).to include('rules' => rules[0]['rules']) expect(result).to include('scanners' => 'hello') end - end - - describe '.default_waf_scanners' do - around do |example| - described_class.instance_variable_set(:@default_waf_scanners, nil) - example.run - described_class.instance_variable_set(:@default_waf_scanners, nil) - end - it 'returns default waf scanners' do - expect(described_class.default_waf_scanners).not_to be_empty - end - - it 'returns []' do - allow(Datadog::AppSec::Assets).to receive(:waf_scanners).and_raise - expect(Datadog.logger).to receive(:error).with(/libddwaf rulemerger failed to parse default waf scanners/) - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( - a_kind_of(StandardError), - level: :error, - description: 'libddwaf rulemerger failed to parse default waf scanners' - ) - expect(described_class.default_waf_scanners).to be_empty + context 'when fail to parse default waf scanners' do + it 'returns []' do + allow(Datadog::AppSec::Assets).to receive(:waf_scanners).and_raise + expect(Datadog.logger).to receive(:error).with(/libddwaf rulemerger failed to parse default waf scanners/) + expect(telemetry).to receive(:report).with( + a_kind_of(StandardError), + level: :error, + description: 'libddwaf rulemerger failed to parse default waf scanners' + ) + + result = described_class.merge(rules: rules, telemetry: telemetry) + expect(result).to include('rules' => rules[0]['rules']) + expect(result).to include('scanners' => []) + end end end end diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index dc129f4d9c2..832f2df3f50 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -21,82 +21,192 @@ allow(Datadog).to receive(:logger).and_return(logger) end + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } - let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended) } - - context 'self' do - it 'detects if the WAF is unavailable' do - hide_const('Datadog::AppSec::WAF') - - expect(described_class.libddwaf_provides_waf?).to be false + describe '#require_libddwaf' do + before do + allow_any_instance_of(described_class).to receive(:libddwaf_provides_waf?).and_return(true) + allow_any_instance_of(described_class).to receive(:create_waf_handle).and_return(true) end - it 'detects if the WAF is available' do - stub_const('Datadog::AppSec::WAF', Module.new) + context 'successful' do + it do + allow_any_instance_of(described_class).to receive(:require).with('libddwaf') - expect(described_class.libddwaf_provides_waf?).to be true - end + expect(telemetry).not_to receive(:report) - it 'reports via return of libddwaf loading failure' do - allow(described_class).to receive(:require).with('libddwaf').and_raise(LoadError) + described_class.new(ruleset: ruleset, telemetry: telemetry) - expect(described_class.require_libddwaf).to be false + expect(Datadog.logger).not_to have_received(:warn) + end end - it 'reports via return of libddwaf loading success (first require)' do - allow(described_class).to receive(:require).with('libddwaf').and_return(true) - - expect(described_class.require_libddwaf).to be true - end + context 'when LoadError is raised' do + it do + allow_any_instance_of(described_class).to receive(:require).with('libddwaf').and_raise(LoadError) + expect(telemetry).to receive(:report).with( + an_instance_of(LoadError), + level: :error, + description: 'libddwaf failed to load' + ).at_least(:once) - it 'reports via return of libddwaf loading success (second require)' do - allow(described_class).to receive(:require).with('libddwaf').and_return(false) + described_class.new(ruleset: ruleset, telemetry: telemetry) - expect(described_class.require_libddwaf).to be true + expect(Datadog.logger).to have_received(:warn).with(/AppSec is disabled/) + end end end - describe '#load_libddwaf' do - context 'when LoadError is raised' do - before do - allow(Object).to receive(:require).with('libddwaf').and_raise(LoadError) - end - - it { expect(described_class.new(ruleset: ruleset).send(:load_libddwaf)).to be false } + describe '#libddwaf_provides_waf?' do + before do + allow_any_instance_of(described_class).to receive(:require_libddwaf).and_return(true) + allow_any_instance_of(described_class).to receive(:create_waf_handle).and_return(true) + end + context 'when const is present' do it do - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( - an_instance_of(LoadError), - level: :error, - description: 'libddwaf failed to load' - ) - expect { described_class.new(ruleset: ruleset) }.to_not raise_error + stub_const('Datadog::AppSec::WAF', Module.new) + + described_class.new(ruleset: ruleset, telemetry: telemetry) + + expect(Datadog.logger).not_to have_received(:warn) end end context 'when loaded but missing mandatory const' do - before do - allow(Object).to receive(:require).with('libddwaf').and_return(true) + it do hide_const('Datadog::AppSec::WAF') + + described_class.new(ruleset: ruleset, telemetry: telemetry) + + expect(Datadog.logger).to have_received(:warn).with(/AppSec is disabled/) end + end + end - it { expect(described_class.new(ruleset: ruleset).send(:load_libddwaf)).to be false } + describe '#create_waf_handle' do + before do + allow_any_instance_of(described_class).to receive(:require_libddwaf).and_return(true) + allow_any_instance_of(described_class).to receive(:libddwaf_provides_waf?).and_return(true) end - context 'when loaded successfully' do - before do - allow(Object).to receive(:require).with('libddwaf').and_return(true) + context 'when success' do + it do stub_const('Datadog::AppSec::WAF', Module.new) stub_const('Datadog::AppSec::WAF::LibDDWAF', Module.new) stub_const('Datadog::AppSec::WAF::LibDDWAF::Error', Class.new(StandardError)) + stub_const( + 'Datadog::AppSec::WAF::Handle', + Class.new do + def initialize(*); end + + def diagnostics + :handle_diagnostics + end + + def required_addresses + [:required_addresses] + end + end + ) + expect(telemetry).not_to receive(:report) + + processor = described_class.new(ruleset: ruleset, telemetry: telemetry) + + expect(processor).to be_ready + expect(processor.diagnostics).to eq(:handle_diagnostics) + expect(processor.addresses).to eq([:required_addresses]) + + expect(Datadog.logger).not_to have_received(:warn) end + end - it { expect(described_class.new(ruleset: ruleset).send(:load_libddwaf)).to be true } + context 'when fail' do + it do + stub_const('Datadog::AppSec::WAF', Module.new) + stub_const('Datadog::AppSec::WAF::LibDDWAF', Module.new) + stub_const( + 'Datadog::AppSec::WAF::LibDDWAF::Error', + Class.new(StandardError) do + def diagnostics + :error_diagnostics + end + end + ) + stub_const( + 'Datadog::AppSec::WAF::Handle', + Class.new do + def initialize(*) + raise Datadog::AppSec::WAF::LibDDWAF::Error + end + + def diagnostics + :handle_diagnostics + end + + def required_addresses + [] + end + end + ) + expect(telemetry).to receive(:report).with( + a_kind_of(Datadog::AppSec::WAF::LibDDWAF::Error), + level: :error, + description: 'libddwaf failed to initialize' + ) + + processor = described_class.new(ruleset: ruleset, telemetry: telemetry) + + expect(processor).not_to be_ready + expect(processor.diagnostics).to eq(:error_diagnostics) + expect(processor.addresses).to eq([]) + + expect(Datadog.logger).to have_received(:warn).with(/AppSec is disabled/) + end + + it do + stub_const('Datadog::AppSec::WAF', Module.new) + stub_const('Datadog::AppSec::WAF::LibDDWAF', Module.new) + stub_const( + 'Datadog::AppSec::WAF::LibDDWAF::Error', + Class.new(StandardError) + ) + stub_const( + 'Datadog::AppSec::WAF::Handle', + Class.new do + def initialize(*) + raise StandardError + end + + def diagnostics + :handle_diagnostics + end + + def required_addresses + [] + end + end + ) + expect(telemetry).to receive(:report).with( + a_kind_of(StandardError), + level: :error, + description: 'libddwaf failed to initialize' + ) + + processor = described_class.new(ruleset: ruleset, telemetry: telemetry) + + expect(processor).not_to be_ready + expect(processor.diagnostics).to be_nil + expect(processor.addresses).to eq([]) + + expect(Datadog.logger).to have_received(:warn).with(/AppSec is disabled/) + end end end describe '#initialize' do - subject(:processor) { described_class.new(ruleset: ruleset) } + subject(:processor) { described_class.new(ruleset: ruleset, telemetry: telemetry) } context 'when valid ruleset' do it { is_expected.to be_ready } @@ -104,9 +214,8 @@ context 'when libddwaf fails to load' do before do - expect(described_class).to receive(:require_libddwaf).and_return(false) - expect(Datadog.logger).to receive(:warn) + expect_any_instance_of(described_class).to receive(:require_libddwaf).and_return(false) end it { is_expected.to_not be_ready } @@ -114,8 +223,8 @@ context 'when libddwaf fails to provide WAF' do before do - expect(described_class).to receive(:require_libddwaf).and_return(true) - expect(described_class).to receive(:libddwaf_provides_waf?).and_return(false) + expect_any_instance_of(described_class).to receive(:require_libddwaf).and_return(true) + expect_any_instance_of(described_class).to receive(:libddwaf_provides_waf?).and_return(false) expect(Datadog.logger).to receive(:warn) end @@ -126,11 +235,16 @@ context 'when ruleset is invalid' do let(:ruleset) { { 'not' => 'valid' } } - before do + it do expect(Datadog.logger).to receive(:warn) - end + expect(telemetry).to receive(:report).with( + a_kind_of(StandardError), + level: :error, + description: 'libddwaf failed to initialize' + ) - it { is_expected.to_not be_ready } + is_expected.to_not be_ready + end end context 'when reporting errors' do @@ -138,11 +252,11 @@ stub_const('Datadog::AppSec::WAF::Handle', double) stub_const('Datadog::AppSec::WAF::LibDDWAF::Error', Class.new(StandardError)) - expect(described_class).to receive(:require_libddwaf).and_return(true) - expect(described_class).to receive(:libddwaf_provides_waf?).and_return(true) + expect_any_instance_of(described_class).to receive(:require_libddwaf).and_return(true) + expect_any_instance_of(described_class).to receive(:libddwaf_provides_waf?).and_return(true) expect(Datadog::AppSec::WAF::Handle).to receive(:new).and_raise(StandardError) - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + expect(telemetry).to receive(:report).with( an_instance_of(StandardError), level: :error, description: 'libddwaf failed to initialize' @@ -162,11 +276,11 @@ def diagnostics end ) - expect(described_class).to receive(:require_libddwaf).and_return(true) - expect(described_class).to receive(:libddwaf_provides_waf?).and_return(true) + expect_any_instance_of(described_class).to receive(:require_libddwaf).and_return(true) + expect_any_instance_of(described_class).to receive(:libddwaf_provides_waf?).and_return(true) expect(Datadog::AppSec::WAF::Handle).to receive(:new).and_raise(Datadog::AppSec::WAF::LibDDWAF::Error) - expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + expect(telemetry).to receive(:report).with( an_instance_of(Datadog::AppSec::WAF::LibDDWAF::Error), level: :error, description: 'libddwaf failed to initialize' @@ -179,9 +293,10 @@ def diagnostics end RSpec.describe Datadog::AppSec::Processor::Context do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } let(:ruleset) do - rules = Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended) - Datadog::AppSec::Processor::RuleMerger.merge(rules: [rules]) + rules = Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) + Datadog::AppSec::Processor::RuleMerger.merge(rules: [rules], telemetry: telemetry) end let(:input_safe) { { 'server.request.headers.no_cookies' => { 'user-agent' => 'Ruby' } } } @@ -193,7 +308,7 @@ def diagnostics let(:input) { input_scanner } - let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset) } + let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } let(:run_count) { 1 } let(:timeout) { 10_000_000_000 } @@ -406,12 +521,13 @@ def diagnostics let(:input) { input_client_ip } let(:ruleset) do - rules = Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended) + rules = Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) data = Datadog::AppSec::Processor::RuleLoader.load_data(ip_denylist: [client_ip]) Datadog::AppSec::Processor::RuleMerger.merge( rules: [rules], data: data, + telemetry: telemetry ) end diff --git a/spec/datadog/appsec/remote_spec.rb b/spec/datadog/appsec/remote_spec.rb index 9bd1f06c8e8..acf53ba9abd 100644 --- a/spec/datadog/appsec/remote_spec.rb +++ b/spec/datadog/appsec/remote_spec.rb @@ -48,13 +48,15 @@ end describe '.receivers' do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + context 'remote configuration disabled' do before do expect(described_class).to receive(:remote_features_enabled?).and_return(false) end it 'returns empty array' do - expect(described_class.receivers).to eq([]) + expect(described_class.receivers(telemetry)).to eq([]) end end @@ -64,7 +66,7 @@ end it 'returns receivers' do - receivers = described_class.receivers + receivers = described_class.receivers(telemetry) expect(receivers.size).to eq(1) expect(receivers.first).to be_a(Datadog::Core::Remote::Dispatcher::Receiver) end @@ -106,7 +108,7 @@ }.to_json end - let(:receiver) { described_class.receivers[0] } + let(:receiver) { described_class.receivers(telemetry)[0] } let(:target) do Datadog::Core::Remote::Configuration::Target.parse( @@ -164,8 +166,9 @@ 'scanners' => Datadog::AppSec::Processor::RuleMerger.default_waf_scanners, } - expect(Datadog::AppSec).to receive(:reconfigure).with(ruleset: expected_ruleset, actions: []) - .and_return(nil) + expect(Datadog::AppSec).to receive(:reconfigure).with( + ruleset: expected_ruleset, actions: [], telemetry: telemetry + ).and_return(nil) changes = transaction receiver.call(repository, changes) end @@ -177,7 +180,9 @@ end let(:default_ruleset) do - [Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: Datadog.configuration.appsec.ruleset)] + [Datadog::AppSec::Processor::RuleLoader.load_rules( + ruleset: Datadog.configuration.appsec.ruleset, telemetry: telemetry + )] end let(:target) do @@ -313,6 +318,7 @@ overrides: [rules_override], exclusions: [], custom_rules: [], + telemetry: telemetry ) changes = transaction @@ -334,6 +340,7 @@ overrides: [], exclusions: [exclusions], custom_rules: [], + telemetry: telemetry ) changes = transaction @@ -354,7 +361,8 @@ data: [], overrides: [], exclusions: [], - custom_rules: [custom_rules] + custom_rules: [custom_rules], + telemetry: telemetry ) changes = transaction @@ -370,9 +378,13 @@ end it 'pass the actions to reconfigure' do - ruleset = Datadog::AppSec::Processor::RuleMerger.merge(rules: default_ruleset) + ruleset = Datadog::AppSec::Processor::RuleMerger.merge(rules: default_ruleset, telemetry: telemetry) - expect(Datadog::AppSec).to receive(:reconfigure).with(ruleset: ruleset, actions: actions) + expect(Datadog::AppSec).to receive(:reconfigure).with( + ruleset: ruleset, + actions: actions, + telemetry: telemetry + ) .and_return(nil) changes = transaction @@ -395,6 +407,7 @@ overrides: [rules_override], exclusions: [exclusions], custom_rules: [], + telemetry: telemetry ) changes = transaction @@ -416,6 +429,7 @@ overrides: [], exclusions: [], custom_rules: [], + telemetry: telemetry ) changes = transaction @@ -441,6 +455,7 @@ overrides: [], exclusions: [], custom_rules: [], + telemetry: telemetry ) changes = transaction @@ -462,6 +477,7 @@ overrides: [], exclusions: [], custom_rules: [], + telemetry: telemetry ) changes = transaction @@ -475,17 +491,18 @@ let(:transaction) { repository.transaction { |repository, transaction| } } it 'uses the rules from the appsec settings' do - ruleset = Datadog::AppSec::Processor::RuleMerger.merge(rules: default_ruleset) + ruleset = Datadog::AppSec::Processor::RuleMerger.merge(rules: default_ruleset, telemetry: telemetry) changes = transaction - expect(Datadog::AppSec).to receive(:reconfigure).with(ruleset: ruleset, actions: []) + expect(Datadog::AppSec).to receive(:reconfigure).with(ruleset: ruleset, actions: [], telemetry: telemetry) .and_return(nil) receiver.call(repository, changes) end it 'raises SyncError if no default rules available' do expect(Datadog::AppSec::Processor::RuleLoader).to receive(:load_rules).with( - ruleset: Datadog.configuration.appsec.ruleset + ruleset: Datadog.configuration.appsec.ruleset, + telemetry: telemetry ).and_return(nil) changes = transaction diff --git a/spec/datadog/appsec/scope_spec.rb b/spec/datadog/appsec/scope_spec.rb index b630806bfb8..540e7fc3546 100644 --- a/spec/datadog/appsec/scope_spec.rb +++ b/spec/datadog/appsec/scope_spec.rb @@ -6,9 +6,10 @@ RSpec.describe Datadog::AppSec::Scope do let(:trace) { double } let(:span) { double } + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } - let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended) } - let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset) } + let(:ruleset) { Datadog::AppSec::Processor::RuleLoader.load_rules(ruleset: :recommended, telemetry: telemetry) } + let(:processor) { Datadog::AppSec::Processor.new(ruleset: ruleset, telemetry: telemetry) } after do described_class.send(:reset_active_scope) diff --git a/spec/datadog/core/remote/client/capabilities_spec.rb b/spec/datadog/core/remote/client/capabilities_spec.rb index 75a7e26bc0e..2d5b07184bd 100644 --- a/spec/datadog/core/remote/client/capabilities_spec.rb +++ b/spec/datadog/core/remote/client/capabilities_spec.rb @@ -5,10 +5,11 @@ require 'datadog/appsec/configuration' RSpec.describe Datadog::Core::Remote::Client::Capabilities do - subject(:capabilities) { described_class.new(settings) } + subject(:capabilities) { described_class.new(settings, telemetry) } let(:settings) do double(Datadog::Core::Configuration) end + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } before do capabilities diff --git a/spec/datadog/core/remote/client_spec.rb b/spec/datadog/core/remote/client_spec.rb index d74ebd6c6b1..288b27eaa80 100644 --- a/spec/datadog/core/remote/client_spec.rb +++ b/spec/datadog/core/remote/client_spec.rb @@ -239,7 +239,10 @@ let(:repository) { Datadog::Core::Remote::Configuration::Repository.new } let(:capabilities) do - capabilities = Datadog::Core::Remote::Client::Capabilities.new(Datadog::Core::Configuration::Settings.new) + capabilities = Datadog::Core::Remote::Client::Capabilities.new( + Datadog::Core::Configuration::Settings.new, + instance_double(Datadog::Core::Telemetry::Component) + ) capabilities.send(:register_products, ['ASM_DATA', 'ASM_DD', 'ASM']) capabilities diff --git a/spec/datadog/core/remote/component_spec.rb b/spec/datadog/core/remote/component_spec.rb index 7e02c0c9a3a..b579a97bdac 100644 --- a/spec/datadog/core/remote/component_spec.rb +++ b/spec/datadog/core/remote/component_spec.rb @@ -6,7 +6,8 @@ RSpec.describe Datadog::Core::Remote::Component, :integration do let(:settings) { Datadog::Core::Configuration::Settings.new } let(:agent_settings) { Datadog::Core::Configuration::AgentSettingsResolver.call(settings, logger: nil) } - let(:capabilities) { Datadog::Core::Remote::Client::Capabilities.new(settings) } + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + let(:capabilities) { Datadog::Core::Remote::Client::Capabilities.new(settings, telemetry) } let(:component) { described_class.new(settings, capabilities, agent_settings) } around do |example| @@ -14,7 +15,7 @@ end describe '.build' do - subject(:build) { described_class.build(settings, agent_settings) } + subject(:build) { described_class.build(settings, agent_settings, telemetry: telemetry) } after { build.shutdown! if build } @@ -37,7 +38,10 @@ let(:component) { double('component', shutdown!: nil) } it 'initializes component' do - expect(Datadog::Core::Remote::Client::Capabilities).to receive(:new).with(settings).and_return(capabilities) + expect(Datadog::Core::Remote::Client::Capabilities).to receive(:new).with( + settings, + telemetry + ).and_return(capabilities) expect(described_class).to receive(:new).with(settings, capabilities, agent_settings).and_return(component) is_expected.to eq(component) diff --git a/spec/datadog/tracing/remote_spec.rb b/spec/datadog/tracing/remote_spec.rb index 00d57036f39..2f03a176608 100644 --- a/spec/datadog/tracing/remote_spec.rb +++ b/spec/datadog/tracing/remote_spec.rb @@ -13,7 +13,9 @@ end it 'declares matches that match APM_TRACING' do - expect(remote.receivers).to all( + telemetry = instance_double(Datadog::Core::Telemetry::Component) + + expect(remote.receivers(telemetry)).to all( match( lambda do |receiver| receiver.match? Datadog::Core::Remote::Configuration::Path.parse(path) From b39b4507b2bb30b4bd7371ff4fea5cd5b0b9b895 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Thu, 5 Sep 2024 16:19:43 +0200 Subject: [PATCH 2/4] Refactor with Logger module --- lib/datadog/appsec/processor.rb | 2 - lib/datadog/appsec/processor/rule_loader.rb | 1 - lib/datadog/appsec/processor/rule_merger.rb | 1 - lib/datadog/core/configuration/components.rb | 1 - lib/datadog/core/telemetry.rb | 24 ------- lib/datadog/core/telemetry/logger.rb | 26 ++++++++ lib/datadog/core/telemetry/logging.rb | 1 - lib/datadog/profiling/http_transport.rb | 6 +- lib/datadog/profiling/scheduler.rb | 4 +- lib/datadog/profiling/stack_recorder.rb | 4 +- sig/datadog/appsec.rbs | 6 +- sig/datadog/appsec/component.rbs | 6 +- sig/datadog/core/telemetry/component.rbs | 4 +- sig/datadog/core/telemetry/logger.rbs | 15 +++++ sig/datadog/core/telemetry/logging.rbs | 12 ++-- sig/datadog/tracing/remote.rbs | 2 +- spec/datadog/core/telemetry/logger_spec.rb | 64 ++++++++++++++++++ spec/datadog/core/telemetry/logging_spec.rb | 66 ++++++------------- spec/datadog/profiling/http_transport_spec.rb | 4 +- spec/datadog/profiling/scheduler_spec.rb | 2 +- spec/datadog/profiling/stack_recorder_spec.rb | 2 +- 21 files changed, 153 insertions(+), 100 deletions(-) delete mode 100644 lib/datadog/core/telemetry.rb create mode 100644 lib/datadog/core/telemetry/logger.rb create mode 100644 sig/datadog/core/telemetry/logger.rbs create mode 100644 spec/datadog/core/telemetry/logger_spec.rb diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index cd2587514f9..d8623981756 100644 --- a/lib/datadog/appsec/processor.rb +++ b/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 diff --git a/lib/datadog/appsec/processor/rule_loader.rb b/lib/datadog/appsec/processor/rule_loader.rb index 27a76fe1c52..1448ad8ac54 100644 --- a/lib/datadog/appsec/processor/rule_loader.rb +++ b/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 diff --git a/lib/datadog/appsec/processor/rule_merger.rb b/lib/datadog/appsec/processor/rule_merger.rb index a53c7be7ee3..ec334fe8a6e 100644 --- a/lib/datadog/appsec/processor/rule_merger.rb +++ b/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 diff --git a/lib/datadog/core/configuration/components.rb b/lib/datadog/core/configuration/components.rb index 40500c0eda8..3f73a750c7c 100644 --- a/lib/datadog/core/configuration/components.rb +++ b/lib/datadog/core/configuration/components.rb @@ -6,7 +6,6 @@ require_relative '../diagnostics/health' require_relative '../logger' require_relative '../runtime/metrics' -require_relative '../telemetry' require_relative '../telemetry/component' require_relative '../workers/runtime_metrics' diff --git a/lib/datadog/core/telemetry.rb b/lib/datadog/core/telemetry.rb deleted file mode 100644 index 20631d5cc79..00000000000 --- a/lib/datadog/core/telemetry.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module Datadog - module Core - # Telemetry module for sending telemetry events - module Telemetry - class << self - def report(exception, level: :error, description: nil) - instance&.report(exception, level: level, description: description) - end - - def error(description) - instance&.error(description) - end - - private - - def instance - Datadog.send(:components).telemetry - end - end - end - end -end diff --git a/lib/datadog/core/telemetry/logger.rb b/lib/datadog/core/telemetry/logger.rb new file mode 100644 index 00000000000..81ac0ea8f99 --- /dev/null +++ b/lib/datadog/core/telemetry/logger.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Datadog + module Core + module Telemetry + # Module for sending telemetry logs to the global telemetry instance + module Logger + class << self + def report(exception, level: :error, description: nil) + instance&.report(exception, level: level, description: description) + end + + def error(description) + instance&.error(description) + end + + private + + def instance + Datadog.send(:components).telemetry + end + end + end + end + end +end diff --git a/lib/datadog/core/telemetry/logging.rb b/lib/datadog/core/telemetry/logging.rb index 4d8ec75c74d..647cc2bf971 100644 --- a/lib/datadog/core/telemetry/logging.rb +++ b/lib/datadog/core/telemetry/logging.rb @@ -21,7 +21,6 @@ module Telemetry # - What information needed to make it actionable? # module Logging - # Extract datadog stack trace from the exception module DatadogStackTrace GEM_ROOT = Pathname.new("#{__dir__}/../../../..").cleanpath.to_s diff --git a/lib/datadog/profiling/http_transport.rb b/lib/datadog/profiling/http_transport.rb index 49de7f30ef7..7842dc1eee9 100644 --- a/lib/datadog/profiling/http_transport.rb +++ b/lib/datadog/profiling/http_transport.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative "../core/transport/ext" -require_relative "../core/telemetry/logging" +require_relative "../core/telemetry/logger" module Datadog module Profiling @@ -61,14 +61,14 @@ def export(flush) "Failed to report profiling data (#{config_without_api_key}): " \ "server returned unexpected HTTP #{result} status code" ) - Datadog::Core::Telemetry::Logging.error( + Datadog::Core::Telemetry::Logger.error( "Failed to report profiling data: unexpected HTTP #{result} status code" ) false end else Datadog.logger.error("Failed to report profiling data (#{config_without_api_key}): #{result}") - Datadog::Core::Telemetry::Logging.error("Failed to report profiling data") + Datadog::Core::Telemetry::Logger.error("Failed to report profiling data") false end end diff --git a/lib/datadog/profiling/scheduler.rb b/lib/datadog/profiling/scheduler.rb index 1b01fccd41c..8c2ce87e5e7 100644 --- a/lib/datadog/profiling/scheduler.rb +++ b/lib/datadog/profiling/scheduler.rb @@ -4,7 +4,7 @@ require_relative "../core/worker" require_relative "../core/workers/polling" -require_relative "../core/telemetry/logging" +require_relative "../core/telemetry/logger" module Datadog module Profiling @@ -135,7 +135,7 @@ def flush_events Datadog.logger.error( "Unable to report profile. Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}" ) - Datadog::Core::Telemetry::Logging.report(e, level: :error, description: "Unable to report profile") + Datadog::Core::Telemetry::Logger.report(e, level: :error, description: "Unable to report profile") end true diff --git a/lib/datadog/profiling/stack_recorder.rb b/lib/datadog/profiling/stack_recorder.rb index 868bf7b0c38..ceffaaef8fe 100644 --- a/lib/datadog/profiling/stack_recorder.rb +++ b/lib/datadog/profiling/stack_recorder.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "../core/telemetry/logging" +require_relative "../core/telemetry/logger" module Datadog module Profiling @@ -44,7 +44,7 @@ def serialize error_message = result Datadog.logger.error("Failed to serialize profiling data: #{error_message}") - Datadog::Core::Telemetry::Logging.error("Failed to serialize profiling data") + Datadog::Core::Telemetry::Logger.error("Failed to serialize profiling data") nil end diff --git a/sig/datadog/appsec.rbs b/sig/datadog/appsec.rbs index df6659ad690..a39a0a33e3e 100644 --- a/sig/datadog/appsec.rbs +++ b/sig/datadog/appsec.rbs @@ -4,7 +4,11 @@ module Datadog def self.processor: () -> Datadog::AppSec::Processor? - def self.reconfigure: (ruleset: ::Hash[untyped, untyped], actions: Array[Hash[String, untyped]]) -> void + def self.reconfigure: ( + ruleset: ::Hash[untyped, untyped], + actions: Array[Hash[String, untyped]], + telemetry: Datadog::Core::Telemetry::Component + ) -> void def self.reconfigure_lock: () { () -> void } -> void diff --git a/sig/datadog/appsec/component.rbs b/sig/datadog/appsec/component.rbs index 9908d305e32..71409bb4d2c 100644 --- a/sig/datadog/appsec/component.rbs +++ b/sig/datadog/appsec/component.rbs @@ -12,7 +12,11 @@ module Datadog def initialize: (processor: Datadog::AppSec::Processor?) -> void - def self.reconfigure: (ruleset: ::Hash[untyped, untyped], actions: Array[Hash[String, untyped]]) -> void + def self.reconfigure: ( + ruleset: ::Hash[untyped, untyped], + actions: Array[Hash[String, untyped]], + telemetry: Datadog::Core::Telemetry::Component + ) -> void def self.reconfigure_lock: () { () -> void } -> void diff --git a/sig/datadog/core/telemetry/component.rbs b/sig/datadog/core/telemetry/component.rbs index 27f3a830dd7..2d14a670d17 100644 --- a/sig/datadog/core/telemetry/component.rbs +++ b/sig/datadog/core/telemetry/component.rbs @@ -9,6 +9,8 @@ module Datadog attr_reader enabled: bool + include Core::Telemetry::Logging + include Core::Telemetry::_Logging include Core::Utils::Forking def self.build: (untyped settings, Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings, Datadog::Core::Logger logger) -> Component @@ -25,8 +27,6 @@ module Datadog def integrations_change!: () -> void - def log!: (Datadog::Core::Telemetry::Event::Log) -> void - def inc: (String namespace, String metric_name, Datadog::Core::Telemetry::Metric::input_value value, ?tags: Datadog::Core::Telemetry::Metric::tags_input, ?common: bool) -> void def dec: (String namespace, String metric_name, Datadog::Core::Telemetry::Metric::input_value value, ?tags: Datadog::Core::Telemetry::Metric::tags_input, ?common: bool) -> void diff --git a/sig/datadog/core/telemetry/logger.rbs b/sig/datadog/core/telemetry/logger.rbs new file mode 100644 index 00000000000..a8bdb5c1df9 --- /dev/null +++ b/sig/datadog/core/telemetry/logger.rbs @@ -0,0 +1,15 @@ +module Datadog + module Core + module Telemetry + module Logger + def self.report: (Exception exception, level: Symbol, ?description: String?) -> void + + def self.error: (String description) -> void + + private + + def self.instance: () -> Datadog::Core::Telemetry::Component? + end + end + end +end diff --git a/sig/datadog/core/telemetry/logging.rbs b/sig/datadog/core/telemetry/logging.rbs index 75ef035f062..eadd62e40ba 100644 --- a/sig/datadog/core/telemetry/logging.rbs +++ b/sig/datadog/core/telemetry/logging.rbs @@ -1,22 +1,20 @@ module Datadog module Core module Telemetry - module Logging + interface _Logging + def log!: (Datadog::Core::Telemetry::Event::Log) -> void + end + + module Logging : _Logging module DatadogStackTrace GEM_ROOT: String def self.from: (Exception exception) -> String? end - extend Datadog::Core::Telemetry::Logging - def report: (Exception exception, level: Symbol, ?description: String?) -> void def error: (String description) -> void - - private - - def telemetry_component: () -> Datadog::Core::Telemetry::Component end end end diff --git a/sig/datadog/tracing/remote.rbs b/sig/datadog/tracing/remote.rbs index 0c745606125..2366abe48bc 100644 --- a/sig/datadog/tracing/remote.rbs +++ b/sig/datadog/tracing/remote.rbs @@ -13,7 +13,7 @@ module Datadog def self.process_config: (Hash[String, untyped] config, Core::Remote::Configuration::Content content) -> void - def self.receivers: (Datadog::Core::Telemetry::Component OpenTelemetry) -> ::Array[Core::Remote::Dispatcher::Receiver] + def self.receivers: (Datadog::Core::Telemetry::Component) -> ::Array[Core::Remote::Dispatcher::Receiver] def self.receiver: (?::Array[String] products) { (Core::Remote::Configuration::Repository repository, Array[Core::Remote::Configuration::Repository::change] changes) -> void } -> ::Array[Core::Remote::Dispatcher::Receiver] diff --git a/spec/datadog/core/telemetry/logger_spec.rb b/spec/datadog/core/telemetry/logger_spec.rb new file mode 100644 index 00000000000..033a2458b5c --- /dev/null +++ b/spec/datadog/core/telemetry/logger_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +require 'datadog/core/telemetry/logger' + +RSpec.describe Datadog::Core::Telemetry::Logger do + describe '.report' do + context 'when there is a telemetry component configured' do + it do + exception = StandardError.new + telemetry = instance_double(Datadog::Core::Telemetry::Component) + allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) + expect(telemetry).to receive(:report).with(exception, level: :error, description: 'Oops...') + + expect do + described_class.report(exception, level: :error, description: 'Oops...') + end.not_to raise_error + end + + context 'when only given an exception' do + it do + exception = StandardError.new + telemetry = instance_double(Datadog::Core::Telemetry::Component) + allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) + expect(telemetry).to receive(:report).with(exception, level: :error, description: nil) + + expect do + described_class.report(exception) + end.not_to raise_error + end + end + end + + context 'when there is no telemetry component configured' do + it do + exception = StandardError.new + allow(Datadog.send(:components)).to receive(:telemetry).and_return(nil) + + expect do + described_class.report(exception, level: :error, description: 'Oops...') + end.not_to raise_error + end + end + end + + describe '.error' do + context 'when there is a telemetry component configured' do + it do + telemetry = instance_double(Datadog::Core::Telemetry::Component) + allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) + expect(telemetry).to receive(:error).with('description') + + expect { described_class.error('description') }.not_to raise_error + end + end + + context 'when there is no telemetry component configured' do + it do + allow(Datadog.send(:components)).to receive(:telemetry).and_return(nil) + + expect { described_class.error('description') }.not_to raise_error + end + end + end +end diff --git a/spec/datadog/core/telemetry/logging_spec.rb b/spec/datadog/core/telemetry/logging_spec.rb index 577f833cdf8..8f41a25bb3e 100644 --- a/spec/datadog/core/telemetry/logging_spec.rb +++ b/spec/datadog/core/telemetry/logging_spec.rb @@ -1,15 +1,23 @@ require 'spec_helper' require 'datadog/core/telemetry/logging' -require 'datadog/core/telemetry/component' RSpec.describe Datadog::Core::Telemetry::Logging do + let(:dummy_class) do + Class.new do + include Datadog::Core::Telemetry::Logging + def log!(_event) + :logs! + end + end + end + + let(:component) { dummy_class.new } + describe '.report' do context 'with named exception' do it 'sends a log event to via telemetry' do - telemetry = instance_double(Datadog::Core::Telemetry::Component) - allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) - expect(telemetry).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| + expect(component).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| expect(event.payload).to include( logs: [{ message: 'RuntimeError', level: 'ERROR', stack_trace: a_string_including('REDACTED') }] @@ -19,15 +27,13 @@ begin raise 'Invalid token: p@ssw0rd' rescue StandardError => e - described_class.report(e, level: :error) + component.report(e, level: :error) end end context 'with description' do it 'sends a log event to via telemetry' do - telemetry = instance_double(Datadog::Core::Telemetry::Component) - allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) - expect(telemetry).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| + expect(component).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| expect(event.payload).to include( logs: [{ message: 'RuntimeError:Must not contain PII', level: 'ERROR', stack_trace: a_string_including('REDACTED') }] @@ -37,7 +43,7 @@ begin raise 'Invalid token: p@ssw0rd' rescue StandardError => e - described_class.report(e, level: :error, description: 'Must not contain PII') + component.report(e, level: :error, description: 'Must not contain PII') end end end @@ -45,9 +51,7 @@ context 'with anonymous exception' do it 'sends a log event to via telemetry' do - telemetry = instance_double(Datadog::Core::Telemetry::Component) - allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) - expect(telemetry).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| + expect(component).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| expect(event.payload).to include( logs: [{ message: /# e - described_class.report(e, level: :error) - end - end - end - - context 'when telemetry component is not available' do - it 'does not sends a log event to via telemetry' do - logger = Logger.new($stdout) - expect(Datadog.send(:components)).to receive(:telemetry).and_return(nil) - expect(Datadog).to receive(:logger).and_return(logger) - expect(logger).to receive(:debug).with(no_args) do |&block| - expect(block.call).to match(/Attempting to send telemetry log when telemetry component is not ready/) - end - - begin - raise 'Invalid token: p@ssw0rd' - rescue StandardError => e - described_class.report(e, level: :error) + component.report(e, level: :error) end end end @@ -85,26 +72,11 @@ describe '.error' do context 'with description' do it 'sends a log event to via telemetry' do - telemetry = instance_double(Datadog::Core::Telemetry::Component) - allow(Datadog.send(:components)).to receive(:telemetry).and_return(telemetry) - expect(telemetry).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| + expect(component).to receive(:log!).with(instance_of(Datadog::Core::Telemetry::Event::Log)) do |event| expect(event.payload).to include(logs: [{ message: 'Must not contain PII', level: 'ERROR' }]) end - described_class.error('Must not contain PII') - end - end - - context 'when telemetry component is not available' do - it 'does not sends a log event to via telemetry' do - logger = Logger.new($stdout) - expect(Datadog.send(:components)).to receive(:telemetry).and_return(nil) - expect(Datadog).to receive(:logger).and_return(logger) - expect(logger).to receive(:debug).with(no_args) do |&block| - expect(block.call).to match(/Attempting to send telemetry log when telemetry component is not ready/) - end - - described_class.error('Must not contain PII') + component.error('Must not contain PII') end end end diff --git a/spec/datadog/profiling/http_transport_spec.rb b/spec/datadog/profiling/http_transport_spec.rb index 29d60449f2a..dc75554f31a 100644 --- a/spec/datadog/profiling/http_transport_spec.rb +++ b/spec/datadog/profiling/http_transport_spec.rb @@ -234,7 +234,7 @@ end it "sends a telemetry log" do - expect(Datadog::Core::Telemetry::Logging).to receive(:error).with( + expect(Datadog::Core::Telemetry::Logger).to receive(:error).with( "Failed to report profiling data: unexpected HTTP 500 status code" ) @@ -258,7 +258,7 @@ end it "sends a telemetry log" do - expect(Datadog::Core::Telemetry::Logging).to receive(:error).with( + expect(Datadog::Core::Telemetry::Logger).to receive(:error).with( "Failed to report profiling data" ) diff --git a/spec/datadog/profiling/scheduler_spec.rb b/spec/datadog/profiling/scheduler_spec.rb index 1cf3d5e3980..ae609e05707 100644 --- a/spec/datadog/profiling/scheduler_spec.rb +++ b/spec/datadog/profiling/scheduler_spec.rb @@ -187,7 +187,7 @@ it "gracefully handles the exception, logging it" do expect(Datadog.logger).to receive(:error).with(/Kaboom/) - expect(Datadog::Core::Telemetry::Logging).to receive(:report) + expect(Datadog::Core::Telemetry::Logger).to receive(:report) .with(an_instance_of(RuntimeError), level: :error, description: "Unable to report profile") flush_events diff --git a/spec/datadog/profiling/stack_recorder_spec.rb b/spec/datadog/profiling/stack_recorder_spec.rb index 8171af6fe19..07b87203461 100644 --- a/spec/datadog/profiling/stack_recorder_spec.rb +++ b/spec/datadog/profiling/stack_recorder_spec.rb @@ -834,7 +834,7 @@ def create_obj_in_recycled_slot(should_sample_original: false) end it "sends a telemetry log" do - expect(Datadog::Core::Telemetry::Logging).to receive(:error).with("Failed to serialize profiling data") + expect(Datadog::Core::Telemetry::Logger).to receive(:error).with("Failed to serialize profiling data") serialize end From 80356eb00fefe86af710499fc05e713cbccd6cbd Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Thu, 5 Sep 2024 23:10:45 +0200 Subject: [PATCH 3/4] Refactor with default level --- lib/datadog/appsec/processor.rb | 6 +++--- lib/datadog/appsec/processor/rule_loader.rb | 2 +- lib/datadog/appsec/processor/rule_merger.rb | 2 -- lib/datadog/core/telemetry/logging.rb | 2 +- lib/datadog/profiling/scheduler.rb | 2 +- sig/datadog/core/telemetry/logger.rbs | 2 +- sig/datadog/core/telemetry/logging.rbs | 2 +- spec/datadog/appsec/processor/rule_loader_spec.rb | 2 -- spec/datadog/appsec/processor/rule_merger_spec.rb | 2 -- spec/datadog/appsec/processor_spec.rb | 6 ------ spec/datadog/core/telemetry/component_spec.rb | 8 ++++++++ spec/datadog/profiling/scheduler_spec.rb | 2 +- 12 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/datadog/appsec/processor.rb b/lib/datadog/appsec/processor.rb index d8623981756..420e2317057 100644 --- a/lib/datadog/appsec/processor.rb +++ b/lib/datadog/appsec/processor.rb @@ -112,7 +112,7 @@ def require_libddwaf 'libddwaf failed to load,' \ "installed platform: #{libddwaf_platform} ruby platforms: #{ruby_platforms} error: #{e.inspect}" end - @telemetry.report(e, level: :error, description: 'libddwaf failed to load') + @telemetry.report(e, description: 'libddwaf failed to load') false end @@ -140,7 +140,7 @@ def create_waf_handle(settings, ruleset) Datadog.logger.error do "libddwaf failed to initialize, error: #{e.inspect}" end - @telemetry.report(e, level: :error, description: 'libddwaf failed to initialize') + @telemetry.report(e, description: 'libddwaf failed to initialize') @diagnostics = e.diagnostics if e.diagnostics @@ -149,7 +149,7 @@ def create_waf_handle(settings, ruleset) Datadog.logger.error do "libddwaf failed to initialize, error: #{e.inspect}" end - @telemetry.report(e, level: :error, description: 'libddwaf failed to initialize') + @telemetry.report(e, description: 'libddwaf failed to initialize') false end diff --git a/lib/datadog/appsec/processor/rule_loader.rb b/lib/datadog/appsec/processor/rule_loader.rb index 1448ad8ac54..8ca587df75b 100644 --- a/lib/datadog/appsec/processor/rule_loader.rb +++ b/lib/datadog/appsec/processor/rule_loader.rb @@ -35,7 +35,7 @@ def load_rules(ruleset:, telemetry:) "libddwaf ruleset failed to load, ruleset: #{ruleset.inspect} error: #{e.inspect}" end - telemetry.report(e, level: :error, description: 'libddwaf ruleset failed to load') + telemetry.report(e, description: 'libddwaf ruleset failed to load') nil end diff --git a/lib/datadog/appsec/processor/rule_merger.rb b/lib/datadog/appsec/processor/rule_merger.rb index ec334fe8a6e..c9e5b6829d9 100644 --- a/lib/datadog/appsec/processor/rule_merger.rb +++ b/lib/datadog/appsec/processor/rule_merger.rb @@ -31,7 +31,6 @@ def merge( Datadog.logger.error("libddwaf rulemerger failed to parse default waf processors. Error: #{e.inspect}") telemetry.report( e, - level: :error, description: 'libddwaf rulemerger failed to parse default waf processors' ) [] @@ -43,7 +42,6 @@ def merge( Datadog.logger.error("libddwaf rulemerger failed to parse default waf scanners. Error: #{e.inspect}") telemetry.report( e, - level: :error, description: 'libddwaf rulemerger failed to parse default waf scanners' ) [] diff --git a/lib/datadog/core/telemetry/logging.rb b/lib/datadog/core/telemetry/logging.rb index 647cc2bf971..2ddf12bd0b6 100644 --- a/lib/datadog/core/telemetry/logging.rb +++ b/lib/datadog/core/telemetry/logging.rb @@ -45,7 +45,7 @@ def self.from(exception) end end - def report(exception, level:, description: nil) + def report(exception, level: :error, description: nil) # Annoymous exceptions to be logged as message = +'' message << (exception.class.name || exception.class.inspect) diff --git a/lib/datadog/profiling/scheduler.rb b/lib/datadog/profiling/scheduler.rb index 8c2ce87e5e7..0a9cc43d2d5 100644 --- a/lib/datadog/profiling/scheduler.rb +++ b/lib/datadog/profiling/scheduler.rb @@ -135,7 +135,7 @@ def flush_events Datadog.logger.error( "Unable to report profile. Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}" ) - Datadog::Core::Telemetry::Logger.report(e, level: :error, description: "Unable to report profile") + Datadog::Core::Telemetry::Logger.report(e, description: "Unable to report profile") end true diff --git a/sig/datadog/core/telemetry/logger.rbs b/sig/datadog/core/telemetry/logger.rbs index a8bdb5c1df9..b32bf487c1c 100644 --- a/sig/datadog/core/telemetry/logger.rbs +++ b/sig/datadog/core/telemetry/logger.rbs @@ -2,7 +2,7 @@ module Datadog module Core module Telemetry module Logger - def self.report: (Exception exception, level: Symbol, ?description: String?) -> void + def self.report: (Exception exception, ?level: Symbol, ?description: String?) -> void def self.error: (String description) -> void diff --git a/sig/datadog/core/telemetry/logging.rbs b/sig/datadog/core/telemetry/logging.rbs index eadd62e40ba..f64aa023f3c 100644 --- a/sig/datadog/core/telemetry/logging.rbs +++ b/sig/datadog/core/telemetry/logging.rbs @@ -12,7 +12,7 @@ module Datadog def self.from: (Exception exception) -> String? end - def report: (Exception exception, level: Symbol, ?description: String?) -> void + def report: (Exception exception, ?level: Symbol, ?description: String?) -> void def error: (String description) -> void end diff --git a/spec/datadog/appsec/processor/rule_loader_spec.rb b/spec/datadog/appsec/processor/rule_loader_spec.rb index 2ba437698ff..4b0bec8a041 100644 --- a/spec/datadog/appsec/processor/rule_loader_spec.rb +++ b/spec/datadog/appsec/processor/rule_loader_spec.rb @@ -63,7 +63,6 @@ it 'returns `nil`' do expect(telemetry).to receive(:report).with( an_instance_of(Errno::ENOENT), - level: :error, description: 'libddwaf ruleset failed to load' ) @@ -95,7 +94,6 @@ it 'returns `nil`' do expect(telemetry).to receive(:report).with( an_instance_of(JSON::ParserError), - level: :error, description: 'libddwaf ruleset failed to load' ) diff --git a/spec/datadog/appsec/processor/rule_merger_spec.rb b/spec/datadog/appsec/processor/rule_merger_spec.rb index b233aa20835..5a73dd3c424 100644 --- a/spec/datadog/appsec/processor/rule_merger_spec.rb +++ b/spec/datadog/appsec/processor/rule_merger_spec.rb @@ -719,7 +719,6 @@ expect(Datadog.logger).to receive(:error).with(/libddwaf rulemerger failed to parse default waf processors/) expect(telemetry).to receive(:report).with( a_kind_of(StandardError), - level: :error, description: 'libddwaf rulemerger failed to parse default waf processors' ) @@ -749,7 +748,6 @@ expect(Datadog.logger).to receive(:error).with(/libddwaf rulemerger failed to parse default waf scanners/) expect(telemetry).to receive(:report).with( a_kind_of(StandardError), - level: :error, description: 'libddwaf rulemerger failed to parse default waf scanners' ) diff --git a/spec/datadog/appsec/processor_spec.rb b/spec/datadog/appsec/processor_spec.rb index 832f2df3f50..107eb5c6a87 100644 --- a/spec/datadog/appsec/processor_spec.rb +++ b/spec/datadog/appsec/processor_spec.rb @@ -47,7 +47,6 @@ allow_any_instance_of(described_class).to receive(:require).with('libddwaf').and_raise(LoadError) expect(telemetry).to receive(:report).with( an_instance_of(LoadError), - level: :error, description: 'libddwaf failed to load' ).at_least(:once) @@ -152,7 +151,6 @@ def required_addresses ) expect(telemetry).to receive(:report).with( a_kind_of(Datadog::AppSec::WAF::LibDDWAF::Error), - level: :error, description: 'libddwaf failed to initialize' ) @@ -190,7 +188,6 @@ def required_addresses ) expect(telemetry).to receive(:report).with( a_kind_of(StandardError), - level: :error, description: 'libddwaf failed to initialize' ) @@ -239,7 +236,6 @@ def required_addresses expect(Datadog.logger).to receive(:warn) expect(telemetry).to receive(:report).with( a_kind_of(StandardError), - level: :error, description: 'libddwaf failed to initialize' ) @@ -258,7 +254,6 @@ def required_addresses expect(Datadog::AppSec::WAF::Handle).to receive(:new).and_raise(StandardError) expect(telemetry).to receive(:report).with( an_instance_of(StandardError), - level: :error, description: 'libddwaf failed to initialize' ) @@ -282,7 +277,6 @@ def diagnostics expect(Datadog::AppSec::WAF::Handle).to receive(:new).and_raise(Datadog::AppSec::WAF::LibDDWAF::Error) expect(telemetry).to receive(:report).with( an_instance_of(Datadog::AppSec::WAF::LibDDWAF::Error), - level: :error, description: 'libddwaf failed to initialize' ) diff --git a/spec/datadog/core/telemetry/component_spec.rb b/spec/datadog/core/telemetry/component_spec.rb index 1eb81e98852..f5c2852a268 100644 --- a/spec/datadog/core/telemetry/component_spec.rb +++ b/spec/datadog/core/telemetry/component_spec.rb @@ -219,6 +219,14 @@ end end + describe 'includes Datadog::Core::Telemetry::Logging' do + after do + telemetry.stop! + end + + it { is_expected.to a_kind_of(Datadog::Core::Telemetry::Logging) } + end + describe '#log!' do after do telemetry.stop! diff --git a/spec/datadog/profiling/scheduler_spec.rb b/spec/datadog/profiling/scheduler_spec.rb index ae609e05707..0478d8a3a6b 100644 --- a/spec/datadog/profiling/scheduler_spec.rb +++ b/spec/datadog/profiling/scheduler_spec.rb @@ -188,7 +188,7 @@ it "gracefully handles the exception, logging it" do expect(Datadog.logger).to receive(:error).with(/Kaboom/) expect(Datadog::Core::Telemetry::Logger).to receive(:report) - .with(an_instance_of(RuntimeError), level: :error, description: "Unable to report profile") + .with(an_instance_of(RuntimeError), description: "Unable to report profile") flush_events end From 9868ed2c31c6522bfe4cd2ed1b493613fa045015 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Fri, 6 Sep 2024 10:14:02 +0200 Subject: [PATCH 4/4] Add comments about deadlock risk --- lib/datadog/core/telemetry/logger.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/datadog/core/telemetry/logger.rb b/lib/datadog/core/telemetry/logger.rb index 81ac0ea8f99..b18650ebd5e 100644 --- a/lib/datadog/core/telemetry/logger.rb +++ b/lib/datadog/core/telemetry/logger.rb @@ -3,7 +3,15 @@ module Datadog module Core module Telemetry - # Module for sending telemetry logs to the global telemetry instance + # === INTRENAL USAGE ONLY === + # + # Report telemetry logs via delegating to the telemetry component instance via mutex. + # + # IMPORTANT: Invoking this method during the lifecycle of component initialization will + # cause a non-recoverable deadlock + # + # For developer using this module: + # read: lib/datadog/core/telemetry/logging.rb module Logger class << self def report(exception, level: :error, description: nil)