Skip to content

Commit

Permalink
Merge pull request #3844 from DataDog/ivoanjo/profiler-standardrb-todos
Browse files Browse the repository at this point in the history
[NO-TICKET] Apply more of standardrb autoformatter to profiler files
  • Loading branch information
ivoanjo authored Aug 14, 2024
2 parents a589a98 + 52c3b5a commit 7c67ce3
Show file tree
Hide file tree
Showing 24 changed files with 317 additions and 464 deletions.
3 changes: 3 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ f89961150051276ea21bbca3fa00bda9b3cf4f1d
6924556fc9e38c244e2ff3d429d581819f275ee9
aba860197ce4f708e917e27323884d8efe3692ca
464dd7177b72d6fe2c7f6e1aba0e4a6d3826f0d9
acee9c7f3d953551cc0f20ef60e5045432bcf7e6
6e4193c5bf3c2b948c91598c7a70bc34b59872fa
b1481b215d8b1bf33a1d53419d1b95ebd1a70877
133 changes: 2 additions & 131 deletions .standard_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,142 +44,13 @@ ignore:
- yard/**/**

# These profiling ignores are ALL going to be fixed in separate PRs
- lib/datadog/profiling/collectors/code_provenance.rb:
- lib/datadog/profiling/**/**:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb:
- spec/datadog/profiling/**/**:
- Style/StringLiterals
- Style/RedundantBegin
- lib/datadog/profiling/collectors/idle_sampling_helper.rb:
- Style/RedundantBegin
- Style/StringLiterals
- lib/datadog/profiling/collectors/info.rb:
- Style/StringLiterals
- lib/datadog/profiling/component.rb:
- Layout/SpaceInsideHashLiteralBraces
- Style/StringLiterals
- lib/datadog/profiling/crashtracker.rb:
- Style/StringLiterals
- Style/RedundantBegin
- lib/datadog/profiling/exporter.rb:
- Style/StringLiterals
- lib/datadog/profiling/ext.rb:
- Style/StringLiterals
- lib/datadog/profiling/ext/dir_monkey_patches.rb:
- Style/StringLiterals
- lib/datadog/profiling/ext/forking.rb:
- Style/StringLiterals
- lib/datadog/profiling/flush.rb:
- Style/StringLiterals
- lib/datadog/profiling/http_transport.rb:
- Style/StringLiterals
- lib/datadog/profiling/load_native_extension.rb:
- Style/StringLiterals
- lib/datadog/profiling/preload.rb:
- Style/StringLiterals
- lib/datadog/profiling/profiler.rb:
- Style/StringLiterals
- lib/datadog/profiling/scheduler.rb:
- Style/StringLiterals
- Style/RedundantBegin
- lib/datadog/profiling/tag_builder.rb:
- Style/StringLiterals
- lib/datadog/profiling/tasks/exec.rb:
- Style/StringLiterals
- lib/datadog/profiling/tasks/setup.rb:
- Style/StringLiterals
- Style/RedundantBegin
- lib/datadog/profiling.rb:
- Style/StringLiterals
- Style/RedundantBegin
- spec/datadog/profiling/collectors/code_provenance_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- Style/QuotedSymbols
- Style/RedundantBegin
- spec/datadog/profiling/collectors/discrete_dynamic_sampler_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/collectors/dynamic_sampling_rate_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/collectors/idle_sampling_helper_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/collectors/info_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/collectors/stack_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/collectors/thread_context_spec.rb:
- Style/StringLiterals
- Style/QuotedSymbols
- Style/RedundantBegin
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/component_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/crashtracker_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- Style/QuotedSymbols
- spec/datadog/profiling/exporter_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/ext/dir_monkey_patches_spec.rb:
- Style/StringLiterals
- Style/RedundantBegin
- spec/datadog/profiling/ext/forking_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/flush_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/http_transport_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- Style/RedundantBegin
- spec/datadog/profiling/load_native_extension_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/native_extension_helpers_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/native_extension_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/pprof/pprof_pb.rb:
- Style/StringLiterals
- spec/datadog/profiling/preload_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/profiler_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/scheduler_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/spec_helper.rb:
- Style/StringLiterals
- Style/QuotedSymbols
- spec/datadog/profiling/stack_recorder_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- Style/QuotedSymbols
- Style/RedundantBegin
- spec/datadog/profiling/tag_builder_spec.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- spec/datadog/profiling/tasks/exec_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/tasks/help_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/tasks/setup_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling/validate_benchmarks_spec.rb:
- Style/StringLiterals
- spec/datadog/profiling_spec.rb:
- Style/StringLiterals
- Style/RedundantBegin
- ext/datadog_profiling_loader/extconf.rb:
- Style/StringLiterals
- ext/datadog_profiling_native_extension/extconf.rb:
- Style/StringLiterals
- ext/datadog_profiling_native_extension/native_extension_helpers.rb:
- Style/StringLiterals
- Layout/SpaceInsideHashLiteralBraces
- Style/RedundantBegin
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.fail_install_if_missing_extension?
# system may not be supported.
module Supported
private_class_method def self.explain_issue(*reason, suggested:)
{ reason: reason, suggested: suggested }
{reason: reason, suggested: suggested}
end

