From c59c4f34235a421f5f23f11f2fa86475627d0ba9 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Tue, 6 Aug 2024 11:26:58 +0200 Subject: [PATCH] Extract crashtracking component into core --- Rakefile | 11 +- ext/libdatadog_api/crashtracker.c | 11 +- lib/datadog/core/configuration/components.rb | 15 +- lib/datadog/core/configuration/settings.rb | 28 +- .../core/crashtracking/agent_base_url.rb | 21 ++ lib/datadog/core/crashtracking/component.rb | 104 ++++++ lib/datadog/core/crashtracking/tag_builder.rb | 39 +++ lib/datadog/profiling.rb | 1 - lib/datadog/profiling/component.rb | 35 +- lib/datadog/profiling/crashtracker.rb | 99 ------ lib/datadog/profiling/profiler.rb | 8 +- sig/datadog/core/configuration/settings.rbs | 4 +- .../core/crashtracking/agent_base_url.rbs | 9 + sig/datadog/core/crashtracking/component.rbs | 51 +++ .../core/crashtracking/tag_builder.rbs | 9 + .../core/utils/at_fork_monkey_patch.rbs | 28 +- sig/datadog/profiling/component.rbs | 5 - sig/datadog/profiling/crashtracker.rbs | 46 --- sig/datadog/profiling/profiler.rbs | 4 +- .../core/configuration/components_spec.rb | 8 +- .../core/configuration/settings_spec.rb | 45 ++- .../core/crashtracking/agent_base_url_spec.rb | 64 ++++ .../core/crashtracking/component_spec.rb | 302 ++++++++++++++++++ .../core/crashtracking/tag_builder_spec.rb | 111 +++++++ spec/datadog/core/telemetry/event_spec.rb | 5 +- spec/datadog/profiling/component_spec.rb | 81 +---- spec/datadog/profiling/crashtracker_spec.rb | 221 ------------- spec/datadog/profiling/profiler_spec.rb | 63 +--- 28 files changed, 849 insertions(+), 579 deletions(-) create mode 100644 lib/datadog/core/crashtracking/agent_base_url.rb create mode 100644 lib/datadog/core/crashtracking/component.rb create mode 100644 lib/datadog/core/crashtracking/tag_builder.rb delete mode 100644 lib/datadog/profiling/crashtracker.rb create mode 100644 sig/datadog/core/crashtracking/agent_base_url.rbs create mode 100644 sig/datadog/core/crashtracking/component.rbs create mode 100644 sig/datadog/core/crashtracking/tag_builder.rbs delete mode 100644 sig/datadog/profiling/crashtracker.rbs create mode 100644 spec/datadog/core/crashtracking/agent_base_url_spec.rb create mode 100644 spec/datadog/core/crashtracking/component_spec.rb create mode 100644 spec/datadog/core/crashtracking/tag_builder_spec.rb delete mode 100644 spec/datadog/profiling/crashtracker_spec.rb diff --git a/Rakefile b/Rakefile index 67e33714796..016c24e73da 100644 --- a/Rakefile +++ b/Rakefile @@ -73,7 +73,7 @@ namespace :spec do task all: [:main, :benchmark, :rails, :railsredis, :railsredis_activesupport, :railsactivejob, :elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument, - :profiling] + :profiling, :crashtracking] desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:main) do |t, args| @@ -169,6 +169,15 @@ namespace :spec do t.rspec_opts = args.to_a.join(' ') end + # rubocop:disable Style/MultilineBlockChain + RSpec::Core::RakeTask.new(:crashtracking) do |t, args| + t.pattern = 'spec/datadog/core/crashtracking/**/*_spec.rb' + t.rspec_opts = args.to_a.join(' ') + end.tap do |t| + Rake::Task[t.name].enhance(["compile:libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}"]) + end + # rubocop:enable Style/MultilineBlockChain + desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:contrib) do |t, args| contrib_paths = [ diff --git a/ext/libdatadog_api/crashtracker.c b/ext/libdatadog_api/crashtracker.c index ce574bca3a9..8f9552935eb 100644 --- a/ext/libdatadog_api/crashtracker.c +++ b/ext/libdatadog_api/crashtracker.c @@ -5,20 +5,21 @@ static VALUE _native_start_or_update_on_fork(int argc, VALUE *argv, DDTRACE_UNUSED VALUE _self); static VALUE _native_stop(DDTRACE_UNUSED VALUE _self); -static void crashtracker_init(VALUE profiling_module); +static void crashtracker_init(VALUE crashtracking_module); // Used to report Ruby VM crashes. // Once initialized, segfaults will be reported automatically using libdatadog. void DDTRACE_EXPORT Init_libdatadog_api(void) { VALUE datadog_module = rb_define_module("Datadog"); - VALUE profiling_module = rb_define_module_under(datadog_module, "Profiling"); + VALUE core_module = rb_define_module_under(datadog_module, "Core"); + VALUE crashtracking_module = rb_define_module_under(core_module, "Crashtracking"); - crashtracker_init(profiling_module); + crashtracker_init(crashtracking_module); } -void crashtracker_init(VALUE profiling_module) { - VALUE crashtracker_class = rb_define_class_under(profiling_module, "Crashtracker", rb_cObject); +void crashtracker_init(VALUE crashtracking_module) { + VALUE crashtracker_class = rb_define_class_under(crashtracking_module, "Component", rb_cObject); rb_define_singleton_method(crashtracker_class, "_native_start_or_update_on_fork", _native_start_or_update_on_fork, -1); rb_define_singleton_method(crashtracker_class, "_native_stop", _native_stop, 0); diff --git a/lib/datadog/core/configuration/components.rb b/lib/datadog/core/configuration/components.rb index f14152eecd4..ac999fb2825 100644 --- a/lib/datadog/core/configuration/components.rb +++ b/lib/datadog/core/configuration/components.rb @@ -13,6 +13,7 @@ require_relative '../../tracing/component' require_relative '../../profiling/component' require_relative '../../appsec/component' +require_relative '../crashtracking/component' module Datadog module Core @@ -58,6 +59,17 @@ def build_runtime_metrics_worker(settings) def build_telemetry(settings, agent_settings, logger) Telemetry::Component.build(settings, agent_settings, logger) end + + def build_crashtracker(settings, agent_settings, logger:) + return unless settings.crashtracking.enabled + + if (libdatadog_api_failure = Datadog::Core::Crashtracking::Component::LIBDATADOG_API_FAILURE) + logger.debug("Cannot enable crashtracking: #{libdatadog_api_failure}") + return + end + + Datadog::Core::Crashtracking::Component.build(settings, agent_settings, logger: logger) + end end include Datadog::Tracing::Component::InstanceMethods @@ -83,11 +95,12 @@ def initialize(settings) @remote = Remote::Component.build(settings, agent_settings) @tracer = self.class.build_tracer(settings, agent_settings, logger: @logger) + self.class.build_crashtracker(settings, agent_settings, logger: @logger) @profiler, profiler_logger_extra = Datadog::Profiling::Component.build_profiler_component( settings: settings, agent_settings: agent_settings, - optional_tracer: @tracer, + optional_tracer: @tracer ) @environment_logger_extra.merge!(profiler_logger_extra) if profiler_logger_extra diff --git a/lib/datadog/core/configuration/settings.rb b/lib/datadog/core/configuration/settings.rb index db7e0596337..3fbb2b34db1 100644 --- a/lib/datadog/core/configuration/settings.rb +++ b/lib/datadog/core/configuration/settings.rb @@ -451,17 +451,16 @@ def initialize(*_) o.default 60 end - # Enables reporting of information when the Ruby VM crashes. - # - # This feature is no longer experimental, and we plan to deprecate this setting and replace it with a - # properly-named one soon. - # - # @default `DD_PROFILING_EXPERIMENTAL_CRASH_TRACKING_ENABLED` environment variable as a boolean, - # otherwise `true` + # DEV-3.0: Remove `experimental_crash_tracking_enabled` option option :experimental_crash_tracking_enabled do |o| - o.type :bool - o.env 'DD_PROFILING_EXPERIMENTAL_CRASH_TRACKING_ENABLED' - o.default true + o.after_set do |_, _, precedence| + unless precedence == Datadog::Core::Configuration::Option::Precedence::DEFAULT + Core.log_deprecation(key: :experimental_crash_tracking_enabled) do + 'The profiling.advanced.experimental_crash_tracking_enabled setting has been deprecated for removal '\ + 'and no longer does anything. Please remove it from your Datadog.configure block.' + end + end + end end end @@ -833,6 +832,15 @@ def initialize(*_) option :service end + settings :crashtracking do + # Enables reporting of information when Ruby VM crashes. + option :enabled do |o| + o.type :bool + o.default true + o.env 'DD_CRASHTRACKING_ENABLED' + end + end + # TODO: Tracing should manage its own settings. # Keep this extension here for now to keep things working. extend Datadog::Tracing::Configuration::Settings diff --git a/lib/datadog/core/crashtracking/agent_base_url.rb b/lib/datadog/core/crashtracking/agent_base_url.rb new file mode 100644 index 00000000000..0a1a95ac605 --- /dev/null +++ b/lib/datadog/core/crashtracking/agent_base_url.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require_relative '../configuration/ext' + +module Datadog + module Core + module Crashtracking + # This module provides a method to resolve the base URL of the agent + module AgentBaseUrl + def self.resolve(agent_settings) + case agent_settings.adapter + when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER + "#{agent_settings.ssl ? 'https' : 'http'}://#{agent_settings.hostname}:#{agent_settings.port}/" + when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER + "unix://#{agent_settings.uds_path}" + end + end + end + end + end +end diff --git a/lib/datadog/core/crashtracking/component.rb b/lib/datadog/core/crashtracking/component.rb new file mode 100644 index 00000000000..6ebf389b9d1 --- /dev/null +++ b/lib/datadog/core/crashtracking/component.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'libdatadog' + +require_relative 'tag_builder' +require_relative 'agent_base_url' +require_relative '../utils/only_once' +require_relative '../utils/at_fork_monkey_patch' + +module Datadog + module Core + module Crashtracking + # Used to report Ruby VM crashes. + # + # NOTE: The crashtracker native state is a singleton; so even if you create multiple instances of `Crashtracker` + # and start them, it only works as "last writer wins". Same for stop -- there's only one state, so calling stop + # on it will stop the crash tracker, regardless of which instance started it. + # + # Methods prefixed with _native_ are implemented in `crashtracker.c` + class Component + LIBDATADOG_API_FAILURE = + begin + require "libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}" + nil + rescue LoadError => e + e.message + end + + ONLY_ONCE = Core::Utils::OnlyOnce.new + + def self.build(settings, agent_settings, logger:) + tags = TagBuilder.call(settings) + agent_base_url = AgentBaseUrl.resolve(agent_settings) + logger.warn('Missing agent base URL; cannot enable crash tracking') unless agent_base_url + + ld_library_path = ::Libdatadog.ld_library_path + logger.warn('Missing ld_library_path; cannot enable crash tracking') unless ld_library_path + + path_to_crashtracking_receiver_binary = ::Libdatadog.path_to_crashtracking_receiver_binary + unless path_to_crashtracking_receiver_binary + logger.warn('Missing path_to_crashtracking_receiver_binary; cannot enable crash tracking') + end + + return unless agent_base_url + return unless ld_library_path + return unless path_to_crashtracking_receiver_binary + + new( + tags: tags, + agent_base_url: agent_base_url, + ld_library_path: ld_library_path, + path_to_crashtracking_receiver_binary: path_to_crashtracking_receiver_binary, + logger: logger + ).tap(&:start) + end + + def initialize(tags:, agent_base_url:, ld_library_path:, path_to_crashtracking_receiver_binary:, logger:) + @tags = tags + @agent_base_url = agent_base_url + @ld_library_path = ld_library_path + @path_to_crashtracking_receiver_binary = path_to_crashtracking_receiver_binary + @logger = logger + end + + def start + Utils::AtForkMonkeyPatch.apply! + + start_or_update_on_fork(action: :start) + ONLY_ONCE.run do + Utils::AtForkMonkeyPatch.at_fork(:child) do + start_or_update_on_fork(action: :update_on_fork) + end + end + end + + def stop + self.class._native_stop + logger.debug('Crash tracking stopped successfully') + rescue => e + logger.error("Failed to stop crash tracking: #{e.message}") + end + + private + + attr_reader :tags, :agent_base_url, :ld_library_path, :path_to_crashtracking_receiver_binary, :logger + + def start_or_update_on_fork(action:) + logger.debug("Crash tracking #{action}...") + self.class._native_start_or_update_on_fork( + action: action, + exporter_configuration: [:agent, agent_base_url], + path_to_crashtracking_receiver_binary: path_to_crashtracking_receiver_binary, + ld_library_path: ld_library_path, + tags_as_array: tags.to_a, + upload_timeout_seconds: 1 + ) + logger.debug("Crash tracking #{action} successful") + rescue => e + logger.error("Failed to #{action} crash tracking: #{e.message}") + end + end + end + end +end diff --git a/lib/datadog/core/crashtracking/tag_builder.rb b/lib/datadog/core/crashtracking/tag_builder.rb new file mode 100644 index 00000000000..0033deb73d8 --- /dev/null +++ b/lib/datadog/core/crashtracking/tag_builder.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require_relative '../utils' +require_relative '../environment/socket' +require_relative '../environment/identity' +require_relative '../environment/git' + +module Datadog + module Core + module Crashtracking + # This module builds a hash of tags + module TagBuilder + def self.call(settings) + hash = { + 'host' => Environment::Socket.hostname, + 'language' => Environment::Identity.lang, + 'process_id' => Process.pid.to_s, + 'profiler_version' => Environment::Identity.gem_datadog_version, + 'runtime' => Environment::Identity.lang, # This is known to be repeated from language, above + 'runtime_engine' => Environment::Identity.lang_engine, + 'runtime-id' => Environment::Identity.id, + 'runtime_platform' => Environment::Identity.lang_platform, + 'runtime_version' => Environment::Identity.lang_version, + 'env' => settings.env, + 'service' => settings.service, + 'version' => settings.version, + 'git.repository_url' => Environment::Git.git_repository_url, + 'git.commit.sha' => Environment::Git.git_commit_sha, + }.compact + + # Make sure everything is an utf-8 string, to avoid encoding issues in downstream + settings.tags.merge(hash).each_with_object({}) do |(key, value), h| + h[Utils.utf8_encode(key)] = Utils.utf8_encode(value) + end + end + end + end + end +end diff --git a/lib/datadog/profiling.rb b/lib/datadog/profiling.rb index 37390cb9ccb..ad28313bc77 100644 --- a/lib/datadog/profiling.rb +++ b/lib/datadog/profiling.rb @@ -144,7 +144,6 @@ def self.allocation_count # rubocop:disable Lint/NestedMethodDefinition (On purp require_relative 'profiling/collectors/idle_sampling_helper' require_relative 'profiling/collectors/stack' require_relative 'profiling/collectors/thread_context' - require_relative 'profiling/crashtracker' require_relative 'profiling/stack_recorder' require_relative 'profiling/exporter' require_relative 'profiling/flush' diff --git a/lib/datadog/profiling/component.rb b/lib/datadog/profiling/component.rb index 5d4e68e82d1..a70e5c03e61 100644 --- a/lib/datadog/profiling/component.rb +++ b/lib/datadog/profiling/component.rb @@ -73,8 +73,10 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:) exporter = build_profiler_exporter(settings, recorder, worker, internal_metadata: internal_metadata) transport = build_profiler_transport(settings, agent_settings) scheduler = Profiling::Scheduler.new(exporter: exporter, transport: transport, interval: upload_period_seconds) - crashtracker = build_crashtracker(settings, transport) - profiler = Profiling::Profiler.new(worker: worker, scheduler: scheduler, optional_crashtracker: crashtracker) + profiler = Profiling::Profiler.new( + worker: worker, + scheduler: scheduler + ) if dir_interruption_workaround_enabled?(settings, no_signals_workaround_enabled) Datadog::Profiling::Ext::DirMonkeyPatches.apply! @@ -117,35 +119,6 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:) ) end - private_class_method def self.build_crashtracker(settings, transport) - return unless settings.profiling.advanced.experimental_crash_tracking_enabled - - # By default, the transport is an instance of HttpTransport, which validates the configuration and makes - # it available for us to use here. - # But we support overriding the transport with a user-specific one, which may e.g. write stuff to a file, - # and thus can't really provide a valid configuration to talk to a Datadog agent. Thus, in this situation, - # we can't use the crashtracker, even if enabled. - unless transport.respond_to?(:exporter_configuration) - Datadog.logger.debug( - 'Cannot enable profiling crash tracking as a custom settings.profiling.exporter.transport is configured' - ) - return - end - - if Datadog::Profiling::Crashtracker::LIBDATADOG_API_FAILURE - Datadog.logger.debug( - "Cannot enable crashtracking: #{Datadog::Profiling::Crashtracker::LIBDATADOG_API_FAILURE}" - ) - return - end - - 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 - private_class_method def self.enable_gc_profiling?(settings) return false unless settings.profiling.advanced.gc_enabled diff --git a/lib/datadog/profiling/crashtracker.rb b/lib/datadog/profiling/crashtracker.rb deleted file mode 100644 index 99c55a4d4b3..00000000000 --- a/lib/datadog/profiling/crashtracker.rb +++ /dev/null @@ -1,99 +0,0 @@ -# frozen_string_literal: true - -require 'libdatadog' - -module Datadog - module Profiling - # Used to report Ruby VM crashes. - # The interesting bits are implemented as native code and using libdatadog. - # - # NOTE: The crashtracker native state is a singleton; so even if you create multiple instances of `Crashtracker` - # and start them, it only works as "last writer wins". Same for stop -- there's only one state, so calling stop - # on it will stop the crash tracker, regardless of which instance started it. - # - # Methods prefixed with _native_ are implemented in `crashtracker.c` - class Crashtracker - LIBDATADOG_API_FAILURE = - begin - require "libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}" - nil - rescue LoadError => e - e.message - end - - private - - 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 - - def start - start_or_update_on_fork(action: :start) - end - - def reset_after_fork - start_or_update_on_fork(action: :update_on_fork) - end - - def stop - begin - self.class._native_stop - Datadog.logger.debug('Crash tracking stopped successfully') - rescue => e - Datadog.logger.error("Failed to stop crash tracking: #{e.message}") - end - end - - private - - def start_or_update_on_fork(action:) - unless path_to_crashtracking_receiver_binary - Datadog.logger.warn( - "Cannot #{action} profiling crash tracking as no path_to_crashtracking_receiver_binary was found" - ) - return - end - - unless ld_library_path - Datadog.logger.warn( - "Cannot #{action} profiling crash tracking as no ld_library_path was found" - ) - return - end - - begin - self.class._native_start_or_update_on_fork( - action: action, - exporter_configuration: exporter_configuration, - 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: Integer(upload_timeout_seconds), - ) - Datadog.logger.debug("Crash tracking #{action} successful") - rescue => e - Datadog.logger.error("Failed to #{action} crash tracking: #{e.message}") - end - end - end - end -end diff --git a/lib/datadog/profiling/profiler.rb b/lib/datadog/profiling/profiler.rb index 8d6578a8fc8..30c25e2b2ad 100644 --- a/lib/datadog/profiling/profiler.rb +++ b/lib/datadog/profiling/profiler.rb @@ -8,24 +8,21 @@ class Profiler private - attr_reader :worker, :scheduler, :optional_crashtracker + attr_reader :worker, :scheduler public - def initialize(worker:, scheduler:, optional_crashtracker:) + def initialize(worker:, scheduler:) @worker = worker @scheduler = scheduler - @optional_crashtracker = optional_crashtracker end def start after_fork! do - optional_crashtracker.reset_after_fork if optional_crashtracker worker.reset_after_fork scheduler.reset_after_fork end - optional_crashtracker.start if optional_crashtracker worker.start(on_failure_proc: proc { component_failed(:worker) }) scheduler.start(on_failure_proc: proc { component_failed(:scheduler) }) end @@ -35,7 +32,6 @@ def shutdown! stop_worker stop_scheduler - optional_crashtracker.stop if optional_crashtracker end private diff --git a/sig/datadog/core/configuration/settings.rbs b/sig/datadog/core/configuration/settings.rbs index ca81552f618..fe07d63c6db 100644 --- a/sig/datadog/core/configuration/settings.rbs +++ b/sig/datadog/core/configuration/settings.rbs @@ -70,12 +70,14 @@ module Datadog def initialize: (*untyped _) -> untyped - def env: -> String + def env: -> String? def service: -> String def version: -> String? + def tags: -> ::Hash[::String, ::String] + def logger=: (untyped logger) -> untyped def runtime_metrics: (?untyped? options) -> untyped diff --git a/sig/datadog/core/crashtracking/agent_base_url.rbs b/sig/datadog/core/crashtracking/agent_base_url.rbs new file mode 100644 index 00000000000..c83ba9f180d --- /dev/null +++ b/sig/datadog/core/crashtracking/agent_base_url.rbs @@ -0,0 +1,9 @@ +module Datadog + module Core + module Crashtracking + module AgentBaseUrl + def self.resolve: (untyped) -> ::String? + end + end + end +end diff --git a/sig/datadog/core/crashtracking/component.rbs b/sig/datadog/core/crashtracking/component.rbs new file mode 100644 index 00000000000..4c9fac3664c --- /dev/null +++ b/sig/datadog/core/crashtracking/component.rbs @@ -0,0 +1,51 @@ +module Datadog + module Core + module Crashtracking + class Component + LIBDATADOG_API_FAILURE: ::String? + ONLY_ONCE: Core::Utils::OnlyOnce + + def self.build: ( + Datadog::Core::Configuration::Settings settings, + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings, + logger: untyped + ) -> Datadog::Core::Crashtracking::Component? + + def initialize: ( + tags: ::Hash[::String, ::String], + agent_base_url: ::String, + ld_library_path: ::String, + path_to_crashtracking_receiver_binary: ::String, + logger: untyped + ) -> void + + def start: -> void + + def reset_after_fork: -> void + + def stop: -> void + + private + + attr_reader tags: ::Hash[::String, ::String] + attr_reader agent_base_url: ::String + attr_reader ld_library_path: ::String + 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 self._native_start_or_update_on_fork: ( + action: :start | :update_on_fork, + exporter_configuration: Array[:agent | ::String], + 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 + end + end + end +end diff --git a/sig/datadog/core/crashtracking/tag_builder.rbs b/sig/datadog/core/crashtracking/tag_builder.rbs new file mode 100644 index 00000000000..7be270c565e --- /dev/null +++ b/sig/datadog/core/crashtracking/tag_builder.rbs @@ -0,0 +1,9 @@ +module Datadog + module Core + module Crashtracking + module TagBuilder + def self.call: (Datadog::Core::Configuration::Settings) -> ::Hash[::String, ::String] + end + end + end +end diff --git a/sig/datadog/core/utils/at_fork_monkey_patch.rbs b/sig/datadog/core/utils/at_fork_monkey_patch.rbs index 464090415c4..04036f322b0 100644 --- a/sig/datadog/core/utils/at_fork_monkey_patch.rbs +++ b/sig/datadog/core/utils/at_fork_monkey_patch.rbs @@ -1 +1,27 @@ -# TODO +module Datadog + module Core + module Utils + module AtForkMonkeyPatch + AT_FORK_CHILD_BLOCKS: ::Array[untyped] + + def self.supported?: () -> untyped + + def self.apply!: () -> (false | true) + + def self.run_at_fork_blocks: (untyped stage) -> untyped + + def self.at_fork: (untyped stage) ?{ () -> untyped } -> true + + module KernelMonkeyPatch + def fork: () ?{ () -> untyped } -> untyped + end + + module ProcessMonkeyPatch + def _fork: () -> untyped + + def daemon: (*untyped args) -> untyped + end + end + end + end +end diff --git a/sig/datadog/profiling/component.rbs b/sig/datadog/profiling/component.rbs index bda16b8f78e..df04cea2959 100644 --- a/sig/datadog/profiling/component.rbs +++ b/sig/datadog/profiling/component.rbs @@ -26,11 +26,6 @@ module Datadog Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings ) -> untyped - def self.build_crashtracker: ( - untyped settings, - untyped transport, - ) -> Datadog::Profiling::Crashtracker? - def self.enable_gc_profiling?: (untyped settings) -> bool def self.enable_allocation_profiling?: (untyped settings) -> bool def self.get_heap_sample_every: (untyped settings) -> ::Integer diff --git a/sig/datadog/profiling/crashtracker.rbs b/sig/datadog/profiling/crashtracker.rbs deleted file mode 100644 index e1975cd5a50..00000000000 --- a/sig/datadog/profiling/crashtracker.rbs +++ /dev/null @@ -1,46 +0,0 @@ -module Datadog - module Profiling - class Crashtracker - type exporter_configuration_array = [:agentless | :agent, untyped] - - LIBDATADOG_API_FAILURE: ::String? - - private - - attr_reader exporter_configuration: exporter_configuration_array - 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 - - def start: -> void - def stop: -> void - def reset_after_fork: -> void - - private - - def start_or_update_on_fork: (action: :start | :update_on_fork) -> void - - def self._native_start_or_update_on_fork: ( - action: :start | :update_on_fork, - exporter_configuration: exporter_configuration_array, - 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 - end - end -end diff --git a/sig/datadog/profiling/profiler.rbs b/sig/datadog/profiling/profiler.rbs index b9c3180d41e..99f0a744a83 100644 --- a/sig/datadog/profiling/profiler.rbs +++ b/sig/datadog/profiling/profiler.rbs @@ -7,14 +7,12 @@ module Datadog attr_reader worker: Datadog::Profiling::Collectors::CpuAndWallTimeWorker attr_reader scheduler: Datadog::Profiling::Scheduler - attr_reader optional_crashtracker: Datadog::Profiling::Crashtracker public def initialize: ( worker: Datadog::Profiling::Collectors::CpuAndWallTimeWorker, - scheduler: Datadog::Profiling::Scheduler, - optional_crashtracker: Datadog::Profiling::Crashtracker?, + scheduler: Datadog::Profiling::Scheduler ) -> void def start: () -> void diff --git a/spec/datadog/core/configuration/components_spec.rb b/spec/datadog/core/configuration/components_spec.rb index bc78feaee0e..7d7efe78180 100644 --- a/spec/datadog/core/configuration/components_spec.rb +++ b/spec/datadog/core/configuration/components_spec.rb @@ -67,11 +67,15 @@ expect(described_class).to receive(:build_tracer) .with(settings, agent_settings, logger: logger) .and_return(tracer) + crashtracker = double('crashtracker') + expect(described_class).to receive(:build_crashtracker) + .with(settings, agent_settings, logger: logger) + .and_return(crashtracker) expect(Datadog::Profiling::Component).to receive(:build_profiler_component).with( settings: settings, agent_settings: agent_settings, - optional_tracer: tracer, + optional_tracer: tracer ).and_return([profiler, environment_logger_extra]) expect(described_class).to receive(:build_runtime_metrics_worker) @@ -1082,7 +1086,7 @@ expect(Datadog::Profiling::Component).to receive(:build_profiler_component).with( settings: settings, agent_settings: agent_settings, - optional_tracer: anything, + optional_tracer: anything ).and_return([profiler, environment_logger_extra]) end diff --git a/spec/datadog/core/configuration/settings_spec.rb b/spec/datadog/core/configuration/settings_spec.rb index c4c232de10e..c6da174f948 100644 --- a/spec/datadog/core/configuration/settings_spec.rb +++ b/spec/datadog/core/configuration/settings_spec.rb @@ -828,14 +828,14 @@ context 'is not defined' do let(:environment) { nil } - it { is_expected.to be true } + it { is_expected.to be_nil } end [true, false].each do |value| context "is defined as #{value}" do let(:environment) { value.to_s } - it { is_expected.to be value } + it { is_expected.to be_nil } end end end @@ -843,10 +843,9 @@ describe '#experimental_crash_tracking_enabled=' do it 'updates the #experimental_crash_tracking_enabled setting' do - expect { settings.profiling.advanced.experimental_crash_tracking_enabled = false } + expect { settings.profiling.advanced.experimental_crash_tracking_enabled = true } .to change { settings.profiling.advanced.experimental_crash_tracking_enabled } - .from(true) - .to(false) + .from(nil).to(true) end end @@ -1836,4 +1835,40 @@ end end end + + describe '#crashtracking' do + describe '#enabled' do + subject(:crashtracking_enabled) { settings.crashtracking.enabled } + + context 'when DD_CRASHTRACKING_ENABLED' do + around do |example| + ClimateControl.modify('DD_CRASHTRACKING_ENABLED' => environment) do + example.run + end + end + + context 'is not defined' do + let(:environment) { nil } + + it { is_expected.to be true } + end + + [true, false].each do |value| + context "is defined as #{value}" do + let(:environment) { value.to_s } + + it { is_expected.to be value } + end + end + end + end + + describe '#enabled=' do + it 'updates the #enabled setting' do + expect { settings.crashtracking.enabled = false } + .to change { settings.crashtracking.enabled } + .from(true).to(false) + end + end + end end diff --git a/spec/datadog/core/crashtracking/agent_base_url_spec.rb b/spec/datadog/core/crashtracking/agent_base_url_spec.rb new file mode 100644 index 00000000000..99c73019906 --- /dev/null +++ b/spec/datadog/core/crashtracking/agent_base_url_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'datadog/core/crashtracking/agent_base_url' + +RSpec.describe Datadog::Core::Crashtracking::AgentBaseUrl do + describe '.resolve' do + context 'when using HTTP adapter' do + context 'when SSL is enabled' do + let(:agent_settings) do + double( + 'agent_settings', + adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, + ssl: true, + hostname: 'example.com', + port: 8080 + ) + end + + it 'returns the correct base URL' do + expect(described_class.resolve(agent_settings)).to eq('https://example.com:8080/') + end + end + + context 'when SSL is disabled' do + let(:agent_settings) do + double( + 'agent_settings', + adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, + ssl: false, + hostname: 'example.com', + port: 8080 + ) + end + + it 'returns the correct base URL' do + expect(described_class.resolve(agent_settings)).to eq('http://example.com:8080/') + end + end + end + + context 'when using UnixSocket adapter' do + let(:agent_settings) do + double( + 'agent_settings', + adapter: Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER, + uds_path: '/var/run/datadog.sock' + ) + end + + it 'returns the correct base URL' do + expect(described_class.resolve(agent_settings)).to eq('unix:///var/run/datadog.sock') + end + end + + context 'when using unknownm adapter' do + let(:agent_settings) { double('agent_settings', adapter: 'unknown') } + + it 'returns nil' do + expect(described_class.resolve(agent_settings)).to be_nil + end + end + end +end diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb new file mode 100644 index 00000000000..8d261608ac9 --- /dev/null +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -0,0 +1,302 @@ +require 'spec_helper' +require 'datadog/core/crashtracking/component' + +require 'webrick' +require 'fiddle' + +RSpec.describe Datadog::Core::Crashtracking::Component, + skip: Datadog::Core::Crashtracking::Component::LIBDATADOG_API_FAILURE do + describe '.build' do + let(:settings) { Datadog::Core::Configuration::Settings.new } + let(:agent_settings) { double('agent_settings') } + let(:logger) { Logger.new($stdout) } + let(:tags) { {} } + let(:agent_base_url) { 'agent_base_url' } + let(:ld_library_path) { 'ld_library_path' } + let(:path_to_crashtracking_receiver_binary) { 'path_to_crashtracking_receiver_binary' } + + context 'when all required parameters are provided' do + it 'creates a new instance of Component and starts it' do + expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) + .and_return(tags) + expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) + .and_return(agent_base_url) + expect(::Libdatadog).to receive(:ld_library_path) + .and_return(ld_library_path) + expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) + .and_return(path_to_crashtracking_receiver_binary) + + component = double('component') + expect(described_class).to receive(:new).with( + tags: tags, + agent_base_url: agent_base_url, + ld_library_path: ld_library_path, + path_to_crashtracking_receiver_binary: path_to_crashtracking_receiver_binary, + logger: logger + ).and_return(component) + + expect(component).to receive(:start) + + described_class.build(settings, agent_settings, logger: logger) + end + end + + context 'when missing `agent_base_url`' do + let(:agent_base_url) { nil } + + it 'returns nil' do + expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) + .and_return(tags) + expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) + .and_return(agent_base_url) + expect(::Libdatadog).to receive(:ld_library_path) + .and_return(ld_library_path) + expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) + .and_return(path_to_crashtracking_receiver_binary) + + expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil + end + end + + context 'when missing `ld_library_path`' do + let(:ld_library_path) { nil } + + it 'returns nil' do + expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) + .and_return(tags) + expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) + .and_return(agent_base_url) + expect(::Libdatadog).to receive(:ld_library_path) + .and_return(ld_library_path) + expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) + .and_return(path_to_crashtracking_receiver_binary) + + expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil + end + end + + context 'when missing `path_to_crashtracking_receiver_binary`' do + let(:path_to_crashtracking_receiver_binary) { nil } + + it 'returns nil' do + expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) + .and_return(tags) + expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) + .and_return(agent_base_url) + expect(::Libdatadog).to receive(:ld_library_path) + .and_return(ld_library_path) + expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) + .and_return(path_to_crashtracking_receiver_binary) + + expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil + end + end + end + + context 'instance methods' do + before do + # No crash tracker process should still be running at the start of each testcase + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty + end + + after do + # No crash tracker process should still be running at the end of each testcase + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty + end + + let(:logger) { Logger.new($stdout) } + let(:agent_base_url) { 'http://localhost:6006' } + + let(:crashtracker_options) do + { + agent_base_url: agent_base_url, + tags: { 'tag1' => 'value1', 'tag2' => 'value2' }, + path_to_crashtracking_receiver_binary: ::Libdatadog.path_to_crashtracking_receiver_binary, + ld_library_path: ::Libdatadog.ld_library_path, + logger: logger, + } + end + + subject(:crashtracker) { described_class.new(**crashtracker_options) } + + describe '#start' do + context 'when _native_start_or_update_on_fork raises an exception' do + it 'logs the exception' do + expect(described_class).to receive(:_native_start_or_update_on_fork) { raise 'Test failure' } + expect(logger).to receive(:error).with(/Failed to start crash tracking: Test failure/) + + crashtracker.start + end + end + + it 'starts the crash tracker' do + crashtracker.start + + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to_not be_empty + + crashtracker.stop + end + + context 'when calling start multiple times in a row' do + it 'only starts the crash tracker once' do + 3.times { crashtracker.start } + + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + + crashtracker.stop + end + end + + context 'when called in a fork' do + it 'starts a second crash tracker for the fork' do + crashtracker.start + + expect_in_fork do + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 2 + + crashtracker.stop + end + + crashtracker.stop + end + end + end + + describe '#stop' do + context 'when _native_stop_crashtracker raises an exception' do + it 'logs the exception' do + expect(described_class).to receive(:_native_stop) { raise 'Test failure' } + expect(logger).to receive(:error).with(/Failed to stop crash tracking: Test failure/) + + crashtracker.stop + end + end + + it 'stops the crash tracker' do + crashtracker.start + + crashtracker.stop + + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty + end + end + + context 'integration testing' do + shared_context 'HTTP server' do + let(:server) do + WEBrick::HTTPServer.new( + Port: 0, + Logger: log, + AccessLog: access_log, + StartCallback: -> { init_signal.push(1) } + ) + end + let(:hostname) { '127.0.0.1' } + let(:log) { WEBrick::Log.new(StringIO.new, WEBrick::Log::WARN) } + let(:access_log_buffer) { StringIO.new } + let(:access_log) { [[access_log_buffer, WEBrick::AccessLog::COMBINED_LOG_FORMAT]] } + let(:server_proc) do + proc do |req, res| + messages << req.tap { req.body } # Read body, store message before socket closes. + res.body = '{}' + end + end + let(:init_signal) { Queue.new } + + let(:messages) { [] } + + before do + server.mount_proc('/', &server_proc) + @server_thread = Thread.new { server.start } + init_signal.pop + end + + after do + unless RSpec.current_example.skipped? + # When the test is skipped, server has not been initialized and @server_thread would be nil; thus we only + # want to touch them when the test actually run, otherwise we would cause the server to start (incorrectly) + # and join to be called on a nil @server_thread + server.shutdown + @server_thread.join + end + end + end + + include_context 'HTTP server' + + let(:request) { messages.first } + let(:port) { server[:Port] } + + let(:agent_base_url) { "http://#{hostname}:#{port}" } + + [:fiddle, :signal].each do |trigger| + it "reports crashes via http when app crashes with #{trigger}" do + fork_expectations = proc do |status:, stdout:, stderr:| + expect(Signal.signame(status.termsig)).to eq('SEGV').or eq('ABRT') + expect(stderr).to include('[BUG] Segmentation fault') + end + + expect_in_fork(fork_expectations: fork_expectations) do + crashtracker.start + + if trigger == :fiddle + Fiddle.free(42) + else + Process.kill('SEGV', Process.pid) + end + end + + crash_report = JSON.parse(request.body, symbolize_names: true)[:payload].first + + expect(crash_report[:stack_trace]).to_not be_empty + expect(crash_report[:tags]).to include('signum:11', 'signame:SIGSEGV') + + crash_report_message = JSON.parse(crash_report[:message], symbolize_names: true) + + expect(crash_report_message[:metadata]).to include( + profiling_library_name: 'dd-trace-rb', + profiling_library_version: Datadog::VERSION::STRING, + family: 'ruby', + tags: ['tag1:value1', 'tag2:value2'], + ) + expect(crash_report_message[:files][:'/proc/self/maps']).to_not be_empty + expect(crash_report_message[:os_info]).to_not be_empty + end + end + + context do + it do + allow(described_class).to receive(:_native_start_or_update_on_fork) + + crashtracker = build_crashtracker(agent_base_url: 'http://localhost:6001').tap(&:start) + + fork_expectations = proc do + expect(described_class).to have_received(:_native_start_or_update_on_fork).with( + hash_including( + action: :start, + exporter_configuration: [:agent, 'http://localhost:6001'] + ) + ) + end + + expect_in_fork(fork_expectations: fork_expectations) do + allow(described_class).to receive(:_native_start_or_update_on_fork) + build_crashtracker(agent_base_url: 'http://localhost:6002').tap(&:start) + end + + crashtracker.stop + end + + def build_crashtracker(options) + described_class.new( + agent_base_url: options[:agent_base_url], + tags: {}, + path_to_crashtracking_receiver_binary: Libdatadog.path_to_crashtracking_receiver_binary, + ld_library_path: Libdatadog.ld_library_path, + logger: Logger.new($stdout), + ) + end + end + end + end + end diff --git a/spec/datadog/core/crashtracking/tag_builder_spec.rb b/spec/datadog/core/crashtracking/tag_builder_spec.rb new file mode 100644 index 00000000000..6f60b35401f --- /dev/null +++ b/spec/datadog/core/crashtracking/tag_builder_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' +require 'datadog/core/crashtracking/tag_builder' + +RSpec.describe Datadog::Core::Crashtracking::TagBuilder do + describe '.call' do + let(:settings) { Datadog::Core::Configuration::Settings.new } + + subject(:call) { described_class.call(settings) } + + it 'returns a hash with the tags to be attached to a profile' do + expect(call).to include( + 'host' => Datadog::Core::Environment::Socket.hostname, + 'language' => 'ruby', + 'process_id' => Process.pid.to_s, + 'profiler_version' => start_with('2.'), + 'runtime' => 'ruby', + 'runtime_engine' => RUBY_ENGINE, + 'runtime-id' => Datadog::Core::Environment::Identity.id, + 'runtime_platform' => RUBY_PLATFORM, + 'runtime_version' => RUBY_VERSION, + ) + end + + describe 'unified service tagging' do + [:env, :service, :version].each do |tag| + context "when a #{tag} is defined" do + before do + settings.send("#{tag}=".to_sym, 'expected_value') + end + + it 'includes it as a tag' do + expect(call).to include(tag.to_s => 'expected_value') + end + end + + context "when #{tag} is nil" do + before do + settings.send("#{tag}=".to_sym, nil) + end + + it do + expect(call.keys).to_not include(tag.to_s) + end + end + end + end + + it 'includes the provided user tags' do + settings.tags = { 'foo' => 'bar' } + + expect(call).to include('foo' => 'bar') + end + + context 'when there is a conflict between user and metadata tags' do + it 'overrides the user-provided tags' do + settings.tags = { 'foo' => 'bar', 'language' => 'python' } + + expect(call).to include('foo' => 'bar', 'language' => 'ruby') + end + end + + context 'when user tag keys and values are not strings' do + it 'encodes them as strings' do + settings.tags = { :symbol_key => :symbol_value, nil => 'nil key', 'nil value' => nil, 12 => 34 } + + expect(call).to include('symbol_key' => 'symbol_value', '' => 'nil key', 'nil value' => '', '12' => '34') + end + end + + context 'when tagging key or value is not utf-8' do + it 'converts them to utf-8' do + settings.tags = { 'ascii-key'.encode(Encoding::ASCII) => 'ascii-value'.encode(Encoding::ASCII) } + + result = call + + result.each do |key, value| + expect([key, value]).to all(have_attributes(encoding: Encoding::UTF_8)) + end + expect(result).to include('ascii-key' => 'ascii-value') + end + end + + describe 'source code integration' do + context 'when git environment is available' do + before do + allow(Datadog::Core::Environment::Git).to receive(:git_repository_url).and_return( + 'git_repository_url' + ) + allow(Datadog::Core::Environment::Git).to receive(:git_commit_sha).and_return('git_commit_sha') + end + + it 'includes the git repository URL and commit SHA' do + expect(call).to include( + 'git.repository_url' => 'git_repository_url', 'git.commit.sha' => 'git_commit_sha' + ) + end + end + + context 'when git environment is not available' do + before do + allow(Datadog::Core::Environment::Git).to receive(:git_repository_url).and_return(nil) + allow(Datadog::Core::Environment::Git).to receive(:git_commit_sha).and_return(nil) + end + + it 'includes the git repository URL and commit SHA' do + expect(call).to_not include('git.repository_url', 'git.commit.sha') + end + end + end + end +end diff --git a/spec/datadog/core/telemetry/event_spec.rb b/spec/datadog/core/telemetry/event_spec.rb index b8eee1f48d8..da44bd2d0ae 100644 --- a/spec/datadog/core/telemetry/event_spec.rb +++ b/spec/datadog/core/telemetry/event_spec.rb @@ -42,10 +42,7 @@ def contain_configuration(*array) appsec: { enabled: false, }, - profiler: { - enabled: false, - error: anything, - }, + profiler: hash_including(enabled: false), }, configuration: contain_configuration( ['DD_AGENT_HOST', '1.2.3.4'], diff --git a/spec/datadog/profiling/component_spec.rb b/spec/datadog/profiling/component_spec.rb index 8b72d3aaf0a..078f7c8d8bf 100644 --- a/spec/datadog/profiling/component_spec.rb +++ b/spec/datadog/profiling/component_spec.rb @@ -18,7 +18,11 @@ let(:tracer) { instance_double(Datadog::Tracing::Tracer) } subject(:build_profiler_component) do - described_class.build_profiler_component(settings: settings, agent_settings: agent_settings, optional_tracer: tracer) + described_class.build_profiler_component( + settings: settings, + agent_settings: agent_settings, + optional_tracer: tracer + ) end context 'when profiling is not supported' do @@ -391,8 +395,7 @@ it 'sets up the Profiler with the CpuAndWallTimeWorker collector' do expect(Datadog::Profiling::Profiler).to receive(:new).with( worker: instance_of(Datadog::Profiling::Collectors::CpuAndWallTimeWorker), - scheduler: anything, - optional_crashtracker: anything, + scheduler: anything ) build_profiler_component @@ -536,78 +539,6 @@ end end - context 'when crash tracking is disabled' do - before { settings.profiling.advanced.experimental_crash_tracking_enabled = false } - - it 'does not initialize the crash tracker' do - expect(Datadog::Profiling::Crashtracker).to_not receive(:new) - - build_profiler_component - end - end - - context 'when crash tracking is enabled' do - it 'initializes the crash tracker' do - 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 - end - - context 'when a custom transport is provided' do - let(:custom_transport) { double('Custom transport') } - - before do - settings.profiling.exporter.transport = custom_transport - allow(Datadog.logger).to receive(:debug) - end - - it 'debug logs that crash tracking will not be enabled' do - expect(Datadog.logger).to receive(:debug).with(/Cannot enable profiling crash tracking/) - - build_profiler_component - end - - it 'does not initialize the crash tracker' do - expect(Datadog::Profiling::Crashtracker).to_not receive(:new) - - build_profiler_component - end - end - - context 'when there was a libdatadog_api failure during load' do - before do - allow(Datadog.logger).to receive(:debug) - stub_const('Datadog::Profiling::Crashtracker::LIBDATADOG_API_FAILURE', 'simulated load failure') - end - - it 'debug logs that crash tracking will not be enabled' do - expect(Datadog.logger).to receive(:debug).with(/Cannot enable crashtracking: simulated load failure/) - - build_profiler_component - end - - it 'does not initialize the crash tracker' do - expect(Datadog::Profiling::Crashtracker).to_not receive(:new) - - build_profiler_component - end - end - - it 'initializes the profiler instance with the crash tracker' do - expect(Datadog::Profiling::Profiler).to receive(:new).with( - worker: anything, - scheduler: anything, - optional_crashtracker: instance_of(Datadog::Profiling::Crashtracker), - ) - - build_profiler_component - end - end - describe 'dir interruption workaround' do let(:no_signals_workaround_enabled) { false } diff --git a/spec/datadog/profiling/crashtracker_spec.rb b/spec/datadog/profiling/crashtracker_spec.rb deleted file mode 100644 index 32030fa4180..00000000000 --- a/spec/datadog/profiling/crashtracker_spec.rb +++ /dev/null @@ -1,221 +0,0 @@ -require 'datadog/profiling/spec_helper' -require 'datadog/profiling/crashtracker' - -require 'webrick' -require 'fiddle' - -RSpec.describe Datadog::Profiling::Crashtracker do - before do - skip_if_profiling_not_supported(self) - - # No crash tracker process should still be running at the start of each testcase - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty - end - - after do - # No crash tracker process should still be running at the end of each testcase - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty - end - - let(:exporter_configuration) { [:agent, 'http://localhost:6006'] } - - let(:crashtracker_options) do - { - exporter_configuration: exporter_configuration, - tags: { 'tag1' => 'value1', 'tag2' => 'value2' }, - upload_timeout_seconds: 123, - } - end - - subject(:crashtracker) { described_class.new(**crashtracker_options) } - - describe '#start' do - subject(:start) { crashtracker.start } - - context 'when _native_start_or_update_on_fork raises an exception' do - it 'logs the exception' do - expect(described_class).to receive(:_native_start_or_update_on_fork) { raise 'Test failure' } - expect(Datadog.logger).to receive(:error).with(/Failed to start crash tracking: Test failure/) - - start - end - end - - context 'when path_to_crashtracking_receiver_binary is nil' do - 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/) - - start - end - end - - context 'when ld_library_path is nil' do - 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/) - - start - end - end - - it 'starts the crash tracker' do - start - - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to_not be_empty - - crashtracker.stop - end - - context 'when calling start multiple times in a row' do - it 'only starts the crash tracker once' do - 3.times { crashtracker.start } - - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - - crashtracker.stop - end - end - - context 'when upload_timeout_seconds is not an Integer' do - let(:crashtracker_options) { { **super(), upload_timeout_seconds: 12.34 } } - - it 'converts it to an Integer before calling _native_start_or_update_on_fork' do - expect(described_class) - .to receive(:_native_start_or_update_on_fork).with(hash_including(upload_timeout_seconds: 12)) - - start - end - end - end - - describe '#reset_after_fork' do - subject(:reset_after_fork) { crashtracker.reset_after_fork } - - context 'when called in a fork' do - before { crashtracker.start } - after { crashtracker.stop } - - it 'starts a second crash tracker for the fork' do - expect_in_fork do - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - - crashtracker.reset_after_fork - - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 2 - - crashtracker.stop - - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - end - end - end - end - - describe '#stop' do - subject(:stop) { crashtracker.stop } - - context 'when _native_stop_crashtracker raises an exception' do - it 'logs the exception' do - expect(described_class).to receive(:_native_stop) { raise 'Test failure' } - expect(Datadog.logger).to receive(:error).with(/Failed to stop crash tracking: Test failure/) - - stop - end - end - - it 'stops the crash tracker' do - crashtracker.start - - stop - - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty - end - end - - context 'integration testing' do - shared_context 'HTTP server' do - let(:server) do - WEBrick::HTTPServer.new( - Port: 0, - Logger: log, - AccessLog: access_log, - StartCallback: -> { init_signal.push(1) } - ) - end - let(:hostname) { '127.0.0.1' } - let(:log) { WEBrick::Log.new(StringIO.new, WEBrick::Log::WARN) } - let(:access_log_buffer) { StringIO.new } - let(:access_log) { [[access_log_buffer, WEBrick::AccessLog::COMBINED_LOG_FORMAT]] } - let(:server_proc) do - proc do |req, res| - messages << req.tap { req.body } # Read body, store message before socket closes. - res.body = '{}' - end - end - let(:init_signal) { Queue.new } - - let(:messages) { [] } - - before do - server.mount_proc('/', &server_proc) - @server_thread = Thread.new { server.start } - init_signal.pop - end - - after do - unless RSpec.current_example.skipped? - # When the test is skipped, server has not been initialized and @server_thread would be nil; thus we only - # want to touch them when the test actually run, otherwise we would cause the server to start (incorrectly) - # and join to be called on a nil @server_thread - server.shutdown - @server_thread.join - end - end - end - - include_context 'HTTP server' - - let(:request) { messages.first } - let(:port) { server[:Port] } - - let(:exporter_configuration) { [:agent, "http://#{hostname}:#{port}"] } - - [:fiddle, :signal].each do |trigger| - it "reports crashes via http when app crashes with #{trigger}" do - fork_expectations = proc do |status:, stdout:, stderr:| - expect(Signal.signame(status.termsig)).to eq('SEGV').or eq('ABRT') - expect(stderr).to include('[BUG] Segmentation fault') - end - - expect_in_fork(fork_expectations: fork_expectations) do - crashtracker.start - - if trigger == :fiddle - Fiddle.free(42) - else - Process.kill('SEGV', Process.pid) - end - end - - crash_report = JSON.parse(request.body, symbolize_names: true)[:payload].first - - expect(crash_report[:stack_trace]).to_not be_empty - expect(crash_report[:tags]).to include('signum:11', 'signame:SIGSEGV') - - crash_report_message = JSON.parse(crash_report[:message], symbolize_names: true) - - expect(crash_report_message[:metadata]).to include( - profiling_library_name: 'dd-trace-rb', - profiling_library_version: Datadog::VERSION::STRING, - family: 'ruby', - tags: ['tag1:value1', 'tag2:value2'], - ) - expect(crash_report_message[:files][:'/proc/self/maps']).to_not be_empty - expect(crash_report_message[:os_info]).to_not be_empty - end - end - end -end diff --git a/spec/datadog/profiling/profiler_spec.rb b/spec/datadog/profiling/profiler_spec.rb index ccdbf7b69ff..58988c1614b 100644 --- a/spec/datadog/profiling/profiler_spec.rb +++ b/spec/datadog/profiling/profiler_spec.rb @@ -7,12 +7,11 @@ before { skip_if_profiling_not_supported(self) } subject(:profiler) do - described_class.new(worker: worker, scheduler: scheduler, optional_crashtracker: optional_crashtracker) + described_class.new(worker: worker, scheduler: scheduler) end let(:worker) { instance_double(Datadog::Profiling::Collectors::CpuAndWallTimeWorker) } let(:scheduler) { instance_double(Datadog::Profiling::Scheduler) } - let(:optional_crashtracker) { nil } describe '#start' do subject(:start) { profiler.start } @@ -24,19 +23,6 @@ start end - context 'when a crash tracker instance is provided' do - let(:optional_crashtracker) { instance_double(Datadog::Profiling::Crashtracker) } - - it 'signals the crash tracker to start before other components' do - expect(optional_crashtracker).to receive(:start).ordered - - expect(worker).to receive(:start).ordered - expect(scheduler).to receive(:start).ordered - - start - end - end - context 'when called after a fork' do before { skip('Spec requires Ruby VM supporting fork') unless PlatformHelpers.supports_fork? } @@ -53,26 +39,6 @@ start end end - - context 'when a crash tracker instance is provided' do - let(:optional_crashtracker) { instance_double(Datadog::Profiling::Crashtracker) } - - it 'resets the crash tracker before other coponents, as well as restarts it before other components' do - profiler # make sure instance is created in parent, so it detects the forking - - expect_in_fork do - expect(optional_crashtracker).to receive(:reset_after_fork).ordered - expect(worker).to receive(:reset_after_fork).ordered - expect(scheduler).to receive(:reset_after_fork).ordered - - expect(optional_crashtracker).to receive(:start).ordered - expect(worker).to receive(:start).ordered - expect(scheduler).to receive(:start).ordered - - start - end - end - end end end @@ -87,26 +53,11 @@ shutdown! end - - context 'when a crash tracker instance is provided' do - let(:optional_crashtracker) { instance_double(Datadog::Profiling::Crashtracker) } - - it 'signals the crash tracker to stop, after other components have stopped' do - expect(worker).to receive(:stop).ordered - allow(scheduler).to receive(:enabled=) - expect(scheduler).to receive(:stop).ordered - - expect(optional_crashtracker).to receive(:stop).ordered - - shutdown! - end - end end describe 'Component failure handling' do let(:worker) { instance_double(Datadog::Profiling::Collectors::CpuAndWallTimeWorker, start: nil) } let(:scheduler) { instance_double(Datadog::Profiling::Scheduler, start: nil) } - let(:optional_crashtracker) { instance_double(Datadog::Profiling::Crashtracker, start: nil) } before { allow(Datadog.logger).to receive(:warn) } @@ -144,12 +95,6 @@ worker_on_failure end - - it 'does not stop the crashtracker' do - expect(optional_crashtracker).to_not receive(:stop) - - worker_on_failure - end end context 'when the scheduler failed' do @@ -177,12 +122,6 @@ scheduler_on_failure end - - it 'does not stop the crashtracker' do - expect(optional_crashtracker).to_not receive(:stop) - - scheduler_on_failure - end end context 'when unknown component failed' do