Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock when sending telemetry logs #3886

Merged
merged 4 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/datadog/appsec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 11 additions & 7 deletions lib/datadog/appsec/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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']
Expand All @@ -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
Expand All @@ -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
Expand Down
78 changes: 36 additions & 42 deletions lib/datadog/appsec/processor.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# frozen_string_literal: true

require_relative '../core/telemetry/logging'

module Datadog
module AppSec
# Processor integrates libddwaf into datadog/appsec
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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
Expand Down
5 changes: 2 additions & 3 deletions lib/datadog/appsec/processor/rule_loader.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require_relative '../assets'
require_relative '../../core/telemetry/logging'

module Datadog
module AppSec
Expand All @@ -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
Expand All @@ -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
Expand Down
43 changes: 24 additions & 19 deletions lib/datadog/appsec/processor/rule_merger.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require_relative '../assets'
require_relative '../../core/telemetry/logging'

module Datadog
module AppSec
Expand All @@ -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?
Expand All @@ -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
Expand Down
10 changes: 7 additions & 3 deletions lib/datadog/appsec/remote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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]
Expand Down
7 changes: 4 additions & 3 deletions lib/datadog/core/configuration/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions lib/datadog/core/remote/client/capabilities.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/core/remote/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/datadog/core/telemetry/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand Down
Loading
Loading