diff --git a/lib/datadog/core/configuration.rb b/lib/datadog/core/configuration.rb index bcaa643256..19d3ff2711 100644 --- a/lib/datadog/core/configuration.rb +++ b/lib/datadog/core/configuration.rb @@ -84,23 +84,16 @@ def configure configuration = self.configuration yield(configuration) - built_components = false - - components = safely_synchronize do |write_components| + safely_synchronize do |write_components| write_components.call( if components? replace_components!(configuration, @components) else - components = build_components(configuration) - built_components = true - components + build_components(configuration) end ) end - # Should only be called the first time components are built - components.telemetry.started! if built_components - configuration end @@ -200,20 +193,13 @@ def components(allow_initialization: true) current_components = COMPONENTS_READ_LOCK.synchronize { defined?(@components) && @components } return current_components if current_components || !allow_initialization - built_components = false - - components = safely_synchronize do |write_components| + safely_synchronize do |write_components| if defined?(@components) && @components @components else - built_components = true write_components.call(build_components(configuration)) end end - - # Should only be called the first time components are built - components&.telemetry&.started! if built_components - components end private diff --git a/lib/datadog/core/telemetry/component.rb b/lib/datadog/core/telemetry/component.rb index f457a434ce..0d5046e439 100644 --- a/lib/datadog/core/telemetry/component.rb +++ b/lib/datadog/core/telemetry/component.rb @@ -20,13 +20,12 @@ class Component def initialize(heartbeat_interval_seconds:, dependency_collection:, enabled: true) @enabled = enabled @stopped = false - @started = false - @dependency_collection = dependency_collection @worker = Telemetry::Worker.new( enabled: @enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, - emitter: Emitter.new + emitter: Emitter.new, + dependency_collection: dependency_collection ) @worker.start end @@ -36,14 +35,6 @@ def disable! @worker.enabled = false end - def started! - return if !@enabled || forked? - - @worker.enqueue(Event::AppDependenciesLoaded.new) if @dependency_collection - - @started = true - end - def stop! return if @stopped diff --git a/lib/datadog/core/telemetry/worker.rb b/lib/datadog/core/telemetry/worker.rb index 8ba8e8723b..f2bd7821b4 100644 --- a/lib/datadog/core/telemetry/worker.rb +++ b/lib/datadog/core/telemetry/worker.rb @@ -21,11 +21,13 @@ class Worker def initialize( heartbeat_interval_seconds:, emitter:, + dependency_collection:, enabled: true, shutdown_timeout: Workers::Polling::DEFAULT_SHUTDOWN_TIMEOUT, buffer_size: DEFAULT_BUFFER_MAX_SIZE ) @emitter = emitter + @dependency_collection = dependency_collection # Workers::Polling settings self.enabled = enabled @@ -97,6 +99,9 @@ def started! if res.ok? Datadog.logger.debug('Telemetry app-started event is successfully sent') + + enqueue(Event::AppDependenciesLoaded.new) if @dependency_collection + true else Datadog.logger.debug('Error sending telemetry app-started event, retry after heartbeat interval...') diff --git a/sig/datadog/core/telemetry/component.rbs b/sig/datadog/core/telemetry/component.rbs index 614ac20d69..4d411d3257 100644 --- a/sig/datadog/core/telemetry/component.rbs +++ b/sig/datadog/core/telemetry/component.rbs @@ -3,8 +3,6 @@ module Datadog module Telemetry class Component @enabled: bool - @dependency_collection: bool - @started: bool @stopped: bool @worker: Datadog::Core::Telemetry::Worker @@ -18,8 +16,6 @@ module Datadog def client_configuration_change!: (Enumerable[[String, Numeric | bool | String]] changes) -> void - def started!: () -> void - def emit_closing!: () -> void def stop!: () -> void diff --git a/sig/datadog/core/telemetry/worker.rbs b/sig/datadog/core/telemetry/worker.rbs index b804a19664..01c5107e99 100644 --- a/sig/datadog/core/telemetry/worker.rbs +++ b/sig/datadog/core/telemetry/worker.rbs @@ -15,8 +15,9 @@ module Datadog @sent_started_event: bool @shutdown_timeout: Integer @buffer_size: Integer + @dependency_collection: bool - def initialize: (?enabled: bool, heartbeat_interval_seconds: Numeric, emitter: Emitter, ?shutdown_timeout: Integer, ?buffer_size: Integer) -> void + def initialize: (?enabled: bool, heartbeat_interval_seconds: Numeric, emitter: Emitter, ?shutdown_timeout: Integer, ?buffer_size: Integer, dependency_collection: bool) -> void def start: () -> void diff --git a/spec/datadog/core/configuration_spec.rb b/spec/datadog/core/configuration_spec.rb index ace1b5066b..51a2b44cb0 100644 --- a/spec/datadog/core/configuration_spec.rb +++ b/spec/datadog/core/configuration_spec.rb @@ -11,7 +11,6 @@ let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) } before do - allow(telemetry).to receive(:started!) allow(telemetry).to receive(:stop!) allow(telemetry).to receive(:emit_closing!) allow(Datadog::Core::Telemetry::Component).to receive(:new).and_return(telemetry) @@ -41,10 +40,6 @@ end it do - # We cannot mix `expect().to_not` with `expect().to(...).ordered`. - # One way around that is to force the method to raise an error if it's ever called. - allow(telemetry).to receive(:started!).and_raise('Should not be called') - # Components should have changed expect { configure } .to change { test_class.send(:components) } @@ -84,7 +79,6 @@ .with(test_class.configuration) expect(new_components).to_not have_received(:shutdown!) - expect(telemetry).to have_received(:started!) end end end @@ -501,8 +495,6 @@ describe '#components' do context 'when components are not initialized' do it 'initializes the components' do - expect(telemetry).to receive(:started!) - test_class.send(:components) expect(test_class.send(:components?)).to be true @@ -510,8 +502,6 @@ context 'when allow_initialization is false' do it 'does not initialize the components' do - expect(telemetry).to_not receive(:started!) - test_class.send(:components, allow_initialization: false) expect(test_class.send(:components?)).to be false @@ -527,7 +517,6 @@ it 'returns the components without touching the COMPONENTS_WRITE_LOCK' do described_class.const_get(:COMPONENTS_WRITE_LOCK).lock - expect(telemetry).to_not receive(:started!) expect(test_class.send(:components)).to_not be_nil end end diff --git a/spec/datadog/core/telemetry/component_spec.rb b/spec/datadog/core/telemetry/component_spec.rb index 33f4544ec6..ba17d37c2f 100644 --- a/spec/datadog/core/telemetry/component_spec.rb +++ b/spec/datadog/core/telemetry/component_spec.rb @@ -18,7 +18,13 @@ let(:not_found) { false } before do - allow(Datadog::Core::Telemetry::Worker).to receive(:new).and_return(worker) + allow(Datadog::Core::Telemetry::Worker).to receive(:new).with( + heartbeat_interval_seconds: heartbeat_interval_seconds, + dependency_collection: dependency_collection, + enabled: enabled, + emitter: an_instance_of(Datadog::Core::Telemetry::Emitter) + ).and_return(worker) + allow(worker).to receive(:start) allow(worker).to receive(:enqueue) allow(worker).to receive(:stop) @@ -70,60 +76,6 @@ end end - describe '#started!' do - subject(:started!) { telemetry.started! } - - after do - telemetry.stop! - end - - context 'when disabled' do - let(:enabled) { false } - it do - started! - - expect(worker).to_not have_received(:enqueue) - end - end - - context 'when enabled' do - let(:enabled) { true } - - context 'when dependency_collection is true' do - it do - started! - - expect(worker).to have_received(:enqueue).with( - an_instance_of(Datadog::Core::Telemetry::Event::AppDependenciesLoaded) - ) - end - end - - context 'when dependency_collection is false' do - let(:dependency_collection) { false } - - it do - started! - - expect(worker).not_to have_received(:enqueue) - end - end - end - - context 'when in fork' do - before { skip 'Fork not supported on current platform' unless Process.respond_to?(:fork) } - - it do - telemetry - expect_in_fork do - telemetry.started! - - expect(worker).to_not have_received(:enqueue) - end - end - end - end - describe '#emit_closing!' do subject(:emit_closing!) { telemetry.emit_closing! } @@ -157,8 +109,6 @@ it do telemetry expect_in_fork do - telemetry.started! - expect(worker).not_to have_received(:enqueue) end end @@ -209,8 +159,6 @@ it do telemetry expect_in_fork do - telemetry.started! - expect(worker).not_to have_received(:enqueue) end end @@ -251,8 +199,6 @@ it do telemetry expect_in_fork do - telemetry.started! - expect(worker).not_to have_received(:enqueue) end end diff --git a/spec/datadog/core/telemetry/worker_spec.rb b/spec/datadog/core/telemetry/worker_spec.rb index 6008cefd89..6a1d5ca055 100644 --- a/spec/datadog/core/telemetry/worker_spec.rb +++ b/spec/datadog/core/telemetry/worker_spec.rb @@ -4,12 +4,18 @@ RSpec.describe Datadog::Core::Telemetry::Worker do subject(:worker) do - described_class.new(enabled: enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, emitter: emitter) + described_class.new( + enabled: enabled, + heartbeat_interval_seconds: heartbeat_interval_seconds, + emitter: emitter, + dependency_collection: dependency_collection + ) end let(:enabled) { true } let(:heartbeat_interval_seconds) { 0.5 } let(:emitter) { double(Datadog::Core::Telemetry::Emitter) } + let(:dependency_collection) { false } let(:backend_supports_telemetry?) { true } let(:response) do @@ -113,6 +119,26 @@ try_wait_until { sent_hearbeat } end + + context 'when dependencies collection enabled' do + let(:dependency_collection) { true } + + it 'sends dependencies loaded event after started event' do + sent_dependencies = false + allow(emitter).to receive(:request).with(kind_of(Datadog::Core::Telemetry::Event::AppDependenciesLoaded)) do + # app-started was already sent by now + expect(worker.sent_started_event?).to be(true) + + sent_dependencies = true + + response + end + + worker.start + + try_wait_until { sent_dependencies } + end + end end context 'when internal error returned by emitter' do @@ -147,7 +173,8 @@ described_class.new( enabled: enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, - emitter: emitter + emitter: emitter, + dependency_collection: dependency_collection ) end workers.each(&:start)