def self.supported?
Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/profiling/collectors/code_provenance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def initialize(kind:, name:, version:, path:)
end

def to_json(arg = nil)
{ kind: @kind, name: @name, version: @version, paths: @paths }.to_json(arg)
{kind: @kind, name: @name, version: @version, paths: @paths}.to_json(arg)
end

def path
Expand Down
26 changes: 12 additions & 14 deletions lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,18 @@ def start(on_failure_proc: nil)
@idle_sampling_helper.start

@worker_thread = Thread.new do
begin
Thread.current.name = self.class.name

self.class._native_sampling_loop(self)

Datadog.logger.debug('CpuAndWallTimeWorker thread stopping cleanly')
rescue Exception => e # rubocop:disable Lint/RescueException
@failure_exception = e
Datadog.logger.warn(
'CpuAndWallTimeWorker thread error. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)
on_failure_proc&.call
end
Thread.current.name = self.class.name

self.class._native_sampling_loop(self)

Datadog.logger.debug('CpuAndWallTimeWorker thread stopping cleanly')
rescue Exception => e # rubocop:disable Lint/RescueException
@failure_exception = e
Datadog.logger.warn(
'CpuAndWallTimeWorker thread error. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)
on_failure_proc&.call
end
@worker_thread.name = self.class.name # Repeated from above to make sure thread gets named asap
@worker_thread.thread_variable_set(:fork_safe, true)
Expand Down
20 changes: 9 additions & 11 deletions lib/datadog/profiling/collectors/idle_sampling_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,17 @@ def start
self.class._native_reset(self)

@worker_thread = Thread.new do
begin
Thread.current.name = self.class.name
Thread.current.name = self.class.name

self.class._native_idle_sampling_loop(self)
self.class._native_idle_sampling_loop(self)

Datadog.logger.debug('IdleSamplingHelper thread stopping cleanly')
rescue Exception => e # rubocop:disable Lint/RescueException
@failure_exception = e
Datadog.logger.warn(
'IdleSamplingHelper thread error. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)
end
Datadog.logger.debug('IdleSamplingHelper thread stopping cleanly')
rescue Exception => e # rubocop:disable Lint/RescueException
@failure_exception = e
Datadog.logger.warn(
'IdleSamplingHelper thread error. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)
end
@worker_thread.name = self.class.name # Repeated from above to make sure thread gets named asap
@worker_thread.thread_variable_set(:fork_safe, true)
Expand Down
6 changes: 3 additions & 3 deletions lib/datadog/profiling/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Component
# * Code Hotspots panel in the trace viewer, as well as scoping a profile down to a span
# * Endpoint aggregation in the profiler UX, including normalization (resource per endpoint call)
def self.build_profiler_component(settings:, agent_settings:, optional_tracer:) # rubocop:disable Metrics/MethodLength
return [nil, { profiling_enabled: false }] unless settings.profiling.enabled
return [nil, {profiling_enabled: false}] unless settings.profiling.enabled

# Workaround for weird dependency direction: the Core::Configuration::Components class currently has a
# dependency on individual products, in this case the Profiler.
Expand All @@ -29,7 +29,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
# no-op if profiling is already loaded).
require_relative '../profiling'

return [nil, { profiling_enabled: false }] unless Profiling.supported?
return [nil, {profiling_enabled: false}] unless Profiling.supported?

# Activate forking extensions
Profiling::Tasks::Setup.new.run
Expand Down Expand Up @@ -80,7 +80,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
Datadog::Profiling::Ext::DirMonkeyPatches.apply!
end

[profiler, { profiling_enabled: true }]
[profiler, {profiling_enabled: true}]
end

private_class_method def self.build_thread_context_collector(settings, recorder, optional_tracer, timeline_enabled)
Expand Down
10 changes: 4 additions & 6 deletions lib/datadog/profiling/crashtracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,10 @@ def reset_after_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
self.class._native_stop
Datadog.logger.debug('Crash tracking stopped successfully')
rescue => e
Datadog.logger.error("Failed to stop crash tracking: #{e.message}")
end

private
Expand Down
36 changes: 17 additions & 19 deletions lib/datadog/profiling/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,23 @@ def start(on_failure_proc: nil)
end

def perform(on_failure_proc)
begin
# A profiling flush may be called while the VM is shutting down, to report the last profile. When we do so,
# we impose a strict timeout. This means this last profile may or may not be sent, depending on if the flush can
# successfully finish in the strict timeout.
# This can be somewhat confusing (why did it not get reported?), so let's at least log what happened.
interrupted = true

flush_and_wait
interrupted = false
rescue Exception => e # rubocop:disable Lint/RescueException
Datadog.logger.warn(
'Profiling::Scheduler thread error. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)
on_failure_proc&.call
raise
ensure
Datadog.logger.debug('#flush was interrupted or failed before it could complete') if interrupted
end
# A profiling flush may be called while the VM is shutting down, to report the last profile. When we do so,
# we impose a strict timeout. This means this last profile may or may not be sent, depending on if the flush can
# successfully finish in the strict timeout.
# This can be somewhat confusing (why did it not get reported?), so let's at least log what happened.
interrupted = true

flush_and_wait
interrupted = false
rescue Exception => e # rubocop:disable Lint/RescueException
Datadog.logger.warn(
'Profiling::Scheduler thread error. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)
on_failure_proc&.call
raise
ensure
Datadog.logger.debug('#flush was interrupted or failed before it could complete') if interrupted
end

# Configure Workers::IntervalLoop to not report immediately when scheduler starts
Expand Down
28 changes: 12 additions & 16 deletions lib/datadog/profiling/tasks/setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ class Setup

def run
ACTIVATE_EXTENSIONS_ONLY_ONCE.run do
begin
Datadog::Core::Utils::AtForkMonkeyPatch.apply!
setup_at_fork_hooks
rescue StandardError, ScriptError => e
Datadog.logger.warn do
"Profiler extensions unavailable. Cause: #{e.class.name} #{e.message} " \
"Location: #{Array(e.backtrace).first}"
end
Datadog::Core::Utils::AtForkMonkeyPatch.apply!
setup_at_fork_hooks
rescue StandardError, ScriptError => e
Datadog.logger.warn do
"Profiler extensions unavailable. Cause: #{e.class.name} #{e.message} " \
"Location: #{Array(e.backtrace).first}"
end
end
end
Expand All @@ -28,14 +26,12 @@ def run

def setup_at_fork_hooks
Datadog::Core::Utils::AtForkMonkeyPatch.at_fork(:child) do
begin
# Restart profiler, if enabled
Profiling.start_if_enabled
rescue => e
Datadog.logger.warn do
"Error during post-fork hooks. Cause: #{e.class.name} #{e.message} " \
"Location: #{Array(e.backtrace).first}"
end
# Restart profiler, if enabled
Profiling.start_if_enabled
rescue => e
Datadog.logger.warn do
"Error during post-fork hooks. Cause: #{e.class.name} #{e.message} " \
"Location: #{Array(e.backtrace).first}"
end
end
end
Expand Down
Loading

0 comments on commit 7c67ce3

Please sign in to comment.