Merge pull request #865 from newrelic/rubocop-3

Begin linting with Rubocop

CONTRIBUTING.md

@@ -66,7 +66,8 @@ issues](../../issues?q=is%3Aissue+is%3Aclosed).
 ### Pull Requests
 
 We can only accept PRs for version v6.12.0 or greater due to open source
-licensing restrictions.
+licensing restrictions. Please set the merge branch to `dev` unless the issue
+states otherwise.
 
 ### Code of Conduct
 
@@ -84,18 +85,29 @@ release from this repo, use the appropriate tag. New Relic usually pushes beta
 versions of a release to a working branch before tagging them for General
 Availability.
 
+The `main` branch houses the code from the latest release. The `dev` branch
+includes unreleased work. Please create all new branches off of `dev`.
+
 ## Development Environment Setup
 
 1. Fork and clone the repo locally
     - Fork the repository inside GitHub
     - `git clone [email protected]:<gh username>/newrelic-ruby-agent.git`
 1. Pick a Ruby version
-    - Use rbenv or rvm to install any version of Ruby between 2.2 up to 3.0.1
+    - Use rbenv or rvm to install any version of Ruby between 2.2 up to 3.1
 1. Install development dependencies
     - `bundle install`
 1. Check that your env is setup correctly
     - `bundle exec rake`
 
