Enable standard for most of the remaining files in the codebase (#4936)

Author: p-datadog

.rubocop.yml

@@ -23,6 +23,13 @@ AllCops:
     - "vendor/bundle/**/*"
     - "spec/datadog/tracing/contrib/grpc/support/gen/**/*.rb" # Skip protoc autogenerated code
     - spec/**/*
+    - Rakefile
+    - "*.gemspec"
+    - benchmarks/**/*
+    - lib-injection/**/*
+    - ext/**/*
+    - yard/**/*
+    - Gemfile
   NewCops: disable # Don't allow new cops to be enabled implicitly.
   SuggestExtensions: false # Stop pushing suggestions constantly.
 

.standard_todo.yml

@@ -17,13 +17,8 @@ ignore:
       # character. To avoid being too directive, I would let comments go loose.
       - Layout/LeadingCommentSpace
 
-  # This disables standardrb for the rest of dd-trace-rb
-  # except those products: profiling, appsec.
-  - datadog.gemspec
-  - Rakefile
+  # This disables standardrb for third-party and generated code.
   - appraisal/**/**
-  - benchmarks/**/**
   - gemfiles/**/**
   - integration/**/**
-  - lib-injection/**/**
   - lib/datadog/**/vendor/**/*

Rakefile

@@ -40,11 +40,11 @@ namespace :test do
 
       candidates.each_key do |group|
         env = if group.empty?
-                {}
-              else
-                gemfile = AppraisalConversion.to_bundle_gemfile(group)
-                { 'BUNDLE_GEMFILE' => gemfile }
-              end
+          {}
+        else
+          gemfile = AppraisalConversion.to_bundle_gemfile(group)
+          {'BUNDLE_GEMFILE' => gemfile}
+        end
         command = "bundle check || bundle install && bundle exec rake #{spec_task}"
         command += "'[#{spec_arguments}]'" if spec_arguments
 
@@ -67,15 +67,15 @@ desc 'Run RSpec'
 namespace :spec do
   # REMINDER: If adding a new task here, make sure also add it to the `Matrixfile`
   task all: [:main, :benchmark, :custom_cop,
-             :graphql, :graphql_unified_trace_patcher, :graphql_trace_patcher, :graphql_tracing_patcher,
-             :rails, :railsredis, :railsredis_activesupport, :railsactivejob,
-             :elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument,
-             :profiling, :crashtracking, :error_tracking, :process_discovery, :stable_config]
+    :graphql, :graphql_unified_trace_patcher, :graphql_trace_patcher, :graphql_tracing_patcher,
+    :rails, :railsredis, :railsredis_activesupport, :railsactivejob,
+    :elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument,
+    :profiling, :crashtracking, :error_tracking, :process_discovery, :stable_config]
 
   desc '' # "Explicitly hiding from `rake -T`"
   RSpec::Core::RakeTask.new(:main) do |t, args|
     t.pattern = 'spec/**/*_spec.rb'
-    t.exclude_pattern = 'spec/**/{appsec/integration,contrib,benchmark,redis,auto_instrument,opentelemetry,profiling,crashtracking,error_tracking,rubocop}/**/*_spec.rb,'\
+    t.exclude_pattern = 'spec/**/{appsec/integration,contrib,benchmark,redis,auto_instrument,opentelemetry,profiling,crashtracking,error_tracking,rubocop}/**/*_spec.rb,' \
                         ' spec/**/{auto_instrument,opentelemetry,process_discovery,stable_config}_spec.rb, spec/datadog/gem_packaging_spec.rb'
     t.rspec_opts = args.to_a.join(' ')
   end
@@ -120,7 +120,7 @@ namespace :spec do
   desc '' # "Explicitly hiding from `rake -T`"
   RSpec::Core::RakeTask.new(:rails) do |t, args|
     t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb'
-    t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,'\
+    t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,' \
                         'semantic_logger}*_spec.rb'
     t.rspec_opts = args.to_a.join(' ')
   end
@@ -325,7 +325,7 @@ namespace :spec do
     desc '' # "Explicitly hiding from `rake -T`"
     RSpec::Core::RakeTask.new(:main) do |t, args|
       t.pattern = 'spec/datadog/appsec/**/*_spec.rb'
