Skip to content

Commit

Permalink
remove Telemetry::Component.started! as right now it just contains de…
Browse files Browse the repository at this point in the history
…pendency collection event logic, move it to the Worker
  • Loading branch information
anmarchenko committed Jun 17, 2024
1 parent 2457491 commit aaa8bbc
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 107 deletions.
20 changes: 3 additions & 17 deletions lib/datadog/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

Expand Down
5 changes: 5 additions & 0 deletions lib/datadog/core/telemetry/worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...')
Expand Down
4 changes: 0 additions & 4 deletions sig/datadog/core/telemetry/component.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ module Datadog
module Telemetry
class Component
@enabled: bool
@dependency_collection: bool
@started: bool
@stopped: bool
@worker: Datadog::Core::Telemetry::Worker

Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion sig/datadog/core/telemetry/worker.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 0 additions & 11 deletions spec/datadog/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) }
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -501,17 +495,13 @@
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
end

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
Expand All @@ -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
Expand Down
68 changes: 7 additions & 61 deletions spec/datadog/core/telemetry/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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! }

Expand Down Expand Up @@ -157,8 +109,6 @@
it do
telemetry
expect_in_fork do
telemetry.started!

expect(worker).not_to have_received(:enqueue)
end
end
Expand Down Expand Up @@ -209,8 +159,6 @@
it do
telemetry
expect_in_fork do
telemetry.started!

expect(worker).not_to have_received(:enqueue)
end
end
Expand Down Expand Up @@ -251,8 +199,6 @@
it do
telemetry
expect_in_fork do
telemetry.started!

expect(worker).not_to have_received(:enqueue)
end
end
Expand Down
31 changes: 29 additions & 2 deletions spec/datadog/core/telemetry/worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit aaa8bbc

Please sign in to comment.