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-8917] Add support for the libdatadog crash tracker #3384

Merged
merged 48 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
e1340de
Introduce setting to control crash tracker option
ivoanjo Jan 16, 2024
789db6b
Extract `convert_tags` and `endpoint_from` from `HttpTransport` to li…
ivoanjo Jan 16, 2024
8afc9f2
Extract `ddtrace_version` from `HttpTransport` to `ruby_helpers.h`
ivoanjo Jan 16, 2024
a9c51b0
[PROF-8917] Add support for the libdatadog crash tracker
ivoanjo Jan 16, 2024
d89e876
Add TODO about integration spec
ivoanjo Jan 16, 2024
d3bc290
Update `crash_tracker.c` with latest libdatadog API
ivoanjo Mar 11, 2024
4889d05
Add experimental spec
ivoanjo Mar 11, 2024
37f8082
Minor: Remove unused/outdated type declaration
ivoanjo Mar 11, 2024
18758de
Redesign crash tracker to behave as regular object
ivoanjo Mar 11, 2024
02d8691
Merge branch 'master' into ivoanjo/prof-8917-crash-tracker-ruby
ivoanjo Mar 12, 2024
bc7d72c
Wire new crash tracker design into profiler
ivoanjo Mar 12, 2024
5312591
Minor: Remove redundant log message
ivoanjo Mar 12, 2024
45f5daa
Avoid leaking threads and outputting errors during spec
ivoanjo Mar 12, 2024
a5c0ba2
Rename CrashTracker to Crashtracker to match libdatadog naming
ivoanjo Mar 12, 2024
fb45524
Match fixed case for crashtracker APIs
ivoanjo Mar 12, 2024
6f83a4a
Merge branch 'master' into ivoanjo/prof-8917-crash-tracker-ruby
ivoanjo Mar 12, 2024
6af57da
Update to libdatadog 7 APIs
ivoanjo Mar 22, 2024
d19ecde
Merge branch 'master' into ivoanjo/prof-8917-crash-tracker-ruby
ivoanjo Mar 22, 2024
e670699
Enable frame resolution
ivoanjo Mar 22, 2024
d37cbb8
Use in-receiver resolve frames
ivoanjo Mar 28, 2024
d8b4122
Merge branch 'master' into ivoanjo/prof-8917-crash-tracker-ruby
ivoanjo Apr 3, 2024
6f3f8be
Adjust to latest libdatadog crash tracker changes
ivoanjo Apr 3, 2024
66e8d95
Remove TODO, it's fixed now
ivoanjo Apr 3, 2024
ce15ade
Add detail to explaining why alt stack can't be used
ivoanjo Apr 4, 2024
1b501cc
Clarify section of method where exceptions mustn't be raised
ivoanjo Apr 4, 2024
bd0537e
Document that crashtracker state is a singleton
ivoanjo Apr 4, 2024
0b40fbf
Avoid hardcoding ports when testing with built-in webrick
ivoanjo Apr 4, 2024
a3081b4
Add test coverage for crashtracker surviving in `#component_failed`
ivoanjo Apr 4, 2024
680c4a3
[NO-TICKET] Upgrade to libdatadog 8
ivoanjo Apr 9, 2024
32f02b4
Update gemfiles with libdatadog 7 -> 8 upgrade
ivoanjo Apr 9, 2024
356002e
Merge branch 'ivoanjo/libdatadog8-upgrade' into ivoanjo/prof-8917-cra…
ivoanjo Apr 9, 2024
7776107
[NO-TICKET] Upgrade to libdatadog 9
AlexJF May 6, 2024
520c8ab
Restore `required_ruby_version` to be in single line, but make Ruboco…
ivoanjo May 9, 2024
a6ddf72
Minor cleanups to comments
ivoanjo May 9, 2024
3c27b38
Minor: Fix length of guide comment for breaking lines in message
ivoanjo May 9, 2024
ed971a5
Revert "[NO-TICKET] Upgrade to libdatadog 8"
ivoanjo May 9, 2024
f029683
Revert "Update gemfiles with libdatadog 7 -> 8 upgrade"
ivoanjo May 9, 2024
7083455
Merge branch 'alexjf/libdatadog9' into ivoanjo/prof-8917-crash-tracke…
ivoanjo May 9, 2024
49e9f31
Update Ruby crashtracker to libdatadog v9 API
ivoanjo May 9, 2024
5197792
Setup `ld_library_path` argument for crashtracker
ivoanjo May 9, 2024
7cda332
Use `profiling.upload.timeout_seconds` for crashtracker timeout
ivoanjo May 9, 2024
b87f171
Remove temporary libdatadog monkey patch
ivoanjo May 9, 2024
3acf413
Fix `upload_timeout_seconds` being a float by default
ivoanjo May 9, 2024
bd7de08
Make rubocop happy
ivoanjo May 9, 2024
09976cc
Minor: Add explanation for why we're skipping crash tracker with cust…
ivoanjo May 10, 2024
5431d2b
Assert that no crashtracker is running before each test
ivoanjo May 10, 2024
a2ae730
Assert that correct signal name is reported
ivoanjo May 10, 2024
42f6ca5
Minor: Empty commit to re-trigger CI
ivoanjo May 10, 2024
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
Prev Previous commit
Next Next commit
Use profiling.upload.timeout_seconds for crashtracker timeout
  • Loading branch information