-      t.exclude_pattern = 'spec/datadog/appsec/**/{integration,contrib,auto_instrument}/**/*_spec.rb,'\
+      t.exclude_pattern = 'spec/datadog/appsec/**/{integration,contrib,auto_instrument}/**/*_spec.rb,' \
                           ' spec/datadog/appsec/**/{auto_instrument,autoload}_spec.rb'
       t.rspec_opts = args.to_a.join(' ')
     end
@@ -357,7 +357,7 @@ namespace :spec do
     end
   end
 
-  task appsec: [:'appsec:all']
+  task appsec: [:"appsec:all"]
 
   namespace :di do
     # Datadog DI integrations
@@ -424,7 +424,7 @@ namespace :spec do
     Rake::Task[:all].prerequisite_tasks.each { |t| t.enhance([:compile_native_extensions]) }
   end
 
-  task profiling: [:'profiling:all']
+  task profiling: [:"profiling:all"]
 end
 
 if defined?(RuboCop::RakeTask)
@@ -441,11 +441,11 @@ namespace :coverage do
   task :report do
     require 'simplecov'
 
-    resultset_files = Dir["#{ENV.fetch('COVERAGE_DIR', 'coverage')}/.resultset.json"] +
-      Dir["#{ENV.fetch('COVERAGE_DIR', 'coverage')}/versions/**/.resultset.json"]
+    resultset_files = Dir["#{ENV.fetch("COVERAGE_DIR", "coverage")}/.resultset.json"] +
+      Dir["#{ENV.fetch("COVERAGE_DIR", "coverage")}/versions/**/.resultset.json"]
 
     SimpleCov.collate resultset_files do
-      coverage_dir "#{ENV.fetch('COVERAGE_DIR', 'coverage')}/report"
+      coverage_dir "#{ENV.fetch("COVERAGE_DIR", "coverage")}/report"
       formatter SimpleCov::Formatter::HTMLFormatter
     end
   end
@@ -455,11 +455,11 @@ namespace :coverage do
     require 'simplecov'
     require_relative 'spec/support/simplecov_fix'
 
-    versions = Dir["#{ENV.fetch('COVERAGE_DIR', 'coverage')}/versions/*"].map { |f| File.basename(f) }
+    versions = Dir["#{ENV.fetch("COVERAGE_DIR", "coverage")}/versions/*"].map { |f| File.basename(f) }
     versions.map do |version|
       puts "Generating report for: #{version}"
-      SimpleCov.collate Dir["#{ENV.fetch('COVERAGE_DIR', 'coverage')}/versions/#{version}/**/.resultset.json"] do
-        coverage_dir "#{ENV.fetch('COVERAGE_DIR', 'coverage')}/report/versions/#{version}"
+      SimpleCov.collate Dir["#{ENV.fetch("COVERAGE_DIR", "coverage")}/versions/#{version}/**/.resultset.json"] do
+        coverage_dir "#{ENV.fetch("COVERAGE_DIR", "coverage")}/report/versions/#{version}"
         formatter SimpleCov::Formatter::HTMLFormatter
       end
     end

benchmarks/benchmarks_ips_patch.rb

@@ -2,7 +2,7 @@
 
 module JobReporter
   def report(name, *args, **opts, &block)
-    caller_path = caller_locations.first.path
+    caller_path = caller_locations(1..1).first.path
     prefix = File.basename(caller_path).sub(/_.*\z/, '')
     name = "#{prefix} - #{name}"
     # Older Rubies (e.g. 2.5) do not permit passing *args and &block

benchmarks/di_instrument.rb

