Skip to content

Commit

Permalink
Merge pull request #3910 from DataDog/tonycthsu/avoid-deadlock
Browse files Browse the repository at this point in the history
Prevent telemetry deadlock with no-op
  • Loading branch information
TonyCTHsu authored Sep 13, 2024
2 parents c9994df + f64ae44 commit 3860fa7
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 4 deletions.
23 changes: 20 additions & 3 deletions lib/datadog/core/telemetry/logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
module Datadog
module Core
module Telemetry
# === INTRENAL USAGE ONLY ===
# === INTERNAL 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
# be no-op instead.
#
# For developer using this module:
# read: lib/datadog/core/telemetry/logging.rb
Expand All @@ -25,7 +25,24 @@ def error(description)
private

def instance
Datadog.send(:components).telemetry
# Component initialization uses a mutex to avoid having concurrent initialization.
# Trying to access the telemetry component during initialization (specifically:
# from the thread that's actually doing the initialization) would cause a deadlock,
# since accessing the components would try to recursively lock the mutex.
#
# To work around this, we use allow_initialization: false to avoid triggering this issue.
#
# The downside is: this leaves us unable to report telemetry during component initialization.
components = Datadog.send(:components, allow_initialization: false)

if components && components.telemetry
components.telemetry
else
Datadog.logger.warn(
'Fail to send telemetry log before components initialization or within components lifecycle'
)
nil
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/core/telemetry/logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
module Datadog
module Core
module Telemetry
# === INTRENAL USAGE ONLY ===
# === INTERNAL USAGE ONLY ===
#
# Logging interface for sending telemetry logs... so we can fix them.
#
Expand Down
5 changes: 5 additions & 0 deletions spec/datadog/core/telemetry/logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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(Datadog.logger).not_to receive(:warn)

expect do
described_class.report(exception, level: :error, description: 'Oops...')
Expand All @@ -22,6 +23,7 @@
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(Datadog.logger).not_to receive(:warn)

expect do
described_class.report(exception)
Expand All @@ -34,6 +36,7 @@
it do
exception = StandardError.new
allow(Datadog.send(:components)).to receive(:telemetry).and_return(nil)
expect(Datadog.logger).to receive(:warn).with(/Fail to send telemetry log/)

expect do
described_class.report(exception, level: :error, description: 'Oops...')
Expand All @@ -48,6 +51,7 @@
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(Datadog.logger).not_to receive(:warn)

expect { described_class.error('description') }.not_to raise_error
end
Expand All @@ -56,6 +60,7 @@
context 'when there is no telemetry component configured' do
it do
allow(Datadog.send(:components)).to receive(:telemetry).and_return(nil)
expect(Datadog.logger).to receive(:warn).with(/Fail to send telemetry log/)

expect { described_class.error('description') }.not_to raise_error
end
Expand Down

0 comments on commit 3860fa7

Please sign in to comment.