Skip to content

Commit

Permalink
Collect web metrics on all containers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
adamlogic committed Jul 14, 2023
1 parent 4e1cab6 commit f0efc2c
Show file tree
Hide file tree
Showing 14 changed files with 13 additions and 82 deletions.
2 changes: 0 additions & 2 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"tasks": [
{
"label": "test current gem",
"type": "shell",
"command": "bundle exec rake test",
"options": {
"cwd": "${fileDirname}"
Expand All @@ -19,7 +18,6 @@
},
{
"label": "test current file",
"type": "shell",
"command": "bundle",
"args": ["exec", "ruby", "-I", ".", "${fileBasename}"],
"options": {
Expand Down
1 change: 1 addition & 0 deletions judoscale-delayed_job/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down
1 change: 1 addition & 0 deletions judoscale-good_job/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ GEM

PLATFORMS
arm64-darwin-21
arm64-darwin-22
x86_64-linux

DEPENDENCIES
Expand Down
1 change: 1 addition & 0 deletions judoscale-que/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down
1 change: 1 addition & 0 deletions judoscale-rack/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down
1 change: 1 addition & 0 deletions judoscale-rails/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down
1 change: 1 addition & 0 deletions judoscale-resque/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down
1 change: 1 addition & 0 deletions judoscale-ruby/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down
13 changes: 3 additions & 10 deletions judoscale-ruby/lib/judoscale/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 "."
Expand Down
6 changes: 0 additions & 6 deletions judoscale-ruby/lib/judoscale/web_metrics_collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions judoscale-ruby/test/job_metrics_collector_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 0 additions & 30 deletions judoscale-ruby/test/reporter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
32 changes: 0 additions & 32 deletions judoscale-ruby/test/web_metrics_collector_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions judoscale-sidekiq/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ GEM
PLATFORMS
arm64-darwin-20
arm64-darwin-21
arm64-darwin-22
x86_64-darwin-21
x86_64-linux

Expand Down

0 comments on commit f0efc2c

Please sign in to comment.