+**Optional:** Install [lefthook](https://github.com/evilmartians/lefthook) to
+integrate our team's git hooks, such as [rubocop](https://github.com/rubocop/rubocop)
+linting into your workflow.
+
+**Note:** These setup instructions will not allow you to run the entire test
+suite. Some of our suites require services such as MySQL, Postgres, Redis, and
+others to run.
+
 ## Testing
 
 The agent includes a suite of unit and functional tests which should be used to

Guardfile

@@ -1,24 +1,24 @@
 require './test/multiverse/lib/multiverse/bundler_patch'
 
-test_folders = Dir.glob("test/new_relic/*").select{|f| File.directory?(f)}
-test_folders += Dir.glob("test/new_relic/**/*").select{|f| File.directory?(f)}
+test_folders = Dir.glob("test/new_relic/*").select { |f| File.directory?(f) }
+test_folders += Dir.glob("test/new_relic/**/*").select { |f| File.directory?(f) }
 
-rake_lib_path = Bundler.with_unbundled_env{ `bundle exec gem which rake`.chomp.gsub("lib/rake.rb", "lib") }
-ruby_options = %{-w -I"#{rake_lib_path}" "#{rake_lib_path}/rake/rake_test_loader.rb"}
+rake_lib_path = Bundler.with_unbundled_env { `bundle exec gem which rake`.chomp.gsub("lib/rake.rb", "lib") }
+ruby_options = %(-w -I"#{rake_lib_path}" "#{rake_lib_path}/rake/rake_test_loader.rb")
 
 guard_options = {
   spring: "bundle exec ruby #{ruby_options} ",
-  test_folders: ['test/new_relic'] + test_folders, 
+  test_folders: ['test/new_relic'] + test_folders,
   all_after_pass: false,
   all_on_start: false
 }
 
 guard :minitest, guard_options do
-  watch(%r{^lib/(.+)\.rb$})     { |m| "test/#{m[1]}_test.rb" }
+  watch(%r{^lib/(.+)\.rb$}) { |m| "test/#{m[1]}_test.rb" }
   watch(%r{^test/.+_test\.rb$})
-  watch(%r{^test/rum/.*})       { "test/new_relic/rack/browser_monitoring_test.rb" }
+  watch(%r{^test/rum/.*}) { "test/new_relic/rack/browser_monitoring_test.rb" }
   watch(%r{^test/fixtures/cross_agent_tests/distributed_tracing/(.+).json}) { |m| "test/new_relic/agent/distributed_tracing/#{m[1]}_cross_agent_test.rb" }
-  watch('test/test_helper.rb')  { "test/new_relic" }
+  watch('test/test_helper.rb') { "test/new_relic" }
   watch('test/agent_helper.rb') { "test/new_relic" }
   watch('lib/new_relic/agent/configuration/default_source.rb') { "test/new_relic/agent/configuration/orphan_configuration_test.rb" }
   watch(%r{^lib/new_relic/agent/transaction/(.+).rb}) { |m| "test/new_relic/agent/distributed_tracing/#{m[1]}_cross_agent_test.rb" }

Rakefile

@@ -10,7 +10,7 @@ task :test => ['test:newrelic']
 
 namespace :test do
   desc "Run all tests"
-  task :all => %w{newrelic multiverse}
+  task :all => %w[newrelic multiverse]
 
   begin
     require 'test_bisect'
@@ -27,14 +27,14 @@ namespace :test do
     require File.expand_path(File.join(File.dirname(__FILE__), 'test', 'performance', 'lib', 'performance'))
     options = {}
     options[:suite] = args[:suite] if args[:suite]
-    options[:name]  = args[:name]  if args[:name]
+    options[:name] = args[:name] if args[:name]
     Performance::Runner.new(options).run_and_report
   end
 
   desc "Run agent within existing mini environments"
   task :env, [:env1, :env2, :env3, :env4, :env5, :env6] => [] do |t, args|
     require File.expand_path(File.join(File.dirname(__FILE__), 'test', 'environments', 'lib', 'environments', 'runner'))
-    Environments::Runner.new(args.map{|_,v| v}).run_and_report
+    Environments::Runner.new(args.map { |_, v| v }).run_and_report
   end
 
   Rake::TestTask.new(:intentional_fail) do |t|
@@ -51,11 +51,10 @@ namespace :test do
 
   # Note unit testing task is defined in lib/tasks/tests.rake to facilitate
   # running them in a rails application environment.
-
 end
 
 desc 'Record build number and stage'
-task :record_build, [ :build_number, :stage ] do |t, args|
+task :record_build, [:build_number, :stage] do |t, args|
   build_string = args.build_number
   build_string << ".#{args.stage}" unless args.stage.nil? || args.stage.empty?
 
@@ -111,7 +110,7 @@ end
 
 namespace :cross_agent_tests do
   cross_agent_tests_upstream_path = File.expand_path(File.join(File.dirname(__FILE__), '..', 'cross_agent_tests'))
-  cross_agent_tests_local_path    = File.expand_path(File.join(File.dirname(__FILE__), 'test', 'fixtures', 'cross_agent_tests'))
+  cross_agent_tests_local_path = File.expand_path(File.join(File.dirname(__FILE__), 'test', 'fixtures', 'cross_agent_tests'))
 
   # Note: before you pull, make sure your local repo is on the correct, synced branch!
   desc 'Pull latest changes from cross_agent_tests repo'
@@ -137,4 +136,3 @@ task :console do
   ARGV.clear
   Pry.start
 end
-

bin/mongrel_rpm

@@ -5,14 +5,14 @@ require 'rack/handler/mongrel'
 require 'optparse'
 
 port = 3000
-options = { }
+options = {}
 appname = nil
 OptionParser.new do |opts|
   opts.banner = "Usage: #{File.basename($0)} [options] [app_name]"
-  opts.on("-p", "--port=port", Integer, "default: #{port}") { | port | }
-  opts.on("--[no-]logging", "turn off request logging" ) { | l | options[:logging] = l }
-  opts.on("--license=license_key", "override license key" ) { | l | options[:license_key] = l }
-  opts.on("--install", "install a newrelic.yml template" ) { | l | options[:install] = true }
+  opts.on("-p", "--port=port", Integer, "default: #{port}") { |port| }
+  opts.on("--[no-]logging", "turn off request logging") { |l| options[:logging] = l }
+  opts.on("--license=license_key", "override license key") { |l| options[:license_key] = l }
+  opts.on("--install", "install a newrelic.yml template") { |l| options[:install] = true }
   opts.separator ""
   opts.separator "app_name is the name of the application where the metrics will be stored"
   opts.separator ""
@@ -26,8 +26,7 @@ ru_file = File.expand_path(File.join(File.dirname(__FILE__), "..", "lib", "new_r
 rackup_code = File.read ru_file
 builder = Rack::Builder.new { eval rackup_code, binding, ru_file }
 
-options = { :Host => '127.0.0.1', :Port => port }
-Rack::Handler::Mongrel.run(builder.to_app, options) do | server |
-  NewRelic::Agent.logger.info "Started Mongrel listening for '#{NewRelic::Control.instance.app_names.join(" and ")}' data at #{server.host}:#{server.port}" 
+options = {:Host => '127.0.0.1', :Port => port}
+Rack::Handler::Mongrel.run(builder.to_app, options) do |server|
+  NewRelic::Agent.logger.info "Started Mongrel listening for '#{NewRelic::Control.instance.app_names.join(" and ")}' data at #{server.host}:#{server.port}"
 end
-

bin/newrelic

@@ -2,7 +2,7 @@
 # executes one of the commands in the new_relic/commands directory
 # pass the name of the command as an argument
 
-$LOAD_PATH << File.expand_path(File.join(File.dirname(__FILE__), '..','lib'))
+$LOAD_PATH << File.expand_path(File.join(File.dirname(__FILE__), '..', 'lib'))
 require 'new_relic/cli/command'
 begin
   NewRelic::Cli::Command.run

bin/nrdebug

@@ -7,7 +7,7 @@
 require 'tempfile'
 require 'rbconfig'
 
-def fail(msg, opts={})
+def fail(msg, opts = {})
   $stderr.puts(msg)
   usage() if opts[:usage]
   exit(-1)
@@ -84,7 +84,7 @@ class ProcessDataProvider
 
   def self.for_process(pid)
     case RbConfig::CONFIG['target_os']
-    when /linux/  then LinuxProcessDataProvider.new(pid)
+    when /linux/ then LinuxProcessDataProvider.new(pid)
     when /darwin/ then DarwinProcessDataProvider.new(pid)
     end
   end
@@ -170,7 +170,7 @@ end
 class ProcessReport
   attr_reader :target, :path
 
-  def initialize(target, path=nil)
+  def initialize(target, path = nil)
     @target = target
     @path = path
   end
@@ -185,7 +185,7 @@ class ProcessReport
     end
   end
 
-  def section(f, name=nil)
+  def section(f, name = nil)
     content = begin
       yield
     rescue StandardError => e
@@ -200,27 +200,27 @@ class ProcessReport
 
   def generate
     open do |f|
-      section(f, "Time")        { Time.now }
-      section(f, "PID")         { @target.pid }
-      section(f, "Command")     { @target.procline }
-      section(f, "RSS")         { @target.rss }
-      section(f, "CPU")         { @target.cpu }
-      section(f, "Parent PID")  { @target.ppid }
-      section(f, "OS")          { ShellWrapper.execute('uname -a') }
+      section(f, "Time") { Time.now }
+      section(f, "PID") { @target.pid }
+      section(f, "Command") { @target.procline }
+      section(f, "RSS") { @target.rss }
+      section(f, "CPU") { @target.cpu }
+      section(f, "Parent PID") { @target.ppid }
+      section(f, "OS") { ShellWrapper.execute('uname -a') }
       section(f, "Environment") { @target.environment }
 
       section(f) do
         c_backtraces, ruby_backtraces = @target.gather_backtraces
         if c_backtraces.match(/could not attach/i)
           fail("Failed to attach to target process. Please try again with sudo.")
         end
-        section(f, "C Backtraces")   { c_backtraces }
+        section(f, "C Backtraces") { c_backtraces }
         section(f, "Ruby Backtrace(s)") { ruby_backtraces }
       end
 
       section(f, "Open files") { @target.open_files }
       section(f, "Log") do
-        commands = Logger.messages.map { |(_,msg)| msg }
+        commands = Logger.messages.map { |(_, msg)| msg }
         commands.join("\n")
       end
     end
@@ -243,7 +243,7 @@ def prompt_for_confirmation(target_pid, target_cmd)
   puts ''
   puts "To continue, type 'continue':"
 
-  until ($stdin.gets.strip == 'continue') do
+  until $stdin.gets.strip == 'continue'
     puts "Please type 'continue' to continue, or ctrl-c to abort."
   end
 end

infinite_tracing/Rakefile

@@ -20,5 +20,5 @@ Rake::TestTask.new do |t|
   t.libs << "#{ROOT}/test"
   t.libs << "#{ROOT}/lib"
   t.pattern = Array(file_pattern).join(",")
-  t.verbose =  ENV["VERBOSE"]
+  t.verbose = ENV["VERBOSE"]
 end

infinite_tracing/lib/infinite_tracing.rb

@@ -37,4 +37,4 @@
 
     require 'infinite_tracing/agent_integrations'
   end
-end
+end

infinite_tracing/lib/infinite_tracing/agent_integrations.rb

@@ -17,4 +17,4 @@ module InfiniteTracing
       NewRelic::Agent.logger.debug "Skipped Integrating Infinite Tracer with Agent"
     end
   end
-end
+end

infinite_tracing/lib/infinite_tracing/agent_integrations/agent.rb

@@ -5,14 +5,13 @@
 
 module NewRelic::Agent
   NewRelic::Agent.logger.debug "Installing Infinite Tracer in Agent"
-
-  Agent.class_eval do
 
+  Agent.class_eval do
     def new_infinite_tracer
       # We must start streaming in a thread or we block/deadlock the
       # entire start up process for the Agent.
-      InfiniteTracing::Client.new.tap do |client| 
-        @infinite_tracer_thread = InfiniteTracing::Worker.new(:infinite_tracer) do 
+      InfiniteTracing::Client.new.tap do |client|
+        @infinite_tracer_thread = InfiniteTracing::Worker.new(:infinite_tracer) do
           NewRelic::Agent.logger.debug "Opening Infinite Tracer Stream with gRPC server"
           client.start_streaming
         end

infinite_tracing/lib/infinite_tracing/agent_integrations/external_request_segment.rb

@@ -10,7 +10,7 @@ class ExternalRequestSegment
         def record_span_event
           # don't record a span event if the transaction is ignored
           return if transaction.ignore?
-          
+
           tracer = ::NewRelic::Agent.agent.infinite_tracer
           tracer << Proc.new { SpanEventPrimitive.for_external_request_segment self }
         end

infinite_tracing/lib/infinite_tracing/agent_integrations/segment.rb

@@ -7,7 +7,6 @@ module NewRelic
   module Agent
     class Transaction
       class Segment
-
         def segment_complete
           record_span_event
         end

infinite_tracing/lib/infinite_tracing/channel.rb

@@ -6,23 +6,22 @@
 module NewRelic::Agent
   module InfiniteTracing
     class Channel
-
       def stub
         NewRelic::Agent.logger.debug "Infinite Tracer Opening Channel to #{host_and_port}"
 
         Com::Newrelic::Trace::V1::IngestService::Stub.new \
-          host_and_port, 
-          credentials, 
+          host_and_port,
+          credentials,
           channel_override: channel
       end
 
       def channel
         GRPC::Core::Channel.new(host_and_port, settings, credentials)
       end
-      
+
       def credentials
         if Config.local?
-          :this_channel_is_insecure 
+          :this_channel_is_insecure
         else
           # Uses system configured certificates by default
           GRPC::Core::ChannelCredentials.new
@@ -36,10 +35,9 @@ def host_and_port
       def settings
         {
           'grpc.minimal_stack' => 1,
-          'grpc.enable_deadline_checking' => 0,
+          'grpc.enable_deadline_checking' => 0
         }
       end
-
     end
   end
 end

infinite_tracing/lib/infinite_tracing/client.rb

@@ -7,9 +7,9 @@
 #
 # Restarting the client will cause a new connection to the gRPC server.
 # When the client is restarted, a new streaming buffer is started and contents of old
-# buffer are transferred to the new buffer.  
+# buffer are transferred to the new buffer.
 #
-# Suspending the client will prevent the client from attempting to reconnect to the 
+# Suspending the client will prevent the client from attempting to reconnect to the
 # gRPC server, but will still continue to record the span events `seen` metric.
 module NewRelic::Agent
   module InfiniteTracing
@@ -52,7 +52,7 @@ def flush
       # Turns camelcase base class name into upper snake case version of the name.
       def formatted_class_name class_name
         class_name = class_name.split(":")[-1]
-        formatted_class_name = (class_name.gsub!(/(.)([A-Z])/,'\1_\2') || class_name).upcase
+        formatted_class_name = (class_name.gsub!(/(.)([A-Z])/, '\1_\2') || class_name).upcase
       end
 
       # Literal codes are all mapped to unique class names, so we can deduce the
@@ -91,7 +91,7 @@ def suspended?
 
       # This method is called when the server closes the record status stream without
       # raising an error.  The Channel/Connection is not closed or reset in this case.
-      # We simply start streaming again, which will reuse the channel/connection to the 
+      # We simply start streaming again, which will reuse the channel/connection to the
       # server and re-establish the gRPC bi-directional stream.  Useful for the server
       # to initiate a load-balancing scheme.
       def handle_close
@@ -100,7 +100,7 @@ def handle_close
         start_streaming
       end
 
-      # Places the client into suspended state whereby client will no longer attempt to 
+      # Places the client into suspended state whereby client will no longer attempt to
       # reconnect to the gRPC server nor will it attempt to send span events henceforth.
       # The Suspended Streaming Buffer will be installed in this state.
       def suspend
@@ -141,7 +141,7 @@ def stop
         end
       end
 
-      def start_streaming exponential_backoff=true
+      def start_streaming exponential_backoff = true
         return if suspended?
         Connection.instance.wait_for_agent_connect
         @lock.synchronize { @response_handler = record_spans exponential_backoff }