ivoanjo committed May 9, 2024
commit 7cda3323c0ac8797f7514fdd7f6d0502e81f3fe7
5 changes: 4 additions & 1 deletion ext/datadog_profiling_native_extension/crashtracker.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ static VALUE _native_start_or_update_on_fork(int argc, VALUE *argv, DDTRACE_UNUS
VALUE ld_library_path = rb_hash_fetch(options, ID2SYM(rb_intern("ld_library_path")));
VALUE tags_as_array = rb_hash_fetch(options, ID2SYM(rb_intern("tags_as_array")));
VALUE action = rb_hash_fetch(options, ID2SYM(rb_intern("action")));
VALUE upload_timeout_seconds = rb_hash_fetch(options, ID2SYM(rb_intern("upload_timeout_seconds")));

VALUE start_action = ID2SYM(rb_intern("start"));
VALUE update_on_fork_action = ID2SYM(rb_intern("update_on_fork"));

Expand All @@ -32,6 +34,7 @@ static VALUE _native_start_or_update_on_fork(int argc, VALUE *argv, DDTRACE_UNUS
ENFORCE_TYPE(path_to_crashtracking_receiver_binary, T_STRING);
ENFORCE_TYPE(ld_library_path, T_STRING);
ENFORCE_TYPE(action, T_SYMBOL);
ENFORCE_TYPE(upload_timeout_seconds, T_FIXNUM);

if (action != start_action && action != update_on_fork_action) rb_raise(rb_eArgError, "Unexpected action: %+"PRIsVALUE, action);

Expand All @@ -55,7 +58,7 @@ static VALUE _native_start_or_update_on_fork(int argc, VALUE *argv, DDTRACE_UNUS
.create_alt_stack = false,
.endpoint = endpoint,
.resolve_frames = DDOG_PROF_STACKTRACE_COLLECTION_ENABLED,
.timeout_secs = 123, // FIXME: Get correct config from Ruby
.timeout_secs = FIX2INT(upload_timeout_seconds),
};

ddog_prof_CrashtrackerMetadata metadata = {
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/profiling/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
Datadog::Profiling::Crashtracker.new(
exporter_configuration: transport.exporter_configuration,
tags: Datadog::Profiling::TagBuilder.call(settings: settings),
upload_timeout_seconds: settings.profiling.upload.timeout_seconds,
)
end

Expand Down
6 changes: 5 additions & 1 deletion lib/datadog/profiling/crashtracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,21 @@ module Profiling
class Crashtracker
private

attr_reader :exporter_configuration, :tags_as_array, :path_to_crashtracking_receiver_binary, :ld_library_path
attr_reader :exporter_configuration, :tags_as_array, :path_to_crashtracking_receiver_binary, :ld_library_path, \
:upload_timeout_seconds

public

def initialize(
exporter_configuration:,
tags:,
upload_timeout_seconds:,
path_to_crashtracking_receiver_binary: Libdatadog.path_to_crashtracking_receiver_binary,
ld_library_path: Libdatadog.ld_library_path
)
@exporter_configuration = exporter_configuration
@tags_as_array = tags.to_a
@upload_timeout_seconds = upload_timeout_seconds
@path_to_crashtracking_receiver_binary = path_to_crashtracking_receiver_binary
@ld_library_path = ld_library_path
end
Expand Down Expand Up @@ -83,6 +86,7 @@ def start_or_update_on_fork(action:)
path_to_crashtracking_receiver_binary: path_to_crashtracking_receiver_binary,
ld_library_path: ld_library_path,
tags_as_array: tags_as_array,
upload_timeout_seconds: upload_timeout_seconds,
)
Datadog.logger.debug("Crash tracking #{action} successful")
rescue => e
Expand Down
3 changes: 3 additions & 0 deletions sig/datadog/profiling/crashtracker.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ module Datadog
attr_reader tags_as_array: ::Array[[::String, ::String]]
attr_reader path_to_crashtracking_receiver_binary: ::String
attr_reader ld_library_path: ::String
attr_reader upload_timeout_seconds: ::Integer

public

def initialize: (
exporter_configuration: exporter_configuration_array,
tags: ::Hash[::String, ::String],
upload_timeout_seconds: ::Integer,
?path_to_crashtracking_receiver_binary: ::String,
?ld_library_path: ::String,
) -> void
Expand All @@ -33,6 +35,7 @@ module Datadog
path_to_crashtracking_receiver_binary: ::String,
ld_library_path: ::String,
tags_as_array: ::Array[[::String, ::String]],
upload_timeout_seconds: ::Integer,
) -> void

def self._native_stop: -> void
Expand Down
1 change: 1 addition & 0 deletions spec/datadog/profiling/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@
expect(Datadog::Profiling::Crashtracker).to receive(:new).with(
exporter_configuration: array_including(:agent),
tags: hash_including('runtime' => 'ruby'),
upload_timeout_seconds: settings.profiling.upload.timeout_seconds,
)

build_profiler_component
Expand Down
27 changes: 9 additions & 18 deletions spec/datadog/profiling/crashtracker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@

let(:exporter_configuration) { [:agent, 'http://localhost:6006'] }

subject(:crashtracker) do
described_class.new(
let(:crashtracker_options) {
{
exporter_configuration: exporter_configuration,
tags: { 'tag1' => 'value1', 'tag2' => 'value2' },
)
end
upload_timeout_seconds: 123,
}
}

subject(:crashtracker) { described_class.new(**crashtracker_options) }

describe '#start' do
subject(:start) { crashtracker.start }
Expand All @@ -28,13 +31,7 @@
end

context 'when path_to_crashtracking_receiver_binary is nil' do
subject(:crashtracker) do
described_class.new(
exporter_configuration: exporter_configuration,
tags: { 'tag1' => 'value1', 'tag2' => 'value2' },
path_to_crashtracking_receiver_binary: nil
)
end
subject(:crashtracker) { described_class.new(**crashtracker_options, path_to_crashtracking_receiver_binary: nil) }

it 'logs a warning' do
expect(Datadog.logger).to receive(:warn).with(/no path_to_crashtracking_receiver_binary was found/)
Expand All @@ -44,13 +41,7 @@
end

context 'when ld_library_path is nil' do
subject(:crashtracker) do
described_class.new(
exporter_configuration: exporter_configuration,
tags: { 'tag1' => 'value1', 'tag2' => 'value2' },
ld_library_path: nil
)
end
subject(:crashtracker) { described_class.new(**crashtracker_options, ld_library_path: nil) }

it 'logs a warning' do
expect(Datadog.logger).to receive(:warn).with(/no ld_library_path was found/)
Expand Down