From 456336358d36ece29cea0d23d5156dbce2e272d0 Mon Sep 17 00:00:00 2001 From: Adam McCrea Date: Fri, 14 Jul 2023 14:01:57 -0400 Subject: [PATCH] Simplify RuntimeContainer We have no use for `service_name`, so we can treat the runtime container as just the container ID. --- judoscale-ruby/lib/judoscale/config.rb | 25 ++++------------ judoscale-ruby/test/config_test.rb | 10 +++---- .../test/job_metrics_collector_test.rb | 30 +++++++++---------- judoscale-ruby/test/reporter_test.rb | 4 +-- 4 files changed, 27 insertions(+), 42 deletions(-) diff --git a/judoscale-ruby/lib/judoscale/config.rb b/judoscale-ruby/lib/judoscale/config.rb index 7a09206c..0da928f5 100644 --- a/judoscale-ruby/lib/judoscale/config.rb +++ b/judoscale-ruby/lib/judoscale/config.rb @@ -5,28 +5,14 @@ module Judoscale class Config - class RuntimeContainer - # E.g.: - # (Heroku) => "worker_fast", "3" - # (Render) => "srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr" - def initialize(service_name = nil, instance = nil) - @service_name = service_name - @instance = instance - end - - def to_s - # heroku: 'worker_fast.5' - # render: 'srv-cfa1es5a49987h4vcvfg.5497f74465-m5wwr' - "#{@service_name}.#{@instance}" - end - + class RuntimeContainer < String # 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 # instance must be considered non-redundant def redundant_instance? - instance_is_number = Integer(@instance, exception: false) - instance_is_number && instance_is_number != 1 + instance_number = split(".")[1].to_i + instance_number > 1 end end @@ -99,11 +85,10 @@ 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 + @current_runtime_container = RuntimeContainer.new instance @api_base_url ||= "https://adapter.judoscale.com/api/#{ENV["RENDER_SERVICE_ID"]}" elsif ENV["DYNO"] - service_name, instance = ENV["DYNO"].split "." - @current_runtime_container = RuntimeContainer.new service_name, instance + @current_runtime_container = RuntimeContainer.new ENV["DYNO"] else # unsupported platform? Don't want to leave @current_runtime_container nil though @current_runtime_container = RuntimeContainer.new diff --git a/judoscale-ruby/test/config_test.rb b/judoscale-ruby/test/config_test.rb index a582d274..45c0e4a9 100644 --- a/judoscale-ruby/test/config_test.rb +++ b/judoscale-ruby/test/config_test.rb @@ -14,7 +14,7 @@ module Judoscale use_env env do config = Config.instance _(config.api_base_url).must_equal "https://example.com" - _(config.current_runtime_container.to_s).must_equal "web.1" + _(config.current_runtime_container).must_equal "web.1" _(config.log_level).must_be_nil _(config.logger).must_be_instance_of ::Logger _(config.max_request_size_bytes).must_equal 100_000 @@ -42,7 +42,7 @@ module Judoscale use_env env do config = Config.instance _(config.api_base_url).must_equal "https://adapter.judoscale.com/api/srv-cfa1es5a49987h4vcvfg" - _(config.current_runtime_container.to_s).must_equal "srv-cfa1es5a49987h4vcvfg.5497f74465-m5wwr" + _(config.current_runtime_container).must_equal "5497f74465-m5wwr" _(config.log_level).must_be_nil _(config.logger).must_be_instance_of ::Logger _(config.max_request_size_bytes).must_equal 100_000 @@ -70,7 +70,7 @@ module Judoscale use_env env do config = Config.instance _(config.api_base_url).must_equal "https://custom.example.com" - _(config.current_runtime_container.to_s).must_equal "web.2" + _(config.current_runtime_container).must_equal "web.2" _(config.log_level).must_equal ::Logger::Severity::DEBUG end end @@ -92,7 +92,7 @@ module Judoscale test_logger = ::Logger.new(StringIO.new) Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new("web", "3") + config.current_runtime_container = Config::RuntimeContainer.new("web.3") config.api_base_url = "https://block.example.com" config.log_level = :info @@ -106,7 +106,7 @@ module Judoscale config = Config.instance _(config.api_base_url).must_equal "https://block.example.com" - _(config.current_runtime_container.to_s).must_equal "web.3" + _(config.current_runtime_container).must_equal "web.3" _(config.log_level).must_equal ::Logger::Severity::INFO _(config.logger).must_equal test_logger _(config.max_request_size_bytes).must_equal 50_000 diff --git a/judoscale-ruby/test/job_metrics_collector_test.rb b/judoscale-ruby/test/job_metrics_collector_test.rb index 37f4e148..913b3d66 100644 --- a/judoscale-ruby/test/job_metrics_collector_test.rb +++ b/judoscale-ruby/test/job_metrics_collector_test.rb @@ -7,27 +7,27 @@ module Judoscale describe JobMetricsCollector do describe ".collect?" do it "collects only from the first container in the formation (if we know that), to avoid redundant collection from multiple containers when possible" do - [ - ["web", "1"], - ["worker", "1"], - ["custom_name", "1"], - ["srv-cfa1es5a49987h4vcvfg", "5497f74465-m5wwr"], - ["srv-cfa1es5a49987h4vcvfg", "aaacff2165-m5wwr"] - ].each do |args| + %w[ + web.1 + worker.1 + custom_name.1 + 5497f74465-m5wwr + aaacff2165-m5wwr + ].each do |container_id| Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new(*args) + config.current_runtime_container = Config::RuntimeContainer.new(container_id) end _(Test::TestJobMetricsCollector.collect?(Judoscale::Config.instance)).must_equal true end - [ - ["web", "2"], - ["worker", "8"], - ["custom_name", "15"] - ].each do |args| + %w[ + web.2 + worker.8 + custom_name.15 + ].each do |container_id| Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new(*args) + config.current_runtime_container = Config::RuntimeContainer.new(container_id) end _(Test::TestJobMetricsCollector.collect?(Judoscale::Config.instance)).must_equal false @@ -36,7 +36,7 @@ module Judoscale it "skips collecting if the adapter has been explicitly disabled" do Judoscale.configure { |config| - config.current_runtime_container = Config::RuntimeContainer.new("web", "1") + config.current_runtime_container = Config::RuntimeContainer.new("web.1") config.test_job_config.enabled = true } diff --git a/judoscale-ruby/test/reporter_test.rb b/judoscale-ruby/test/reporter_test.rb index 770843a8..2fcabc89 100644 --- a/judoscale-ruby/test/reporter_test.rb +++ b/judoscale-ruby/test/reporter_test.rb @@ -8,7 +8,7 @@ module Judoscale describe Reporter do before { Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new("web", "1") + config.current_runtime_container = Config::RuntimeContainer.new("web.1") config.api_base_url = "http://example.com/api/test-token" end } @@ -86,7 +86,7 @@ def stub_reporter_loop it "initializes the reporter only with registered web metrics collectors on subsequent runtime containers to avoid redundant worker metrics" do Judoscale.configure do |config| - config.current_runtime_container = Config::RuntimeContainer.new("web", "2") + config.current_runtime_container = Config::RuntimeContainer.new("web.2") end run_loop_stub = proc do |config, metrics_collectors|