From e1f642462972104326a116f4026b466b780e17cb Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Tue, 6 Aug 2024 11:26:58 +0200 Subject: [PATCH 01/16] 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 4a5bd8d4574..9b8c07660e8 100644 --- a/Rakefile +++ b/Rakefile @@ -74,7 +74,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| @@ -170,6 +170,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 526c1aa6898..114e781030f 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 44504214c64..440d690d068 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 5e4e57bc089..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 worker.reset_after_fork scheduler.reset_after_fork end - optional_crashtracker&.start 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 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 From 6dafb7a50fd57ade6ca46b096a60861b49b85b09 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Tue, 13 Aug 2024 11:56:22 +0200 Subject: [PATCH 02/16] Fix stale crashtracker --- lib/datadog/core/configuration/components.rb | 3 +- lib/datadog/core/crashtracking/component.rb | 7 +- .../core/crashtracking/component_spec.rb | 151 ++++++++++++------ 3 files changed, 108 insertions(+), 53 deletions(-) diff --git a/lib/datadog/core/configuration/components.rb b/lib/datadog/core/configuration/components.rb index ac999fb2825..254b0165147 100644 --- a/lib/datadog/core/configuration/components.rb +++ b/lib/datadog/core/configuration/components.rb @@ -82,6 +82,7 @@ def build_crashtracker(settings, agent_settings, logger:) :runtime_metrics, :telemetry, :tracer, + :crashtracker, :appsec def initialize(settings) @@ -95,7 +96,7 @@ 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) + @crashtracker = self.class.build_crashtracker(settings, agent_settings, logger: @logger) @profiler, profiler_logger_extra = Datadog::Profiling::Component.build_profiler_component( settings: settings, diff --git a/lib/datadog/core/crashtracking/component.rb b/lib/datadog/core/crashtracking/component.rb index 6ebf389b9d1..82cee394957 100644 --- a/lib/datadog/core/crashtracking/component.rb +++ b/lib/datadog/core/crashtracking/component.rb @@ -68,11 +68,16 @@ def start 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) + # Must NOT reference `self` here, as it become stale after fork + Datadog.send(:components).crashtracker&.update_on_fork end end end + def update_on_fork + start_or_update_on_fork(action: :update_on_fork) + end + def stop self.class._native_stop logger.debug('Crash tracking stopped successfully') diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index 8d261608ac9..8a689efd884 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -94,34 +94,19 @@ end context 'instance methods' do - before do - # No crash tracker process should still be running at the start of each testcase + # No crash tracker process should still be running at the start of each testcase + around do |example| 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 + example.run 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 + logger = Logger.new($stdout) + crashtracker = build_crashtracker(logger: logger) + 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/) @@ -130,34 +115,38 @@ end it 'starts the crash tracker' do + crashtracker = build_crashtracker + crashtracker.start wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to_not be_empty - crashtracker.stop + tear_down! end context 'when calling start multiple times in a row' do it 'only starts the crash tracker once' do + crashtracker = build_crashtracker + 3.times { crashtracker.start } wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - crashtracker.stop + tear_down! end end - context 'when called in a fork' do + context 'when forked' do it 'starts a second crash tracker for the fork' do + crashtracker = build_crashtracker + crashtracker.start expect_in_fork do wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 2 - - crashtracker.stop end - crashtracker.stop + tear_down! end end end @@ -165,6 +154,9 @@ 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' } expect(logger).to receive(:error).with(/Failed to stop crash tracking: Test failure/) @@ -173,14 +165,67 @@ end it 'stops the crash tracker' do + crashtracker = build_crashtracker + crashtracker.start + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to eq 1 + crashtracker.stop wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty end end + describe '#update_on_fork' 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_start_or_update_on_fork) { raise 'Test failure' } + expect(logger).to receive(:error).with(/Failed to update_on_fork crash tracking: Test failure/) + + crashtracker.update_on_fork + end + end + + it 'update_on_fork 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.update_on_fork + end + + it 'updates existing crash tracking process after started' do + crashtracker = build_crashtracker + + crashtracker.start + crashtracker.update_on_fork + + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + + tear_down! + end + + context 'when multiple instances' do + it 'updates existing crash tracking process after started' do + crashtracker = build_crashtracker + crashtracker.start + + another_crashtracker = build_crashtracker + another_crashtracker.update_on_fork + + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + + tear_down! + end + end + end + context 'integration testing' do shared_context 'HTTP server' do let(:server) do @@ -237,7 +282,8 @@ end expect_in_fork(fork_expectations: fork_expectations) do - crashtracker.start + crash_tracker = build_crashtracker(agent_base_url: agent_base_url) + crash_tracker.start if trigger == :fiddle Fiddle.free(42) @@ -264,39 +310,42 @@ end end - context do - it do + context 'when forked' do + it 'ensures the latest configuration applied' do allow(described_class).to receive(:_native_start_or_update_on_fork) - crashtracker = build_crashtracker(agent_base_url: 'http://localhost:6001').tap(&:start) + Datadog.configure do |c| + c.agent.host = 'example.com' + end + + Datadog.configure do |c| + c.agent.host = 'google.com' + end - fork_expectations = proc do + expect_in_fork do expect(described_class).to have_received(:_native_start_or_update_on_fork).with( hash_including( - action: :start, - exporter_configuration: [:agent, 'http://localhost:6001'] + action: :update_on_fork, + exporter_configuration: [:agent, 'http://google.com:9126/'], ) ) 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 + + def build_crashtracker(options = {}) + described_class.new( + agent_base_url: options[:agent_base_url] || 'http://localhost:6006', + tags: options[:tags] || { 'tag1' => 'value1', 'tag2' => 'value2' }, + path_to_crashtracking_receiver_binary: Libdatadog.path_to_crashtracking_receiver_binary, + ld_library_path: Libdatadog.ld_library_path, + logger: options[:logger] || Logger.new($stdout), + ) + end + + def tear_down! + described_class._native_stop + end end From 1788fe81bc2f4b1469f364b82ea0567fad6bb704 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Fri, 9 Aug 2024 13:42:16 +0200 Subject: [PATCH 03/16] Install libdatadog before datadog --- .gitlab/install_datadog_deps.rb | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/.gitlab/install_datadog_deps.rb b/.gitlab/install_datadog_deps.rb index 3678328cd57..32af8fef2a1 100755 --- a/.gitlab/install_datadog_deps.rb +++ b/.gitlab/install_datadog_deps.rb @@ -63,8 +63,21 @@ puts gem_version_mapping -gem_version_mapping.each do |gem, version| - env = {} +env = { + 'GEM_HOME' => versioned_path.to_s, + # Install `datadog` gem locally without its profiling native extension + 'DD_PROFILING_NO_EXTENSION' => 'true', +} + +[ + 'debase-ruby_core_source', + 'ffi', + 'libddwaf', + 'msgpack', + 'libdatadog', # libdatadog MUST be installed before datadog + 'datadog', +].each do |gem| + version = gem_version_mapping.fetch(gem) gem_install_cmd = "gem install #{gem} "\ "--version #{version} "\ @@ -73,19 +86,13 @@ case gem when 'ffi' - gem_install_cmd << "--install-dir #{versioned_path} " # Install `ffi` gem with its built-in `libffi` native extension instead of using system's `libffi` gem_install_cmd << '-- --disable-system-libffi ' when 'datadog' - # Install `datadog` gem locally without its profiling native extension - env['DD_PROFILING_NO_EXTENSION'] = 'true' gem_install_cmd = "gem install --local #{ENV.fetch('DATADOG_GEM_LOCATION')} "\ '--no-document '\ '--ignore-dependencies '\ - "--install-dir #{versioned_path} " - else - gem_install_cmd << "--install-dir #{versioned_path} " end puts "Execute: #{gem_install_cmd}" From 2ee80c47a105251b37bd9ad2c31d94b550d6a455 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Tue, 13 Aug 2024 16:39:21 +0200 Subject: [PATCH 04/16] Change send `library_version` instead of `profiler_version` --- lib/datadog/core/crashtracking/tag_builder.rb | 2 +- spec/datadog/core/crashtracking/tag_builder_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/datadog/core/crashtracking/tag_builder.rb b/lib/datadog/core/crashtracking/tag_builder.rb index 0033deb73d8..0f1dbaaa98c 100644 --- a/lib/datadog/core/crashtracking/tag_builder.rb +++ b/lib/datadog/core/crashtracking/tag_builder.rb @@ -15,7 +15,7 @@ def self.call(settings) 'host' => Environment::Socket.hostname, 'language' => Environment::Identity.lang, 'process_id' => Process.pid.to_s, - 'profiler_version' => Environment::Identity.gem_datadog_version, + 'library_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, diff --git a/spec/datadog/core/crashtracking/tag_builder_spec.rb b/spec/datadog/core/crashtracking/tag_builder_spec.rb index 6f60b35401f..f75dcf37d3c 100644 --- a/spec/datadog/core/crashtracking/tag_builder_spec.rb +++ b/spec/datadog/core/crashtracking/tag_builder_spec.rb @@ -12,7 +12,7 @@ 'host' => Datadog::Core::Environment::Socket.hostname, 'language' => 'ruby', 'process_id' => Process.pid.to_s, - 'profiler_version' => start_with('2.'), + 'library_version' => start_with('2.'), 'runtime' => 'ruby', 'runtime_engine' => RUBY_ENGINE, 'runtime-id' => Datadog::Core::Environment::Identity.id, From 6cb7c4c01bc87b445f6f08bd5bdf828cae5b6b21 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 11:17:33 +0200 Subject: [PATCH 05/16] Improve at_fork_monkey_patch.rbs --- sig/datadog/core/utils/at_fork_monkey_patch.rbs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sig/datadog/core/utils/at_fork_monkey_patch.rbs b/sig/datadog/core/utils/at_fork_monkey_patch.rbs index 04036f322b0..274a2cfb601 100644 --- a/sig/datadog/core/utils/at_fork_monkey_patch.rbs +++ b/sig/datadog/core/utils/at_fork_monkey_patch.rbs @@ -4,20 +4,20 @@ module Datadog module AtForkMonkeyPatch AT_FORK_CHILD_BLOCKS: ::Array[untyped] - def self.supported?: () -> untyped + def self.supported?: () -> (false | true) def self.apply!: () -> (false | true) - def self.run_at_fork_blocks: (untyped stage) -> untyped + def self.run_at_fork_blocks: (Symbol stage) -> void - def self.at_fork: (untyped stage) ?{ () -> untyped } -> true + def self.at_fork: (Symbol stage) { () -> untyped } -> true module KernelMonkeyPatch def fork: () ?{ () -> untyped } -> untyped end module ProcessMonkeyPatch - def _fork: () -> untyped + def _fork: () -> Integer def daemon: (*untyped args) -> untyped end From 208ef6110a7866336cb3df337e71210dc0204c9e Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 11:48:52 +0200 Subject: [PATCH 06/16] Reduce tags --- lib/datadog/core/crashtracking/component.rb | 4 +++- lib/datadog/core/crashtracking/tag_builder.rb | 3 --- lib/datadog/profiling/component.rb | 5 +---- spec/datadog/core/crashtracking/tag_builder_spec.rb | 8 +++----- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/datadog/core/crashtracking/component.rb b/lib/datadog/core/crashtracking/component.rb index 82cee394957..07df30cd7f1 100644 --- a/lib/datadog/core/crashtracking/component.rb +++ b/lib/datadog/core/crashtracking/component.rb @@ -68,7 +68,9 @@ def start start_or_update_on_fork(action: :start) ONLY_ONCE.run do Utils::AtForkMonkeyPatch.at_fork(:child) do - # Must NOT reference `self` here, as it become stale after fork + # Must NOT reference `self` here, as only the first instance will + # be captured by the ONLY_ONCE and we want to pick the latest active one + # (which may have different tags or agent config) Datadog.send(:components).crashtracker&.update_on_fork end end diff --git a/lib/datadog/core/crashtracking/tag_builder.rb b/lib/datadog/core/crashtracking/tag_builder.rb index 0f1dbaaa98c..31bce32b731 100644 --- a/lib/datadog/core/crashtracking/tag_builder.rb +++ b/lib/datadog/core/crashtracking/tag_builder.rb @@ -13,10 +13,7 @@ module TagBuilder def self.call(settings) hash = { 'host' => Environment::Socket.hostname, - 'language' => Environment::Identity.lang, 'process_id' => Process.pid.to_s, - 'library_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, diff --git a/lib/datadog/profiling/component.rb b/lib/datadog/profiling/component.rb index 440d690d068..cb184b53be3 100644 --- a/lib/datadog/profiling/component.rb +++ b/lib/datadog/profiling/component.rb @@ -73,10 +73,7 @@ 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) - profiler = Profiling::Profiler.new( - worker: worker, - scheduler: scheduler - ) + profiler = Profiling::Profiler.new(worker: worker, scheduler: scheduler) if dir_interruption_workaround_enabled?(settings, no_signals_workaround_enabled) Datadog::Profiling::Ext::DirMonkeyPatches.apply! diff --git a/spec/datadog/core/crashtracking/tag_builder_spec.rb b/spec/datadog/core/crashtracking/tag_builder_spec.rb index f75dcf37d3c..78802b39039 100644 --- a/spec/datadog/core/crashtracking/tag_builder_spec.rb +++ b/spec/datadog/core/crashtracking/tag_builder_spec.rb @@ -10,10 +10,7 @@ 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, - 'library_version' => start_with('2.'), - 'runtime' => 'ruby', 'runtime_engine' => RUBY_ENGINE, 'runtime-id' => Datadog::Core::Environment::Identity.id, 'runtime_platform' => RUBY_PLATFORM, @@ -53,9 +50,10 @@ 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' } + settings.tags = { 'foo' => 'bar', 'version' => '1.0.0' } + settings.version = '2.0.0' - expect(call).to include('foo' => 'bar', 'language' => 'ruby') + expect(call).to include('foo' => 'bar', 'version' => '2.0.0') end end From 029a0dc04f94be376e42538550bca30d775f5511 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 14:11:49 +0200 Subject: [PATCH 07/16] Add crashtracking task in the test matrix --- Matrixfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Matrixfile b/Matrixfile index 99cceec411f..4fc7a3b9ec6 100644 --- a/Matrixfile +++ b/Matrixfile @@ -7,6 +7,9 @@ '' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', 'core-old' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby' }, + 'crashtracking' => { + '' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ❌ jruby', + }, 'appsec:main' => { '' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby' }, From 1ebf56b24ad0c762b394b24dbef5a78bfd39f9db Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 14:15:58 +0200 Subject: [PATCH 08/16] Test multiple instance start --- .../core/crashtracking/component_spec.rb | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index 8a689efd884..dd87876f879 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -136,6 +136,20 @@ end end + context 'when multiple instances' do + it 'only starts the crash tracker once' do + crashtracker = build_crashtracker + crashtracker.start + + another_crashtracker = build_crashtracker + another_crashtracker.start + + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + + tear_down! + end + end + context 'when forked' do it 'starts a second crash tracker for the fork' do crashtracker = build_crashtracker @@ -210,20 +224,6 @@ tear_down! end - - context 'when multiple instances' do - it 'updates existing crash tracking process after started' do - crashtracker = build_crashtracker - crashtracker.start - - another_crashtracker = build_crashtracker - another_crashtracker.update_on_fork - - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - - tear_down! - end - end end context 'integration testing' do @@ -311,9 +311,15 @@ end context 'when forked' do + # This integration test coverages the case that + # the callback registered with `Utils::AtForkMonkeyPatch.at_fork` + # does not contain a stale instance of the crashtracker component. it 'ensures the latest configuration applied' do allow(described_class).to receive(:_native_start_or_update_on_fork) + # `Datadog.configure` to trigger crashtracking component reinstantiation, + # a callback is first registered with `Utils::AtForkMonkeyPatch.at_fork`, + # but not with the second `Datadog.configure` invokation. Datadog.configure do |c| c.agent.host = 'example.com' end From a1bc94fc894e3eecb5cb7749a8dcbcfbb11a6afd Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 14:54:14 +0200 Subject: [PATCH 09/16] Update rbs --- sig/datadog/core/crashtracking/agent_base_url.rbs | 2 +- sig/datadog/core/crashtracking/component.rbs | 2 +- sig/datadog/core/utils/at_fork_monkey_patch.rbs | 1 + spec/datadog/core/crashtracking/tag_builder_spec.rb | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sig/datadog/core/crashtracking/agent_base_url.rbs b/sig/datadog/core/crashtracking/agent_base_url.rbs index c83ba9f180d..2bdf7f07259 100644 --- a/sig/datadog/core/crashtracking/agent_base_url.rbs +++ b/sig/datadog/core/crashtracking/agent_base_url.rbs @@ -2,7 +2,7 @@ module Datadog module Core module Crashtracking module AgentBaseUrl - def self.resolve: (untyped) -> ::String? + def self.resolve: (Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings) -> ::String? end end end diff --git a/sig/datadog/core/crashtracking/component.rbs b/sig/datadog/core/crashtracking/component.rbs index 4c9fac3664c..05a636b4703 100644 --- a/sig/datadog/core/crashtracking/component.rbs +++ b/sig/datadog/core/crashtracking/component.rbs @@ -21,7 +21,7 @@ module Datadog def start: -> void - def reset_after_fork: -> void + def update_on_fork: -> void def stop: -> void diff --git a/sig/datadog/core/utils/at_fork_monkey_patch.rbs b/sig/datadog/core/utils/at_fork_monkey_patch.rbs index 274a2cfb601..b6ccdce0736 100644 --- a/sig/datadog/core/utils/at_fork_monkey_patch.rbs +++ b/sig/datadog/core/utils/at_fork_monkey_patch.rbs @@ -1,3 +1,4 @@ +# TODO: Fix and remove `ignore 'lib/datadog/core/utils/at_fork_monkey_patch.rb` from `Steep` module Datadog module Core module Utils diff --git a/spec/datadog/core/crashtracking/tag_builder_spec.rb b/spec/datadog/core/crashtracking/tag_builder_spec.rb index 78802b39039..a386fa578fb 100644 --- a/spec/datadog/core/crashtracking/tag_builder_spec.rb +++ b/spec/datadog/core/crashtracking/tag_builder_spec.rb @@ -7,7 +7,7 @@ subject(:call) { described_class.call(settings) } - it 'returns a hash with the tags to be attached to a profile' do + it 'returns a hash with the tags to be attached to a crash report' do expect(call).to include( 'host' => Datadog::Core::Environment::Socket.hostname, 'process_id' => Process.pid.to_s, From 9ac6f1e32cb29549279c2f0dfb835df521eb2a76 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 15:10:26 +0200 Subject: [PATCH 10/16] Improve logging --- lib/datadog/core/crashtracking/component.rb | 3 +-- spec/datadog/core/crashtracking/component_spec.rb | 8 ++++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/datadog/core/crashtracking/component.rb b/lib/datadog/core/crashtracking/component.rb index 07df30cd7f1..d2d1374b8a0 100644 --- a/lib/datadog/core/crashtracking/component.rb +++ b/lib/datadog/core/crashtracking/component.rb @@ -92,7 +92,6 @@ def stop 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], @@ -101,7 +100,7 @@ def start_or_update_on_fork(action:) tags_as_array: tags.to_a, upload_timeout_seconds: 1 ) - logger.debug("Crash tracking #{action} successful") + logger.debug("Crash tracking #{action} successfully") rescue => e logger.error("Failed to #{action} crash tracking: #{e.message}") end diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index dd87876f879..390d38f6312 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -10,7 +10,7 @@ let(:settings) { Datadog::Core::Configuration::Settings.new } let(:agent_settings) { double('agent_settings') } let(:logger) { Logger.new($stdout) } - let(:tags) { {} } + let(:tags) { { 'tag1' => 'value1' } } 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' } @@ -25,8 +25,9 @@ .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) .and_return(path_to_crashtracking_receiver_binary) + expect(logger).to_not receive(:warn) - component = double('component') + component = double(instance_double(described_class)) expect(described_class).to receive(:new).with( tags: tags, agent_base_url: agent_base_url, @@ -53,6 +54,7 @@ .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) .and_return(path_to_crashtracking_receiver_binary) + expect(logger).to receive(:warn).with(/cannot enable crash tracking/) expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil end @@ -70,6 +72,7 @@ .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) .and_return(path_to_crashtracking_receiver_binary) + expect(logger).to receive(:warn).with(/cannot enable crash tracking/) expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil end @@ -87,6 +90,7 @@ .and_return(ld_library_path) expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) .and_return(path_to_crashtracking_receiver_binary) + expect(logger).to receive(:warn).with(/cannot enable crash tracking/) expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil end From 51c88e28b1dbab12888dbda19ba67520131d42d2 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 16:00:04 +0200 Subject: [PATCH 11/16] Implement CrashtrackingHelpers --- Rakefile | 2 +- .../core/crashtracking/component_spec.rb | 2 +- spec/spec_helper.rb | 1 + spec/support/crashtracking_helpers.rb | 17 +++++++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 spec/support/crashtracking_helpers.rb diff --git a/Rakefile b/Rakefile index 9b8c07660e8..a71e9ea3259 100644 --- a/Rakefile +++ b/Rakefile @@ -79,7 +79,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:main) do |t, args| t.pattern = 'spec/**/*_spec.rb' - t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,auto_instrument,opentelemetry,profiling}/**/*_spec.rb,'\ + t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,auto_instrument,opentelemetry,profiling,crashtracking}/**/*_spec.rb,'\ ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb' t.rspec_opts = args.to_a.join(' ') end diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index 390d38f6312..e7d90f79f59 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -5,7 +5,7 @@ require 'fiddle' RSpec.describe Datadog::Core::Crashtracking::Component, - skip: Datadog::Core::Crashtracking::Component::LIBDATADOG_API_FAILURE do + skip: !CrashtrackingHelpers.supported? do describe '.build' do let(:settings) { Datadog::Core::Configuration::Settings.new } let(:agent_settings) { double('agent_settings') } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8add7625230..33980773165 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -38,6 +38,7 @@ require 'support/synchronization_helpers' require 'support/test_helpers' require 'support/tracer_helpers' +require 'support/crashtracking_helpers' begin # Ignore interpreter warnings from external libraries diff --git a/spec/support/crashtracking_helpers.rb b/spec/support/crashtracking_helpers.rb new file mode 100644 index 00000000000..dcd7291e57e --- /dev/null +++ b/spec/support/crashtracking_helpers.rb @@ -0,0 +1,17 @@ +require 'datadog/core/crashtracking/component' +require 'support/platform_helpers' + +module CrashtrackingHelpers + def self.supported? + # Only works with MRI on Linux + if PlatformHelpers.mri? && PlatformHelpers.linux? + if Datadog::Core::Crashtracking::Component::LIBDATADOG_API_FAILURE + raise " does not seem to be available: #{Datadog::Core::Crashtracking::Component::LIBDATADOG_API_FAILURE}. " \ + 'Try running `bundle exec rake compile` before running this test.' + end + true + else + false + end + end +end From 556c7051590dcd14f2c19856d9c4fff2df73b317 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 17:24:22 +0200 Subject: [PATCH 12/16] Raise exception when dependency not installed --- .gitlab/install_datadog_deps.rb | 4 +++- spec/datadog/release_gem_spec.rb | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.gitlab/install_datadog_deps.rb b/.gitlab/install_datadog_deps.rb index 32af8fef2a1..21f9118f144 100755 --- a/.gitlab/install_datadog_deps.rb +++ b/.gitlab/install_datadog_deps.rb @@ -77,7 +77,7 @@ 'libdatadog', # libdatadog MUST be installed before datadog 'datadog', ].each do |gem| - version = gem_version_mapping.fetch(gem) + version = gem_version_mapping.delete(gem) gem_install_cmd = "gem install #{gem} "\ "--version #{version} "\ @@ -106,6 +106,8 @@ end end +raise "#{gem_version_mapping.keys.join(',')} are not installed." if gem_version_mapping.any? + FileUtils.cd(versioned_path.join("extensions/#{Gem::Platform.local}"), verbose: true) do # Symlink those directories to be utilized by Ruby compiled with shared libraries FileUtils.ln_sf Gem.extension_api_version, ruby_api_version diff --git a/spec/datadog/release_gem_spec.rb b/spec/datadog/release_gem_spec.rb index ff01af02ae2..0eafbe5d967 100644 --- a/spec/datadog/release_gem_spec.rb +++ b/spec/datadog/release_gem_spec.rb @@ -92,7 +92,8 @@ file.unlink end - # Lib injection package pipeline should be updated to include the following gems + # Lib injection package pipeline should be updated to include the following gems, + # check `install_datadog_deps.rb` for details expect(gem_version_mapping.keys).to contain_exactly( # This list MUST NOT derive from the `gemspec.dependencies`, # since it is used to alarm when dependencies modified. From 016f5accfd6626a423bdcc0e2eabd88fe1381efc Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 14 Aug 2024 17:29:50 +0200 Subject: [PATCH 13/16] Use instance_double --- .../core/crashtracking/agent_base_url_spec.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spec/datadog/core/crashtracking/agent_base_url_spec.rb b/spec/datadog/core/crashtracking/agent_base_url_spec.rb index 99c73019906..86163c035fd 100644 --- a/spec/datadog/core/crashtracking/agent_base_url_spec.rb +++ b/spec/datadog/core/crashtracking/agent_base_url_spec.rb @@ -8,8 +8,8 @@ context 'when using HTTP adapter' do context 'when SSL is enabled' do let(:agent_settings) do - double( - 'agent_settings', + instance_double( + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, ssl: true, hostname: 'example.com', @@ -24,8 +24,8 @@ context 'when SSL is disabled' do let(:agent_settings) do - double( - 'agent_settings', + instance_double( + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, ssl: false, hostname: 'example.com', @@ -41,8 +41,8 @@ context 'when using UnixSocket adapter' do let(:agent_settings) do - double( - 'agent_settings', + instance_double( + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, adapter: Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER, uds_path: '/var/run/datadog.sock' ) @@ -54,7 +54,9 @@ end context 'when using unknownm adapter' do - let(:agent_settings) { double('agent_settings', adapter: 'unknown') } + let(:agent_settings) do + instance_double(Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, adapter: 'unknown') + end it 'returns nil' do expect(described_class.resolve(agent_settings)).to be_nil From 72499cd1a504bd0a7976f959b798aa5e126e243c Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Thu, 15 Aug 2024 11:01:26 +0200 Subject: [PATCH 14/16] Add `is_crash` tag --- lib/datadog/core/crashtracking/tag_builder.rb | 1 + spec/datadog/core/crashtracking/tag_builder_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/datadog/core/crashtracking/tag_builder.rb b/lib/datadog/core/crashtracking/tag_builder.rb index 31bce32b731..6feb64a8a84 100644 --- a/lib/datadog/core/crashtracking/tag_builder.rb +++ b/lib/datadog/core/crashtracking/tag_builder.rb @@ -23,6 +23,7 @@ def self.call(settings) 'version' => settings.version, 'git.repository_url' => Environment::Git.git_repository_url, 'git.commit.sha' => Environment::Git.git_commit_sha, + 'is_crash' => true }.compact # Make sure everything is an utf-8 string, to avoid encoding issues in downstream diff --git a/spec/datadog/core/crashtracking/tag_builder_spec.rb b/spec/datadog/core/crashtracking/tag_builder_spec.rb index a386fa578fb..ddb78ac5327 100644 --- a/spec/datadog/core/crashtracking/tag_builder_spec.rb +++ b/spec/datadog/core/crashtracking/tag_builder_spec.rb @@ -15,6 +15,7 @@ 'runtime-id' => Datadog::Core::Environment::Identity.id, 'runtime_platform' => RUBY_PLATFORM, 'runtime_version' => RUBY_VERSION, + 'is_crash' => 'true', ) end From 9682847b1b8048ed7870108842a368ab3c10e30b Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Thu, 15 Aug 2024 11:13:14 +0200 Subject: [PATCH 15/16] Minor style improvement --- lib/datadog/core/crashtracking/component.rb | 5 +- .../core/crashtracking/component_spec.rb | 539 +++++++++--------- 2 files changed, 272 insertions(+), 272 deletions(-) diff --git a/lib/datadog/core/crashtracking/component.rb b/lib/datadog/core/crashtracking/component.rb index d2d1374b8a0..b9849cf2a30 100644 --- a/lib/datadog/core/crashtracking/component.rb +++ b/lib/datadog/core/crashtracking/component.rb @@ -12,8 +12,9 @@ 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 + # NOTE: The crashtracker native state is a singleton; + # so even if you create multiple instances of `Crashtracking::Component` 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` diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index e7d90f79f59..912f4e758e5 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -4,358 +4,357 @@ require 'webrick' require 'fiddle' -RSpec.describe Datadog::Core::Crashtracking::Component, - skip: !CrashtrackingHelpers.supported? do - 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' } - 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) - expect(logger).to_not receive(:warn) - - component = double(instance_double(described_class)) - 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 +RSpec.describe Datadog::Core::Crashtracking::Component, skip: !CrashtrackingHelpers.supported? do + 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' } + 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) + expect(logger).to_not receive(:warn) + + component = instance_double(described_class) + 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(logger).to receive(:warn).with(/cannot enable crash tracking/) - - expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil - 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(logger).to receive(:warn).with(/cannot enable crash tracking/) + + 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(logger).to receive(:warn).with(/cannot enable crash tracking/) - - expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil - 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(logger).to receive(:warn).with(/cannot enable crash tracking/) + + 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(logger).to receive(:warn).with(/cannot enable crash tracking/) - - expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil - 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(logger).to receive(:warn).with(/cannot enable crash tracking/) + + expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil end end + end - context 'instance methods' do - # No crash tracker process should still be running at the start of each testcase - around do |example| - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty - example.run - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty - end + context 'instance methods' do + # No crash tracker process should still be running at the start of each testcase + around do |example| + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty + example.run + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty + end - 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) + 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' } - expect(logger).to receive(:error).with(/Failed to start crash tracking: Test failure/) + 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 + crashtracker.start end + end - it 'starts the crash tracker' do - crashtracker = build_crashtracker + it 'starts the crash tracker' do + crashtracker = build_crashtracker - crashtracker.start + crashtracker.start - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to_not be_empty + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to_not be_empty - tear_down! - end + tear_down! + end - context 'when calling start multiple times in a row' do - it 'only starts the crash tracker once' do - crashtracker = build_crashtracker + context 'when calling start multiple times in a row' do + it 'only starts the crash tracker once' do + crashtracker = build_crashtracker - 3.times { crashtracker.start } + 3.times { crashtracker.start } - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - tear_down! - end + tear_down! end + end - context 'when multiple instances' do - it 'only starts the crash tracker once' do - crashtracker = build_crashtracker - crashtracker.start + context 'when multiple instances' do + it 'only starts the crash tracker once' do + crashtracker = build_crashtracker + crashtracker.start - another_crashtracker = build_crashtracker - another_crashtracker.start + another_crashtracker = build_crashtracker + another_crashtracker.start - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - tear_down! - end + tear_down! end + end - context 'when forked' do - it 'starts a second crash tracker for the fork' do - crashtracker = build_crashtracker - - crashtracker.start + context 'when forked' do + it 'starts a second crash tracker for the fork' do + crashtracker = build_crashtracker - expect_in_fork do - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 2 - end + crashtracker.start - tear_down! + expect_in_fork do + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 2 end + + tear_down! end end + end - 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) + 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' } - expect(logger).to receive(:error).with(/Failed to stop crash tracking: Test failure/) + 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 + crashtracker.stop end + end - it 'stops the crash tracker' do - crashtracker = build_crashtracker + it 'stops the crash tracker' do + crashtracker = build_crashtracker - crashtracker.start + crashtracker.start - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to eq 1 + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to eq 1 - crashtracker.stop + crashtracker.stop - wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty - end + wait_for { `pgrep -f libdatadog-crashtracking-receiver` }.to be_empty end + end - describe '#update_on_fork' do - context 'when _native_stop_crashtracker raises an exception' do - it 'logs the exception' do - logger = Logger.new($stdout) - crashtracker = build_crashtracker(logger: logger) + describe '#update_on_fork' 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_start_or_update_on_fork) { raise 'Test failure' } - expect(logger).to receive(:error).with(/Failed to update_on_fork crash tracking: Test failure/) + expect(described_class).to receive(:_native_start_or_update_on_fork) { raise 'Test failure' } + expect(logger).to receive(:error).with(/Failed to update_on_fork crash tracking: Test failure/) - crashtracker.update_on_fork - end + crashtracker.update_on_fork end + end - it 'update_on_fork the crash tracker' do - expect(described_class).to receive(:_native_start_or_update_on_fork).with( - hash_including(action: :update_on_fork) - ) + it 'update_on_fork 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 - crashtracker.update_on_fork - end + crashtracker.update_on_fork + end - it 'updates existing crash tracking process after started' do - crashtracker = build_crashtracker + it 'updates existing crash tracking process after started' do + crashtracker = build_crashtracker - crashtracker.start - crashtracker.update_on_fork + crashtracker.start + crashtracker.update_on_fork - wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 + wait_for { `pgrep -f libdatadog-crashtracking-receiver`.lines.size }.to be 1 - tear_down! - end + tear_down! 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 + 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 - let(:init_signal) { Queue.new } + end + let(:init_signal) { Queue.new } - let(:messages) { [] } + let(:messages) { [] } - before do - server.mount_proc('/', &server_proc) - @server_thread = Thread.new { server.start } - init_signal.pop - end + 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 + 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' + include_context 'HTTP server' - let(:request) { messages.first } - let(:port) { server[:Port] } + let(:request) { messages.first } + let(:port) { server[:Port] } - let(:agent_base_url) { "http://#{hostname}:#{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 + [: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 - crash_tracker = build_crashtracker(agent_base_url: agent_base_url) - crash_tracker.start + expect_in_fork(fork_expectations: fork_expectations) do + crash_tracker = build_crashtracker(agent_base_url: agent_base_url) + crash_tracker.start - if trigger == :fiddle - Fiddle.free(42) - else - Process.kill('SEGV', Process.pid) - end + 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 + 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') + 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) + 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 + 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 'when forked' do - # This integration test coverages the case that - # the callback registered with `Utils::AtForkMonkeyPatch.at_fork` - # does not contain a stale instance of the crashtracker component. - it 'ensures the latest configuration applied' do - allow(described_class).to receive(:_native_start_or_update_on_fork) - - # `Datadog.configure` to trigger crashtracking component reinstantiation, - # a callback is first registered with `Utils::AtForkMonkeyPatch.at_fork`, - # but not with the second `Datadog.configure` invokation. - Datadog.configure do |c| - c.agent.host = 'example.com' - end + context 'when forked' do + # This integration test coverages the case that + # the callback registered with `Utils::AtForkMonkeyPatch.at_fork` + # does not contain a stale instance of the crashtracker component. + it 'ensures the latest configuration applied' do + allow(described_class).to receive(:_native_start_or_update_on_fork) + + # `Datadog.configure` to trigger crashtracking component reinstantiation, + # a callback is first registered with `Utils::AtForkMonkeyPatch.at_fork`, + # but not with the second `Datadog.configure` invokation. + Datadog.configure do |c| + c.agent.host = 'example.com' + end - Datadog.configure do |c| - c.agent.host = 'google.com' - end + Datadog.configure do |c| + c.agent.host = 'google.com' + end - expect_in_fork do - expect(described_class).to have_received(:_native_start_or_update_on_fork).with( - hash_including( - action: :update_on_fork, - exporter_configuration: [:agent, 'http://google.com:9126/'], - ) + expect_in_fork do + expect(described_class).to have_received(:_native_start_or_update_on_fork).with( + hash_including( + action: :update_on_fork, + exporter_configuration: [:agent, 'http://google.com:9126/'], ) - end + ) end end end end + end - def build_crashtracker(options = {}) - described_class.new( - agent_base_url: options[:agent_base_url] || 'http://localhost:6006', - tags: options[:tags] || { 'tag1' => 'value1', 'tag2' => 'value2' }, - path_to_crashtracking_receiver_binary: Libdatadog.path_to_crashtracking_receiver_binary, - ld_library_path: Libdatadog.ld_library_path, - logger: options[:logger] || Logger.new($stdout), - ) - end + def build_crashtracker(options = {}) + described_class.new( + agent_base_url: options[:agent_base_url] || 'http://localhost:6006', + tags: options[:tags] || { 'tag1' => 'value1', 'tag2' => 'value2' }, + path_to_crashtracking_receiver_binary: Libdatadog.path_to_crashtracking_receiver_binary, + ld_library_path: Libdatadog.ld_library_path, + logger: options[:logger] || Logger.new($stdout), + ) + end - def tear_down! - described_class._native_stop - end + def tear_down! + described_class._native_stop end +end From 6f3c5fe0e1ac3e801da5d09fd47c077753d6ff0c Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Thu, 15 Aug 2024 11:30:14 +0200 Subject: [PATCH 16/16] Fail pipeline when libdatadog.so is missing --- .gitlab/install_datadog_deps.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.gitlab/install_datadog_deps.rb b/.gitlab/install_datadog_deps.rb index 21f9118f144..397e805ca64 100755 --- a/.gitlab/install_datadog_deps.rb +++ b/.gitlab/install_datadog_deps.rb @@ -74,7 +74,7 @@ 'ffi', 'libddwaf', 'msgpack', - 'libdatadog', # libdatadog MUST be installed before datadog + 'libdatadog', # libdatadog MUST be installed before datadog to ensure libdatadog native extension is compiled 'datadog', ].each do |gem| version = gem_version_mapping.delete(gem) @@ -108,6 +108,12 @@ raise "#{gem_version_mapping.keys.join(',')} are not installed." if gem_version_mapping.any? +datadog_gem_path = versioned_path.join("gems/datadog-#{ENV.fetch('RUBY_PACKAGE_VERSION')}") +libdatadog_so_file = "libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}.so" +unless File.exist?("#{datadog_gem_path}/lib/#{libdatadog_so_file}") + raise "Missing #{libdatadog_so_file} in #{datadog_gem_path}." +end + FileUtils.cd(versioned_path.join("extensions/#{Gem::Platform.local}"), verbose: true) do # Symlink those directories to be utilized by Ruby compiled with shared libraries FileUtils.ln_sf Gem.extension_api_version, ruby_api_version