Skip to content

Commit

Permalink
Merge pull request #4031 from DataDog/ivoanjo/fix-crashtracking-pid-r…
Browse files Browse the repository at this point in the history
…untimeid-on-fork

[NO-TICKET] Fix crashtracking sending parent pid/runtime-id when child crashes
  • Loading branch information
ivoanjo authored Oct 25, 2024
2 parents 503d1e9 + 06dfc27 commit 321f513
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 13 deletions.
13 changes: 8 additions & 5 deletions lib/datadog/core/crashtracking/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ def initialize(tags:, agent_base_url:, ld_library_path:, path_to_crashtracking_r
def start
Utils::AtForkMonkeyPatch.apply!

start_or_update_on_fork(action: :start)
start_or_update_on_fork(action: :start, tags: tags)

ONLY_ONCE.run do
Utils::AtForkMonkeyPatch.at_fork(:child) do
# Must NOT reference `self` here, as only the first instance will
Expand All @@ -77,8 +78,10 @@ def start
end
end

def update_on_fork
start_or_update_on_fork(action: :update_on_fork)
def update_on_fork(settings: Datadog.configuration)
# Here we pick up the latest settings, so that we pick up any tags that change after forking
# such as the pid or runtime-id
start_or_update_on_fork(action: :update_on_fork, tags: TagBuilder.call(settings))
end

def stop
Expand All @@ -92,7 +95,7 @@ def stop

attr_reader :tags, :agent_base_url, :ld_library_path, :path_to_crashtracking_receiver_binary, :logger

def start_or_update_on_fork(action:)
def start_or_update_on_fork(action:, tags:)
self.class._native_start_or_update_on_fork(
action: action,
agent_base_url: agent_base_url,
Expand All @@ -101,7 +104,7 @@ def start_or_update_on_fork(action:)
tags_as_array: tags.to_a,
upload_timeout_seconds: 1
)
logger.debug("Crash tracking #{action} successfully")
logger.debug("Crash tracking action: #{action} successful")
rescue => e
logger.error("Failed to #{action} crash tracking: #{e.message}")
end
Expand Down
4 changes: 2 additions & 2 deletions sig/datadog/core/crashtracking/component.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Datadog

def start: -> void

def update_on_fork: -> void
def update_on_fork: (settings: Datadog::Core::Configuration::Settings) -> void

def stop: -> void

Expand All @@ -33,7 +33,7 @@ module Datadog
attr_reader path_to_crashtracking_receiver_binary: ::String
attr_reader logger: untyped

def start_or_update_on_fork: (action: :start | :update_on_fork) -> void
def start_or_update_on_fork: (action: :start | :update_on_fork, tags: ::Hash[::String, ::String]) -> void

def self._native_start_or_update_on_fork: (
action: :start | :update_on_fork,
Expand Down
25 changes: 19 additions & 6 deletions spec/datadog/core/crashtracking/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
require 'fiddle'

RSpec.describe Datadog::Core::Crashtracking::Component, skip: !CrashtrackingHelpers.supported? do
let(:logger) { Logger.new($stdout) }

describe '.build' do
let(:settings) { Datadog::Core::Configuration::Settings.new }
let(:agent_settings) { double('agent_settings') }
let(:logger) { Logger.new($stdout) }
let(:tags) { { 'tag1' => 'value1' } }
let(:agent_base_url) { 'agent_base_url' }
let(:ld_library_path) { 'ld_library_path' }
Expand Down Expand Up @@ -100,7 +101,6 @@
describe '#start' do
context 'when _native_start_or_update_on_fork raises an exception' do
it 'logs the exception' do
logger = Logger.new($stdout)
crashtracker = build_crashtracker(logger: logger)

expect(described_class).to receive(:_native_start_or_update_on_fork) { raise 'Test failure' }
Expand All @@ -114,7 +114,6 @@
describe '#stop' do
context 'when _native_stop_crashtracker raises an exception' do
it 'logs the exception' do
logger = Logger.new($stdout)
crashtracker = build_crashtracker(logger: logger)

expect(described_class).to receive(:_native_stop) { raise 'Test failure' }
Expand All @@ -126,9 +125,10 @@
end

describe '#update_on_fork' do
before { allow(logger).to receive(:debug) }

context 'when _native_stop_crashtracker raises an exception' do
it 'logs the exception' do
logger = Logger.new($stdout)
crashtracker = build_crashtracker(logger: logger)

expect(described_class).to receive(:_native_start_or_update_on_fork) { raise 'Test failure' }
Expand All @@ -138,12 +138,25 @@
end
end

it 'update_on_fork the crash tracker' do
it 'updates the crash tracker' do
expect(described_class).to receive(:_native_start_or_update_on_fork).with(
hash_including(action: :update_on_fork)
)

crashtracker = build_crashtracker
crashtracker = build_crashtracker(logger: logger)

crashtracker.update_on_fork
end

it 'refreshes the latest settings' do
allow(Datadog).to receive(:configuration).and_return(:latest_settings)
allow(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(:latest_settings).and_return([:latest_tags])

expect(described_class).to receive(:_native_start_or_update_on_fork).with(
hash_including(tags_as_array: [:latest_tags])
)

crashtracker = build_crashtracker(logger: logger)

crashtracker.update_on_fork
end
Expand Down

0 comments on commit 321f513

Please sign in to comment.