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

[PROF-4081] Make threads with empty backtrace visible #1719

Merged
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
21 changes: 21 additions & 0 deletions lib/ddtrace/profiling/collectors/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class Stack < Worker # rubocop:disable Metrics/ClassLength
MIN_INTERVAL = 0.01
THREAD_LAST_CPU_TIME_KEY = :datadog_profiler_last_cpu_time
THREAD_LAST_WALL_CLOCK_KEY = :datadog_profiler_last_wall_clock
SYNTHETIC_STACK_IN_NATIVE_CODE = [BacktraceLocation.new('', 0, 'In native code').freeze].freeze

# This default was picked based on the current sampling performance and on expected concurrency on an average
# Ruby MRI application. Lowering this optimizes for latency (less impact each time we sample), and raising
Expand Down Expand Up @@ -119,6 +120,26 @@ def collect_thread_event(thread, current_wall_time_ns)
locations = thread.backtrace_locations
return if locations.nil?

# Having empty locations means that the thread is alive, but we don't know what it's doing:
#
# 1. It can be starting up
# ```
# > Thread.new { sleep }.backtrace
# => [] # <-- note the thread hasn't actually started running sleep yet, we got there first
# ```
# 2. It can be running native code
# ```
# > t = Process.detach(fork { sleep })
# => #<Process::Waiter:0x00007ffe7285f7a0 run>
# > t.backtrace
# => [] # <-- this can happen even minutes later, e.g. it's not a race as in 1.
# ```
# This effect has been observed in threads created by the Iodine web server and the ffi gem
#
# To give customers visibility into these threads, we replace the empty stack with one containing a
# synthetic placeholder frame, so that these threads are properly represented in the UX.
locations = SYNTHETIC_STACK_IN_NATIVE_CODE if locations.empty?

# Get actual stack size then trim the stack
stack_size = locations.length
locations = locations[0..(max_frames - 1)]
Expand Down
18 changes: 16 additions & 2 deletions spec/ddtrace/profiling/collectors/stack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,13 @@
let(:last_wall_time) { 42 }
let(:current_wall_time) { 123 }

context 'when the backtrace is empty' do
context 'when the backtrace is nil' do
let(:backtrace) { nil }

it { is_expected.to be nil }
end

context 'when the backtrace is not empty' do
context 'when the backtrace is not nil' do
let(:backtrace) do
Array.new(backtrace_size) do
instance_double(
Expand Down Expand Up @@ -470,6 +470,20 @@
end
end
end

context 'when the backtrace is empty' do
let(:backtrace) { [] }

it 'builds an event that includes a includes a synthetic placeholder frame to mark execution in native code' do
is_expected.to have_attributes(
total_frame_count: 1,
frames: [Datadog::Profiling::BacktraceLocation.new('', 0, 'In native code')],
timestamp: kind_of(Float),
thread_id: thread.object_id,
wall_time_interval_ns: current_wall_time - last_wall_time,
)
end
end
end

context 'Process::Waiter crash regression tests' do
Expand Down