From a14eb38c767dba880cdac063eee8102f11d26326 Mon Sep 17 00:00:00 2001 From: fallwith Date: Mon, 10 Jul 2023 21:02:06 -0700 Subject: [PATCH 1/4] CI: Test for the health of all code base URLs - Add a new unit test class for testing the health of all URLs defined in the code base. See the comments in `test/new_relic/healthy_urls_test.rb` for more details - Update `ci_cron.yml` to set a `CI_CRON` env var to true - Update `.ruboycop.yml` to permit `throw` and `catch` calls without parens - Add new `skip_unless_ci_cron` helper to permit tests to only run via `ci_cron.yml` - Add new `skip_unless_newest_ruby` helper to permit tests to only run on the newest Ruby - Add new `agent_root` helper to permit tests to quickly get to the agent root - Fix 5 URLs that the new test caught resolves #2126 --- .github/workflows/ci_cron.yml | 1 + .rubocop.yml | 2 + .../agent/configuration/environment_source.rb | 2 +- .../agent/instrumentation/memcache.rb | 2 +- .../agent/instrumentation/queue_time.rb | 2 +- lib/new_relic/agent/instrumentation/sequel.rb | 2 +- lib/new_relic/agent/pipe_service.rb | 2 +- test/helpers/misc.rb | 25 ++++ test/new_relic/healthy_urls_test.rb | 134 ++++++++++++++++++ 9 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 test/new_relic/healthy_urls_test.rb diff --git a/.github/workflows/ci_cron.yml b/.github/workflows/ci_cron.yml index 09798f22a6..b9b4d82c31 100644 --- a/.github/workflows/ci_cron.yml +++ b/.github/workflows/ci_cron.yml @@ -117,6 +117,7 @@ jobs: env: VERBOSE_TEST_OUTPUT: true DB_PORT: ${{ job.services.mysql.ports[3306] }} + CI_CRON: true multiverse: diff --git a/.rubocop.yml b/.rubocop.yml index ee030cbc1d..a6e4095d54 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1248,6 +1248,7 @@ Style/MethodCallWithArgsParentheses: AllowedMethods: - add_dependency - add_development_dependency + - catch - expect - fail - gem @@ -1261,6 +1262,7 @@ Style/MethodCallWithArgsParentheses: - source - stub - stub_const + - throw - use AllowedPatterns: [^assert, ^refute] diff --git a/lib/new_relic/agent/configuration/environment_source.rb b/lib/new_relic/agent/configuration/environment_source.rb index f5eadb4f2f..382c401eab 100644 --- a/lib/new_relic/agent/configuration/environment_source.rb +++ b/lib/new_relic/agent/configuration/environment_source.rb @@ -101,7 +101,7 @@ def set_key_by_type(config_key, environment_key) end else ::NewRelic::Agent.logger.info("#{environment_key} does not have a corresponding configuration setting (#{config_key} does not exist).") - ::NewRelic::Agent.logger.info('Run `rake newrelic:config:docs` or visit https://newrelic.com/docs/ruby/ruby-agent-configuration to see a list of available configuration settings.') + ::NewRelic::Agent.logger.info('Run `rake newrelic:config:docs` or visit https://docs.newrelic.com/docs/apm/agents/ruby-agent/configuration/ruby-agent-configuration to see a list of available configuration settings.') self[config_key] = value end end diff --git a/lib/new_relic/agent/instrumentation/memcache.rb b/lib/new_relic/agent/instrumentation/memcache.rb index eeafae7e0e..c9a5fbd676 100644 --- a/lib/new_relic/agent/instrumentation/memcache.rb +++ b/lib/new_relic/agent/instrumentation/memcache.rb @@ -6,7 +6,7 @@ # each with slightly different APIs and semantics. # See: # http://www.deveiate.org/code/Ruby-MemCache/ (Gem: Ruby-MemCache) -# http://seattlerb.rubyforge.org/memcache-client/ (Gem: memcache-client) +# https://github.com/mperham/memcache-client (Gem: memcache-client) # https://github.com/mperham/dalli (Gem: dalli) require_relative 'memcache/helper' diff --git a/lib/new_relic/agent/instrumentation/queue_time.rb b/lib/new_relic/agent/instrumentation/queue_time.rb index d303d3f892..60de28f7ed 100644 --- a/lib/new_relic/agent/instrumentation/queue_time.rb +++ b/lib/new_relic/agent/instrumentation/queue_time.rb @@ -5,7 +5,7 @@ module NewRelic module Agent module Instrumentation - # https://newrelic.com/docs/features/tracking-front-end-time + # https://docs.newrelic.com/docs/features/tracking-front-end-time # Record queue time metrics based on any of three headers # which can be set on the request. module QueueTime diff --git a/lib/new_relic/agent/instrumentation/sequel.rb b/lib/new_relic/agent/instrumentation/sequel.rb index 0177b90ae5..9f5aa1a826 100644 --- a/lib/new_relic/agent/instrumentation/sequel.rb +++ b/lib/new_relic/agent/instrumentation/sequel.rb @@ -30,7 +30,7 @@ def supported_sequel_version? else NewRelic::Agent.logger.info('Detected Sequel version %s.' % [Sequel::VERSION]) NewRelic::Agent.logger.info('Please see additional documentation: ' + - 'https://newrelic.com/docs/ruby/sequel-instrumentation') + 'https://docs.newrelic.com/docs/apm/agents/ruby-agent/frameworks/sequel-instrumentation/') end Sequel.synchronize { Sequel::DATABASES.dup }.each do |db| diff --git a/lib/new_relic/agent/pipe_service.rb b/lib/new_relic/agent/pipe_service.rb index 57797bfe94..ab3b22ea85 100644 --- a/lib/new_relic/agent/pipe_service.rb +++ b/lib/new_relic/agent/pipe_service.rb @@ -15,7 +15,7 @@ def initialize(channel_id) if @pipe && @pipe.parent_pid != $$ @pipe.after_fork_in_child else - NewRelic::Agent.logger.error('No communication channel to parent process, please see https://newrelic.com/docs/ruby/resque-instrumentation for more information.') + NewRelic::Agent.logger.error('No communication channel to parent process, please see https://docs.newrelic.com/docs/apm/agents/ruby-agent/background-jobs/resque-instrumentation/ for more information.') end end diff --git a/test/helpers/misc.rb b/test/helpers/misc.rb index 00d6493967..203172c8ab 100644 --- a/test/helpers/misc.rb +++ b/test/helpers/misc.rb @@ -124,3 +124,28 @@ def skip_unless_minitest5_or_above skip 'This test requires MiniTest v5+' end + +def skip_unless_ci_cron + return if ENV['CI_CRON'] + + skip 'This test only runs as part of the CI cron workflow' +end + +def agent_root + @agent_root ||= File.expand_path('../../..', __FILE__).freeze +end + +def newest_ruby + @newest_ruby ||= begin + hash = YAML.load_file(File.join(agent_root, '.github/workflows/ci_cron.yml')) + hash['jobs']['unit_tests']['strategy']['matrix']['ruby-version'].sort do |a, b| + Gem::Version.new(a) <=> Gem::Version.new(b) + end.last + end +end + +def skip_unless_newest_ruby + return if Gem::Version.new(RUBY_VERSION) >= newest_ruby + + skip 'This test only runs on the latest CI cron Ruby version' +end diff --git a/test/new_relic/healthy_urls_test.rb b/test/new_relic/healthy_urls_test.rb new file mode 100644 index 0000000000..0b6cbef8da --- /dev/null +++ b/test/new_relic/healthy_urls_test.rb @@ -0,0 +1,134 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +# The unit test class defined herein will verify the health of all URLs found +# in the project source code. +# +# To run the URL health tests by themselves: +# TEST=test/new_relic/healthy_urls_test bundle exec rake test +# +# A file will be scanned for URLs if the file's basename is found in the +# FILENAMES array OR the file's extension is found in the EXTENSIONS array +# unless the file's absolute path matches the IGNORED_FILE_PATTERN regex. +# +# NOTE that CHANGELOG.md is handled with special logic so that only the most +# recent 2 versions mentioned in the changelog are scannned for URLs. +# +# See TIMEOUT for the number of seconds permitted for a GET request to a given +# URL to be completed. +# +# Enable DEBUG for additional verbosity + +require 'httparty' +require_relative '../test_helper' + +class HealthyUrlsTest < Minitest::Test + ROOT = File.expand_path('../../..', __FILE__).freeze + FILENAMES = %w[ + baselines + Brewfile + Capfile + Dockerfile + Envfile + Gemfile + Guardfile + install_mysql55 + LICENSE + mega-runner + newrelic + newrelic_cmd + nrdebug + Rakefile + run_tests + runner + Thorfile + ].freeze + EXTENSIONS = %w[ + css + erb + gemspec + haml + html + js + json + md + proto + rake + readme + rb + sh + txt + thor + tt + yml + ].freeze + FILE_PATTERN = /(?:^(?:#{FILENAMES.join('|')})$)|\.(?:#{EXTENSIONS.join('|')})$/.freeze + IGNORED_FILE_PATTERN = %r{/(?:coverage|test)/}.freeze + URL_PATTERN = %r{(https?://.*?)[^a-zA-Z0-9/\.\-_#]}.freeze + IGNORED_URL_PATTERN = %r{(?:\{|\(|\$|169\.254|\.\.\.|metadata\.google)} + TIMEOUT = 5 + DEBUG = false + + def test_all_urls + skip_unless_ci_cron + skip_unless_newest_ruby + + urls = gather_urls + errors = urls.each_with_object({}) do |(url, _files), hash| + error = verify_url(url) + hash[url] = error if error + end + + msg = "#{errors.keys.size} URLs were unreachable!\n\n" + msg += errors.map { |url, error| " #{url} - #{error}\n files: #{urls[url].join(',')}" }.join("\n") + + assert_empty errors, msg + end + + private + + def real_url?(url) + return false if url.match?(IGNORED_URL_PATTERN) + + true + end + + def gather_urls + Dir.glob(File.join(ROOT, '**', '*')).each_with_object({}) do |file, urls| + next unless File.file?(file) && File.basename(file).match?(FILE_PATTERN) && !file.match?(IGNORED_FILE_PATTERN) + + catch :done_with_file do + changelog_entries_seen = 0 + File.open(file).each do |line| + changelog_entries_seen += 1 if File.basename(file).eql?('CHANGELOG.md') && line.start_with?('##') + throw :done_with_file if changelog_entries_seen > 2 + next unless line =~ URL_PATTERN + + url = Regexp.last_match(1).sub(%r{(?:/|\.)$}, '') + if real_url?(url) + urls[url] ||= [] + urls[url] << file + end + end + end + end + end + + def verify_url(url) + puts "Testing '#{url}'..." if DEBUG + res = HTTParty.get(url, timeout: TIMEOUT) + if res.success? + puts ' OK.' if DEBUG + return + end + + msg = "HTTP #{res.code}: #{res.message}" + puts " FAILED. #{msg}" if DEBUG + msg + rescue StandardError => e + msg = "#{e.class}: #{e.message}" + puts " FAILED. #{msg}" if DEBUG + msg + end +end From d101509c52c2a422355d60cdbf5c87f675b8b25a Mon Sep 17 00:00:00 2001 From: fallwith Date: Mon, 10 Jul 2023 21:59:53 -0700 Subject: [PATCH 2/4] CI: don't run URL health checks via env tests Prevent the `env` tests from running the URL health check tests --- test/new_relic/healthy_urls_test.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/new_relic/healthy_urls_test.rb b/test/new_relic/healthy_urls_test.rb index 0b6cbef8da..2269b5c28a 100644 --- a/test/new_relic/healthy_urls_test.rb +++ b/test/new_relic/healthy_urls_test.rb @@ -20,7 +20,6 @@ # # Enable DEBUG for additional verbosity -require 'httparty' require_relative '../test_helper' class HealthyUrlsTest < Minitest::Test @@ -73,6 +72,7 @@ class HealthyUrlsTest < Minitest::Test def test_all_urls skip_unless_ci_cron skip_unless_newest_ruby + load_httparty urls = gather_urls errors = urls.each_with_object({}) do |(url, _files), hash| @@ -88,6 +88,12 @@ def test_all_urls private + def load_httparty + require 'httparty' + rescue + skip 'Skipping URL health tests in this context, as HTTParty is not available' + end + def real_url?(url) return false if url.match?(IGNORED_URL_PATTERN) From 0be5af564ac2a33ea989c4b10c9554dce42b678c Mon Sep 17 00:00:00 2001 From: fallwith Date: Mon, 10 Jul 2023 22:28:55 -0700 Subject: [PATCH 3/4] CI: fix newest_ruby helper always compare gem version objects --- test/helpers/misc.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/helpers/misc.rb b/test/helpers/misc.rb index 203172c8ab..47717db285 100644 --- a/test/helpers/misc.rb +++ b/test/helpers/misc.rb @@ -138,9 +138,10 @@ def agent_root def newest_ruby @newest_ruby ||= begin hash = YAML.load_file(File.join(agent_root, '.github/workflows/ci_cron.yml')) - hash['jobs']['unit_tests']['strategy']['matrix']['ruby-version'].sort do |a, b| + version_string = hash['jobs']['unit_tests']['strategy']['matrix']['ruby-version'].sort do |a, b| Gem::Version.new(a) <=> Gem::Version.new(b) end.last + Gem::Version.new(version_string) end end From 76c95a87f7154617f9d32a149e1925eb636e9a45 Mon Sep 17 00:00:00 2001 From: fallwith Date: Tue, 11 Jul 2023 09:52:26 -0700 Subject: [PATCH 4/4] CI: url checker - use 'break' we only need to break out of the current loop, so throw/catch is overkill --- test/new_relic/healthy_urls_test.rb | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/new_relic/healthy_urls_test.rb b/test/new_relic/healthy_urls_test.rb index 2269b5c28a..202485009c 100644 --- a/test/new_relic/healthy_urls_test.rb +++ b/test/new_relic/healthy_urls_test.rb @@ -104,18 +104,16 @@ def gather_urls Dir.glob(File.join(ROOT, '**', '*')).each_with_object({}) do |file, urls| next unless File.file?(file) && File.basename(file).match?(FILE_PATTERN) && !file.match?(IGNORED_FILE_PATTERN) - catch :done_with_file do - changelog_entries_seen = 0 - File.open(file).each do |line| - changelog_entries_seen += 1 if File.basename(file).eql?('CHANGELOG.md') && line.start_with?('##') - throw :done_with_file if changelog_entries_seen > 2 - next unless line =~ URL_PATTERN - - url = Regexp.last_match(1).sub(%r{(?:/|\.)$}, '') - if real_url?(url) - urls[url] ||= [] - urls[url] << file - end + changelog_entries_seen = 0 + File.open(file).each do |line| + changelog_entries_seen += 1 if File.basename(file).eql?('CHANGELOG.md') && line.start_with?('##') + break if changelog_entries_seen > 2 + next unless line =~ URL_PATTERN + + url = Regexp.last_match(1).sub(%r{(?:/|\.)$}, '') + if real_url?(url) + urls[url] ||= [] + urls[url] << file end end end