From f0efc2c7e326e0561fd3de7e80f8385c2d445a62 Mon Sep 17 00:00:00 2001 From: Adam McCrea Date: Fri, 14 Jul 2023 13:12:36 -0400 Subject: [PATCH] Collect web metrics on all containers Worker containers don't receive requests, so they won't have web metrics to report anyway. The logic for suppressing the web metrics collector on worker containers was unnecessary. Removing this logic allows us to remove a bunch of other supporting code since the adapter doesn't actually care whether a container is web or worker. --- .vscode/tasks.json | 2 -- judoscale-delayed_job/Gemfile.lock | 1 + judoscale-good_job/Gemfile.lock | 1 + judoscale-que/Gemfile.lock | 1 + judoscale-rack/Gemfile.lock | 1 + judoscale-rails/Gemfile.lock | 1 + judoscale-resque/Gemfile.lock | 1 + judoscale-ruby/Gemfile.lock | 1 + judoscale-ruby/lib/judoscale/config.rb | 13 ++------ .../lib/judoscale/web_metrics_collector.rb | 6 ---- .../test/job_metrics_collector_test.rb | 4 +-- judoscale-ruby/test/reporter_test.rb | 30 ----------------- .../test/web_metrics_collector_test.rb | 32 ------------------- judoscale-sidekiq/Gemfile.lock | 1 + 14 files changed, 13 insertions(+), 82 deletions(-) diff --git a/.vscode/tasks.json b/.vscode/tasks.json index db87cd50..47cd3035 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -5,7 +5,6 @@ "tasks": [ { "label": "test current gem", - "type": "shell", "command": "bundle exec rake test", "options": { "cwd": "${fileDirname}" @@ -19,7 +18,6 @@ }, { "label": "test current file", - "type": "shell", "command": "bundle", "args": ["exec", "ruby", "-I", ".", "${fileBasename}"], "options": { diff --git a/judoscale-delayed_job/Gemfile.lock b/judoscale-delayed_job/Gemfile.lock index bac9182f..787806b3 100644 --- a/judoscale-delayed_job/Gemfile.lock +++ b/judoscale-delayed_job/Gemfile.lock @@ -48,6 +48,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux diff --git a/judoscale-good_job/Gemfile.lock b/judoscale-good_job/Gemfile.lock index e450802d..0a01a4e3 100644 --- a/judoscale-good_job/Gemfile.lock +++ b/judoscale-good_job/Gemfile.lock @@ -96,6 +96,7 @@ GEM PLATFORMS arm64-darwin-21 + arm64-darwin-22 x86_64-linux DEPENDENCIES diff --git a/judoscale-que/Gemfile.lock b/judoscale-que/Gemfile.lock index a8b05834..9aab4e0c 100644 --- a/judoscale-que/Gemfile.lock +++ b/judoscale-que/Gemfile.lock @@ -44,6 +44,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux diff --git a/judoscale-rack/Gemfile.lock b/judoscale-rack/Gemfile.lock index d696db00..35835e65 100644 --- a/judoscale-rack/Gemfile.lock +++ b/judoscale-rack/Gemfile.lock @@ -41,6 +41,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux diff --git a/judoscale-rails/Gemfile.lock b/judoscale-rails/Gemfile.lock index 85232797..93d13a2c 100644 --- a/judoscale-rails/Gemfile.lock +++ b/judoscale-rails/Gemfile.lock @@ -81,6 +81,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux diff --git a/judoscale-resque/Gemfile.lock b/judoscale-resque/Gemfile.lock index 931d5e8e..d12bb6c2 100644 --- a/judoscale-resque/Gemfile.lock +++ b/judoscale-resque/Gemfile.lock @@ -49,6 +49,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux diff --git a/judoscale-ruby/Gemfile.lock b/judoscale-ruby/Gemfile.lock index f6a05ca6..b7a4cd80 100644 --- a/judoscale-ruby/Gemfile.lock +++ b/judoscale-ruby/Gemfile.lock @@ -31,6 +31,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux diff --git a/judoscale-ruby/lib/judoscale/config.rb b/judoscale-ruby/lib/judoscale/config.rb index 2d72eb8b..7a09206c 100644 --- a/judoscale-ruby/lib/judoscale/config.rb +++ b/judoscale-ruby/lib/judoscale/config.rb @@ -8,11 +8,10 @@ class Config class RuntimeContainer # E.g.: # (Heroku) => "worker_fast", "3" - # (Render) => "srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr", "web" (or "worker", "pserv", "cron", "static") - def initialize(service_name = nil, instance = nil, service_type = nil) + # (Render) => "srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr" + def initialize(service_name = nil, instance = nil) @service_name = service_name @instance = instance - @service_type = service_type end def to_s @@ -21,12 +20,6 @@ def to_s "#{@service_name}.#{@instance}" end - def web? - # NOTE: Heroku isolates 'web' as the required _name_ for its web process - # type, Render exposes the actual service type more explicitly - @service_name == "web" || @service_type == "web" - end - # Since Heroku exposes ordinal dyno 'numbers', we can tell if the current # instance is redundant (and thus skip collecting some metrics sometimes) # We don't have a means of determining that on Render though — so every @@ -106,7 +99,7 @@ def reset if ENV["RENDER_INSTANCE_ID"] instance = ENV["RENDER_INSTANCE_ID"].delete_prefix(ENV["RENDER_SERVICE_ID"]).delete_prefix("-") - @current_runtime_container = RuntimeContainer.new ENV["RENDER_SERVICE_ID"], instance, ENV["RENDER_SERVICE_TYPE"] + @current_runtime_container = RuntimeContainer.new ENV["RENDER_SERVICE_ID"], instance @api_base_url ||= "https://adapter.judoscale.com/api/#{ENV["RENDER_SERVICE_ID"]}" elsif ENV["DYNO"] service_name, instance = ENV["DYNO"].split "." diff --git a/judoscale-ruby/lib/judoscale/web_metrics_collector.rb b/judoscale-ruby/lib/judoscale/web_metrics_collector.rb index ca9bf96e..88bd2d23 100644 --- a/judoscale-ruby/lib/judoscale/web_metrics_collector.rb +++ b/judoscale-ruby/lib/judoscale/web_metrics_collector.rb @@ -5,12 +5,6 @@ module Judoscale class WebMetricsCollector < MetricsCollector - # NOTE: We collect metrics on all running web processes since they - # all receive and handle requests independently - def self.collect?(config) - config.current_runtime_container.web? - end - def collect MetricsStore.instance.flush end diff --git a/judoscale-ruby/test/job_metrics_collector_test.rb b/judoscale-ruby/test/job_metrics_collector_test.rb index b65b2666..37f4e148 100644 --- a/judoscale-ruby/test/job_metrics_collector_test.rb +++ b/judoscale-ruby/test/job_metrics_collector_test.rb @@ -11,8 +11,8 @@ module Judoscale ["web", "1"], ["worker", "1"], ["custom_name", "1"], - ["srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr", "web"], - ["srv-cfa1es5a49987h4vcvfg", "aaacff2165-m5wwr", "worker"] + ["srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr"], + ["srv-cfa1es5a49987h4vcvfg", "aaacff2165-m5wwr"] ].each do |args| Judoscale.configure do |config| config.current_runtime_container = Config::RuntimeContainer.new(*args) diff --git a/judoscale-ruby/test/reporter_test.rb b/judoscale-ruby/test/reporter_test.rb index b4107a82..770843a8 100644 --- a/judoscale-ruby/test/reporter_test.rb +++ b/judoscale-ruby/test/reporter_test.rb @@ -99,36 +99,6 @@ def stub_reporter_loop } end - it "initializes the reporter only with registered job metrics collectors on the first non-web heroku runtime containter to avoid unnecessary web collection attempts" do - Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new("worker", "1") - end - - run_loop_stub = proc do |config, metrics_collectors| - _(metrics_collectors.size).must_equal 1 - _(metrics_collectors[0]).must_be_instance_of Test::TestJobMetricsCollector - end - - Reporter.instance.stub(:run_loop, run_loop_stub) { - Reporter.instance.start!(Config.instance, Judoscale.adapters) - } - end - - it "initializes the reporter only with registered job metrics collectors on every non-web Render runtime containter since we can't tell which instance is which on Render" do - Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new("srv-12345-12345", "abcd-abcd", "worker") - end - - run_loop_stub = proc do |config, metrics_collectors| - _(metrics_collectors.size).must_equal 1 - _(metrics_collectors[0]).must_be_instance_of Test::TestJobMetricsCollector - end - - Reporter.instance.stub(:run_loop, run_loop_stub) { - Reporter.instance.start!(Config.instance, Judoscale.adapters) - } - end - it "respects explicitly disabled job adapters / metrics collectors via config when initializing the reporter" do Judoscale.configure { |config| config.test_job_config.enabled = false } diff --git a/judoscale-ruby/test/web_metrics_collector_test.rb b/judoscale-ruby/test/web_metrics_collector_test.rb index 8ba2f5b9..ad8bd813 100644 --- a/judoscale-ruby/test/web_metrics_collector_test.rb +++ b/judoscale-ruby/test/web_metrics_collector_test.rb @@ -7,38 +7,6 @@ module Judoscale describe WebMetricsCollector do let(:store) { MetricsStore.instance } - describe ".collect?" do - it "collects only from web containers in the formation, to avoid unnecessary collection on workers" do - [ - ["web", "1"], - ["web", "15"], - ["web", "101"], - ["srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr", "web"], - ["srv-cfa1es5a49987h4vcvfg", "aaacff2165-m5wwr", "web"] - ].each do |args| - Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new(*args) - end - - _(WebMetricsCollector.collect?(Judoscale::Config.instance)).must_equal true - end - - [ - ["worker", "1"], - ["secondary", "15"], - ["periodic", "101"], - ["srv-baa1e15a49a87h4vcv22", "5497f74465-m5wwr", "worker"], - ["srv-aff1e14249124abch4vc", "abc18ce8fa-abb1w", "worker"] - ].each do |args| - Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new(*args) - end - - _(WebMetricsCollector.collect?(Judoscale::Config.instance)).must_equal false - end - end - end - describe "#collect" do it "flushes the metrics previously collected from the store" do collector = WebMetricsCollector.new diff --git a/judoscale-sidekiq/Gemfile.lock b/judoscale-sidekiq/Gemfile.lock index d7029d4e..47a05e36 100644 --- a/judoscale-sidekiq/Gemfile.lock +++ b/judoscale-sidekiq/Gemfile.lock @@ -37,6 +37,7 @@ GEM PLATFORMS arm64-darwin-20 arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-21 x86_64-linux