@@ -1,39 +1,37 @@
-=begin
-
-"Instrumentation" part of Dynamic Instrumentation benchmarks.
-
-Typical result:
-
-Comparison:
-  no instrumentation:   589490.0 i/s
-method instrumentation - cleared:   545807.2 i/s - 1.08x  slower
-line instrumentation - cleared:   539686.5 i/s - 1.09x  slower
-no instrumentation - again:   535761.0 i/s - 1.10x  slower
-method instrumentation:   129159.5 i/s - 4.56x  slower
-line instrumentation - targeted:   128848.6 i/s - 4.58x  slower
-line instrumentation:    10771.7 i/s - 54.73x  slower
-
-Targeted line and method instrumentations have similar performance at
-about 25% of baseline. Note that the instrumented method is fairly
-small and probably runs very quickly by itself, so while this is not the
-worst possible case for instrumentation (that would be an empty method),
-likely the vast majority of real world uses of DI would have way expensive
-target code and the relative overhead of instrumentation will be significantly
-lower than it is in this benchmark.
-
-Untargeted line instrumentation is extremely slow, too slow to be usable.
-
-In theory, after instrumentation is removed, performance should return to
-the baseline. We are currently observing about a 6-10% performance loss.
-Two theories for why this is so:
-1. Some overhead remains in the code - to be investigated.
-2. The benchmarks were run on a laptop, and during the benchmarking
-process the CPU is heating up and it can't turbo to the same speeds at
-the end of the run as it can at the beginning. Meaning the observed 6-10%
-slowdown at the end is an environmental issue and not an implementation
-problem.
-
-=end
+#
+# "Instrumentation" part of Dynamic Instrumentation benchmarks.
+#
+# Typical result:
+#
+# Comparison:
+#   no instrumentation:   589490.0 i/s
+# method instrumentation - cleared:   545807.2 i/s - 1.08x  slower
+# line instrumentation - cleared:   539686.5 i/s - 1.09x  slower
+# no instrumentation - again:   535761.0 i/s - 1.10x  slower
+# method instrumentation:   129159.5 i/s - 4.56x  slower
+# line instrumentation - targeted:   128848.6 i/s - 4.58x  slower
+# line instrumentation:    10771.7 i/s - 54.73x  slower
+#
+# Targeted line and method instrumentations have similar performance at
+# about 25% of baseline. Note that the instrumented method is fairly
+# small and probably runs very quickly by itself, so while this is not the
+# worst possible case for instrumentation (that would be an empty method),
+# likely the vast majority of real world uses of DI would have way expensive
+# target code and the relative overhead of instrumentation will be significantly
+# lower than it is in this benchmark.
+#
+# Untargeted line instrumentation is extremely slow, too slow to be usable.
+#
+# In theory, after instrumentation is removed, performance should return to
+# the baseline. We are currently observing about a 6-10% performance loss.
+# Two theories for why this is so:
+# 1. Some overhead remains in the code - to be investigated.
+# 2. The benchmarks were run on a laptop, and during the benchmarking
+# process the CPU is heating up and it can't turbo to the same speeds at
+# the end of the run as it can at the beginning. Meaning the observed 6-10%
+# slowdown at the end is an environmental issue and not an implementation
+# problem.
+#
 
 # Used to quickly run benchmark under RSpec as part of the usual test suite, to validate it didn't bitrot
 VALIDATE_BENCHMARK_MODE = ENV['VALIDATE_BENCHMARK'] == 'true'
@@ -72,7 +70,7 @@ def test_method_for_line_probe
   attr_reader :instrumenter
 
   def logger
-    @logger ||= Logger.new(STDERR)
+    @logger ||= Logger.new($stderr)
   end
 
   def configure
@@ -92,7 +90,7 @@ def run_benchmark
     file, line = m.source_location
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -116,7 +114,7 @@ def run_benchmark
     end
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -159,7 +157,7 @@ def run_benchmark
     end
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -208,7 +206,7 @@ def run_benchmark
     end
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -237,7 +235,7 @@ def run_benchmark
     calls = 0
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -257,7 +255,7 @@ def run_benchmark
     end
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -277,7 +275,7 @@ def run_benchmark
     end
 
     Benchmark.ips do |x|
-      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? {time: 0.01, warmup: 0} : {time: 10, warmup: 2}
       x.config(
         **benchmark_time,
       )
@@ -289,9 +287,7 @@ def run_benchmark
       x.save! 'di-instrument-results.json' unless VALIDATE_BENCHMARK_MODE
       x.compare!
     end
-
   end
-
 end
 
 puts "Current pid is #{Process.pid}"