From d51de9dd147c09dec4fc2c12801f53f3b9e9c5ce Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Wed, 22 Jan 2025 12:40:08 +0100 Subject: [PATCH] Make telemetry an AppSec component service --- lib/datadog/appsec.rb | 10 ++-- lib/datadog/appsec/component.rb | 21 +++++---- sig/datadog/appsec.rbs | 15 +++--- sig/datadog/appsec/component.rbs | 31 ++++++++----- spec/datadog/appsec/component_spec.rb | 67 ++++++++++++++++----------- 5 files changed, 84 insertions(+), 60 deletions(-) diff --git a/lib/datadog/appsec.rb b/lib/datadog/appsec.rb index 72ef8c1cfc0..0c6218fc72f 100644 --- a/lib/datadog/appsec.rb +++ b/lib/datadog/appsec.rb @@ -18,15 +18,16 @@ def active_context Datadog::AppSec::Context.active end - def processor - appsec_component = components.appsec + def telemetry + components.appsec&.telemetry + end - appsec_component.processor if appsec_component + def processor + components.appsec&.processor end def reconfigure(ruleset:, telemetry:) appsec_component = components.appsec - return unless appsec_component appsec_component.reconfigure(ruleset: ruleset, telemetry: telemetry) @@ -34,7 +35,6 @@ def reconfigure(ruleset:, telemetry:) def reconfigure_lock(&block) appsec_component = components.appsec - return unless appsec_component appsec_component.reconfigure_lock(&block) diff --git a/lib/datadog/appsec/component.rb b/lib/datadog/appsec/component.rb index 7951989e0fa..7a769f38fde 100644 --- a/lib/datadog/appsec/component.rb +++ b/lib/datadog/appsec/component.rb @@ -24,7 +24,7 @@ def build_appsec_component(settings, telemetry:) devise_integration = Datadog::AppSec::Contrib::Devise::Integration.new settings.appsec.instrument(:devise) unless devise_integration.patcher.patched? - new(processor: processor) + new(processor, telemetry) end private @@ -73,21 +73,26 @@ def create_processor(settings, telemetry) end end - attr_reader :processor + attr_reader :processor, :telemetry - def initialize(processor:) + def initialize(processor, telemetry) @processor = processor + @telemetry = telemetry + @mutex = Mutex.new end def reconfigure(ruleset:, telemetry:) @mutex.synchronize do - new = Processor.new(ruleset: ruleset, telemetry: telemetry) + new_processor = Processor.new(ruleset: ruleset, telemetry: telemetry) + + if new_processor && new_processor.ready? + old_processor = @processor + + @telemetry = telemetry + @processor = new_processor - if new && new.ready? - old = @processor - @processor = new - old.finalize if old + old_processor.finalize if old_processor end end end diff --git a/sig/datadog/appsec.rbs b/sig/datadog/appsec.rbs index 97b6274eaf4..b3a71cfb34a 100644 --- a/sig/datadog/appsec.rbs +++ b/sig/datadog/appsec.rbs @@ -2,19 +2,20 @@ module Datadog module AppSec def self.enabled?: () -> bool - def self.processor: () -> Datadog::AppSec::Processor? + def self.active_context: () -> Context? - def self.reconfigure: ( - ruleset: ::Hash[untyped, untyped], - telemetry: Datadog::Core::Telemetry::Component - ) -> void + def self.telemetry: () -> Core::Telemetry::Component - def self.reconfigure_lock: () { () -> void } -> void + def self.processor: () -> Processor? + + def self.reconfigure: (ruleset: Processor::RuleMerger::rules, telemetry: Core::Telemetry::Component) -> void + + def self.reconfigure_lock: () { (?) -> untyped } -> void def self.api_security_enabled?: () -> bool private - def self.components: () -> Datadog::Core::Configuration::Components + def self.components: () -> Core::Configuration::Components end end diff --git a/sig/datadog/appsec/component.rbs b/sig/datadog/appsec/component.rbs index 71409bb4d2c..ff69368678f 100644 --- a/sig/datadog/appsec/component.rbs +++ b/sig/datadog/appsec/component.rbs @@ -1,26 +1,33 @@ module Datadog module AppSec class Component - def self.build_appsec_component: (Datadog::Core::Configuration::Settings settings) -> Datadog::AppSec::Component? + @processor: Processor? + + @telemetry: Core::Telemetry::Component + + @mutex: ::Mutex + + def self.build_appsec_component: (Core::Configuration::Settings settings, Core::Telemetry::Component telemetry) -> Component? private - def self.create_processor: () -> Datadog::AppSec::Processor? + def self.incompatible_ffi_version?: () -> bool + + def self.create_processor: (Core::Configuration::Settings settings, Core::Telemetry::Component telemetry) -> Processor? + + public + + attr_reader processor: Processor? - attr_reader processor: Datadog::AppSec::Processor? - attr_reader mutex: Thread::Mutex + attr_reader telemetry: Core::Telemetry::Component - def initialize: (processor: Datadog::AppSec::Processor?) -> void + def initialize: (Processor processor, Core::Telemetry::Component telemetry) -> void - def self.reconfigure: ( - ruleset: ::Hash[untyped, untyped], - actions: Array[Hash[String, untyped]], - telemetry: Datadog::Core::Telemetry::Component - ) -> void + def reconfigure: (ruleset: Processor::RuleMerger::rules, telemetry: Core::Telemetry::Component) -> void - def self.reconfigure_lock: () { () -> void } -> void + def reconfigure_lock: () { (?) -> void } -> void - def shutdown!: () -> untyped + def shutdown!: () -> void end end end diff --git a/spec/datadog/appsec/component_spec.rb b/spec/datadog/appsec/component_spec.rb index 78167642aa2..b5f5c7e1360 100644 --- a/spec/datadog/appsec/component_spec.rb +++ b/spec/datadog/appsec/component_spec.rb @@ -2,6 +2,8 @@ require 'datadog/appsec/component' RSpec.describe Datadog::AppSec::Component do + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + describe '.build_appsec_component' do let(:settings) do settings = Datadog::Core::Configuration::Settings.new @@ -9,8 +11,6 @@ settings end - let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } - context 'when appsec is enabled' do let(:appsec_enabled) { true } @@ -34,9 +34,7 @@ end context 'when ffi is not loaded' do - before do - allow(Gem).to receive(:loaded_specs).and_return({}) - end + before { allow(Gem).to receive(:loaded_specs).and_return({}) } it 'returns a Datadog::AppSec::Component instance with a nil processor and does not warn' do expect(Datadog.logger).not_to receive(:warn) @@ -96,7 +94,9 @@ end describe '#reconfigure' do - let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component, report: nil) } + before { allow(telemetry).to receive(:report) } + + let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } let(:ruleset) do { 'exclusions' => [{ @@ -144,7 +144,7 @@ it 'makes sure to synchronize' do mutex = Mutex.new processor = instance_double(Datadog::AppSec::Processor) - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) component.instance_variable_set(:@mutex, mutex) expect(mutex).to receive(:synchronize) component.reconfigure(ruleset: {}, telemetry: telemetry) @@ -152,24 +152,29 @@ end context 'when the new processor is ready' do + let(:processor) { instance_double(Datadog::AppSec::Processor) } + let(:new_telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + it 'swaps the processor instance and finalize the old processor' do - processor = instance_double(Datadog::AppSec::Processor) - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) - old_processor = component.processor + expect(component.processor).to eq(processor) + expect(component.telemetry).to eq(telemetry) + expect(component.processor).to receive(:finalize) - expect(old_processor).to receive(:finalize) - component.reconfigure(ruleset: ruleset, telemetry: telemetry) - new_processor = component.processor - expect(new_processor).to_not eq(old_processor) - new_processor.finalize + component.reconfigure(ruleset: ruleset, telemetry: new_telemetry) + + expect(component.processor).to_not eq(processor) + expect(component.telemetry).to eq(new_telemetry) + + component.processor.finalize end end context 'when the new processor is ready, and old processor is nil' do it 'swaps the processor instance and do not finalize the old processor' do processor = nil - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) old_processor = component.processor @@ -182,17 +187,23 @@ end context 'when the new processor is not ready' do - it 'does not swap the processor instance and finalize the old processor' do - processor = instance_double(Datadog::AppSec::Processor) - component = described_class.new(processor: processor) + before { allow(new_telemetry).to receive(:report) } - old_processor = component.processor + let(:processor) { instance_double(Datadog::AppSec::Processor) } + let(:new_telemetry) { instance_double(Datadog::Core::Telemetry::Component) } + it 'does not swap the processor instance and finalize the old processor' do + component = described_class.new(processor, telemetry) ruleset = { 'invalid_one' => true } - expect(old_processor).to_not receive(:finalize) - component.reconfigure(ruleset: ruleset, telemetry: telemetry) - expect(component.processor).to eq(old_processor) + expect(processor).to_not receive(:finalize) + expect(component.processor).to eq(processor) + expect(component.telemetry).to eq(telemetry) + + component.reconfigure(ruleset: ruleset, telemetry: new_telemetry) + + expect(component.processor).to eq(processor) + expect(component.telemetry).to eq(telemetry) end end end @@ -202,7 +213,7 @@ it 'makes sure to synchronize' do mutex = Mutex.new processor = instance_double(Datadog::AppSec::Processor) - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) component.instance_variable_set(:@mutex, mutex) expect(mutex).to receive(:synchronize) component.reconfigure_lock(&proc {}) @@ -215,7 +226,7 @@ it 'makes sure to synchronize' do mutex = Mutex.new processor = instance_double(Datadog::AppSec::Processor) - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) component.instance_variable_set(:@mutex, mutex) expect(mutex).to receive(:synchronize) component.shutdown! @@ -226,7 +237,7 @@ it 'finalizes the processor' do processor = instance_double(Datadog::AppSec::Processor) - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) expect(processor).to receive(:ready?).and_return(true) expect(processor).to receive(:finalize) component.shutdown! @@ -238,7 +249,7 @@ processor = instance_double(Datadog::AppSec::Processor) expect(processor).to receive(:ready?).and_return(false) - component = described_class.new(processor: processor) + component = described_class.new(processor, telemetry) expect(processor).to_not receive(:finalize) component.shutdown! @@ -247,7 +258,7 @@ context 'when processor is nil' do it 'does not finalize the processor' do - component = described_class.new(processor: nil) + component = described_class.new(nil, telemetry) expect_any_instance_of(Datadog::AppSec::Processor).to_not receive(:finalize) component.shutdown!