From 1bec245b631baf7e02f18ce2412dc9cca331b6cc Mon Sep 17 00:00:00 2001 From: Bart Date: Tue, 17 Dec 2019 00:38:48 -0500 Subject: [PATCH] Change Puma.stats to return a hash instead of a JSON string (#2086) Having access to the hash allows to produce stats in other ways (such as StatsD) without having to parse JSON of data that is available in memory. An example of this workaround is https://github.com/yob/puma-plugin-statsd/blob/fa6ba1f5074473618643ef7cf99747801a001dec/lib/puma/plugin/statsd.rb#L112-L114 --- History.md | 1 + lib/puma/app/status.rb | 2 +- lib/puma/cluster.rb | 31 ++++++++++++++++++++++++++----- lib/puma/single.rb | 12 +++++++----- test/test_app_status.rb | 2 +- test/test_cli.rb | 11 +++++------ 6 files changed, 41 insertions(+), 18 deletions(-) diff --git a/History.md b/History.md index 58ab594045..ddae41bfcb 100644 --- a/History.md +++ b/History.md @@ -3,6 +3,7 @@ * Features * Add pumactl `thread-backtraces` command to print thread backtraces (#2053) * Configuration: `environment` is read from `RAILS_ENV`, if `RACK_ENV` can't be found (#2022) + * `Puma.stats` now returns a Hash instead of a JSON string (#2086) * Bugfixes * Your bugfix goes here (#Github Number) diff --git a/lib/puma/app/status.rb b/lib/puma/app/status.rb index 19f6d877de..3a6518ee78 100644 --- a/lib/puma/app/status.rb +++ b/lib/puma/app/status.rb @@ -54,7 +54,7 @@ def call(env) rack_response(200, GC.stat.to_json) when /\/stats$/ - rack_response(200, @cli.stats) + rack_response(200, @cli.stats.to_json) when /\/thread-backtraces$/ backtraces = [] diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index fb9b63fa0b..f68ad151ba 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -73,7 +73,7 @@ def initialize(idx, pid, phase, options) @first_term_sent = nil @started_at = Time.now @last_checkin = Time.now - @last_status = '{}' + @last_status = {} @term = false end @@ -94,7 +94,11 @@ def term? def ping!(status) @last_checkin = Time.now - @last_status = status + captures = status.match(/{ "backlog":(?\d*), "running":(?\d*), "pool_capacity":(?\d*), "max_threads": (?\d*) }/) + @last_status = captures.names.inject({}) do |hash, key| + hash[key.to_sym] = captures[key].to_i + hash + end end def ping_timeout?(which) @@ -352,9 +356,26 @@ def reload_worker_directory # the master process. def stats old_worker_count = @workers.count { |w| w.phase != @phase } - booted_worker_count = @workers.count { |w| w.booted? } - worker_status = '[' + @workers.map { |w| %Q!{ "started_at": "#{w.started_at.utc.iso8601}", "pid": #{w.pid}, "index": #{w.index}, "phase": #{w.phase}, "booted": #{w.booted?}, "last_checkin": "#{w.last_checkin.utc.iso8601}", "last_status": #{w.last_status} }!}.join(",") + ']' - %Q!{ "started_at": "#{@started_at.utc.iso8601}", "workers": #{@workers.size}, "phase": #{@phase}, "booted_workers": #{booted_worker_count}, "old_workers": #{old_worker_count}, "worker_status": #{worker_status} }! + worker_status = @workers.map do |w| + { + started_at: w.started_at.utc.iso8601, + pid: w.pid, + index: w.index, + phase: w.phase, + booted: w.booted?, + last_checkin: w.last_checkin.utc.iso8601, + last_status: w.last_status, + } + end + + { + started_at: @started_at.utc.iso8601, + workers: @workers.size, + phase: @phase, + booted_workers: worker_status.count { |w| w[:booted] }, + old_workers: old_worker_count, + worker_status: worker_status, + } end def preload? diff --git a/lib/puma/single.rb b/lib/puma/single.rb index a1673a8eb7..e7a9c48248 100644 --- a/lib/puma/single.rb +++ b/lib/puma/single.rb @@ -14,11 +14,13 @@ module Puma # that this inherits from. class Single < Runner def stats - b = @server.backlog || 0 - r = @server.running || 0 - t = @server.pool_capacity || 0 - m = @server.max_threads || 0 - %Q!{ "started_at": "#{@started_at.utc.iso8601}", "backlog": #{b}, "running": #{r}, "pool_capacity": #{t}, "max_threads": #{m} }! + { + started_at: @started_at.utc.iso8601, + backlog: @server.backlog || 0, + running: @server.running || 0, + pool_capacity: @server.pool_capacity || 0, + max_threads: @server.max_threads || 0, + } end def restart diff --git a/test/test_app_status.rb b/test/test_app_status.rb index c8cd2af6e5..2c4b7f1c3e 100644 --- a/test/test_app_status.rb +++ b/test/test_app_status.rb @@ -24,7 +24,7 @@ def halt end def stats - "{}" + {} end end diff --git a/test/test_cli.rb b/test/test_cli.rb index 5e75cb2eb0..a4e8b4c0c3 100644 --- a/test/test_cli.rb +++ b/test/test_cli.rb @@ -57,8 +57,7 @@ def test_control_for_tcp body = s.read s.close - assert_match(/{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "backlog": 0, "running": 0, "pool_capacity": 16, "max_threads": 16 }/, body.split(/\r?\n/).last) - assert_match(/{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "backlog": 0, "running": 0, "pool_capacity": 16, "max_threads": 16 }/, Puma.stats) + assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":16,"max_threads":16}/, body.split(/\r?\n/).last) ensure cli.launcher.stop @@ -92,9 +91,9 @@ def test_control_for_ssl body = http.request(req).body end - expected_stats = /{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "backlog": 0, "running": 0, "pool_capacity": 16, "max_threads": 16 }/ + expected_stats = /{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":16,"max_threads":16}/ assert_match(expected_stats, body.split(/\r?\n/).last) - assert_match(expected_stats, Puma.stats) + assert_equal([:started_at, :backlog, :running, :pool_capacity, :max_threads], Puma.stats.keys) ensure cli.launcher.stop @@ -137,7 +136,7 @@ def test_control_clustered body = s.read s.close - assert_match(/\{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "workers": 2, "phase": 0, "booted_workers": 2, "old_workers": 0, "worker_status": \[\{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "pid": \d+, "index": 0, "phase": 0, "booted": true, "last_checkin": "[^"]+", "last_status": \{ "backlog":0, "running":2, "pool_capacity":2, "max_threads": 2 \} \},\{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "pid": \d+, "index": 1, "phase": 0, "booted": true, "last_checkin": "[^"]+", "last_status": \{ "backlog":0, "running":2, "pool_capacity":2, "max_threads": 2 \} \}\] \}/, body.split("\r\n").last) + assert_match(/\{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","workers":2,"phase":0,"booted_workers":2,"old_workers":0,"worker_status":\[\{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","pid":\d+,"index":0,"phase":0,"booted":true,"last_checkin":"[^"]+","last_status":\{"backlog":0,"running":2,"pool_capacity":2,"max_threads":2\}\},\{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","pid":\d+,"index":1,"phase":0,"booted":true,"last_checkin":"[^"]+","last_status":\{"backlog":0,"running":2,"pool_capacity":2,"max_threads":2\}\}\]\}/, body.split("\r\n").last) ensure if UNIX_SKT_EXIST && HAS_FORK cli.launcher.stop @@ -172,7 +171,7 @@ def test_control body = s.read s.close - assert_match(/{ "started_at": "\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", "backlog": 0, "running": 0, "pool_capacity": 16, "max_threads": 16 }/, body.split("\r\n").last) + assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":0,"running":0,"pool_capacity":16,"max_threads":16}/, body.split("\r\n").last) ensure if UNIX_SKT_EXIST cli.launcher.stop