JSON parse cluster worker stats instead of regex (#2124)

* JSON parse cluster worker stats instead of regex

* Add tests for Puma.stats

History.md

@@ -36,6 +36,7 @@
   * Simplify `Runner#start_control` URL parsing (#2111)
   * Removed the IOBuffer extension and replaced with Ruby (#1980)
   * Update `Rack::Handler::Puma.run` to use `**options` (#2189)
+  * JSON parse cluster worker stats instead of regex (#2124)
 
 ## 4.3.3 and 3.12.4 / 2020-02-28
 

lib/puma/cluster.rb

@@ -5,6 +5,7 @@
 require 'puma/plugin'
 
 require 'time'
+require 'json'
 
 module Puma
   # This class is instantiated by the `Puma::Launcher` and used
@@ -94,11 +95,7 @@ def term?
 
       def ping!(status)
         @last_checkin = Time.now
-        captures = status.match(/{ "backlog":(?<backlog>\d*), "running":(?<running>\d*), "pool_capacity":(?<pool_capacity>\d*), "max_threads": (?<max_threads>\d*), "requests_count": (?<requests_count>\d*) }/)
-        @last_status = captures.names.inject({}) do |hash, key|
-          hash[key.to_sym] = captures[key].to_i
-          hash
-        end
+        @last_status = JSON.parse(status, symbolize_names: true)
       end
 
       def ping_timeout?(which)
@@ -287,18 +284,11 @@ def worker(index, master)
 
       Thread.new(@worker_write) do |io|
         Puma.set_thread_name "stat payload"
-        base_payload = "p#{Process.pid}"
 
         while true
           sleep Const::WORKER_CHECK_INTERVAL
           begin
-            b = server.backlog || 0
-            r = server.running || 0
-            t = server.pool_capacity || 0
-            m = server.max_threads || 0
-            rc = server.requests_count || 0
-            payload = %Q!#{base_payload}{ "backlog":#{b}, "running":#{r}, "pool_capacity":#{t}, "max_threads": #{m}, "requests_count": #{rc} }\n!
-            io << payload
+            io << "p#{Process.pid}#{server.stats.to_json}\n"
           rescue IOError
             Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
             break

lib/puma/server.rb

@@ -949,5 +949,13 @@ def possible_header_injection?(header_value)
       HTTP_INJECTION_REGEX =~ header_value.to_s
     end
     private :possible_header_injection?
+
+    # List of methods invoked by #stats.
+    STAT_METHODS = [:backlog, :running, :pool_capacity, :max_threads, :requests_count].freeze
+
+    # Returns a hash of stats about the running server for reporting purposes.
+    def stats
+      STAT_METHODS.map {|name| [name, send(name) || 0]}.to_h
+    end
   end
 end

lib/puma/single.rb

@@ -15,13 +15,8 @@ module Puma
   class Single < Runner
     def stats
       {
-        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,
-        requests_count: @server.requests_count || 0,
-      }
+        started_at: @started_at.utc.iso8601
+      }.merge(@server.stats)
     end
 
     def restart

test/test_cli.rb

@@ -91,7 +91,6 @@ def test_control_for_ssl
 
     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,"requests_count":0}/
     assert_match(expected_stats, body.split(/\r?\n/).last)
-    assert_equal([:started_at, :backlog, :running, :pool_capacity, :max_threads, :requests_count], Puma.stats.keys)
 
   ensure
     cli.launcher.stop if cli
@@ -221,7 +220,7 @@ def test_control_requests_count
     body = s.read
     s.close
 
-    assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":\d+,"running":\d+,"pool_capacity":\d+,"max_threads":\d+,"requests_count":0}/, body.split(/\r?\n/).last)
+    assert_equal 0, JSON.parse(body.split(/\r?\n/).last)['requests_count']
 
     # send real requests to server
     3.times do
@@ -236,7 +235,7 @@ def test_control_requests_count
     body = s.read
     s.close
 
-    assert_match(/{"started_at":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z","backlog":\d+,"running":\d+,"pool_capacity":\d+,"max_threads":\d+,"requests_count":3}/, body.split(/\r?\n/).last)
+    assert_equal 3, JSON.parse(body.split(/\r?\n/).last)['requests_count']
   ensure
     cli.launcher.stop
     t.join

test/test_launcher.rb

@@ -1,6 +1,7 @@
 require_relative "helper"
 
 require "puma/configuration"
+require 'puma/events'
 
 class TestLauncher < Minitest::Test
   def test_dependencies_and_files_to_require_after_prune_is_correctly_built_for_no_extra_deps
@@ -78,6 +79,39 @@ def test_pid_file
     File.unlink tmp_path
   end
 
+  def test_puma_stats
+    conf = Puma::Configuration.new do |c|
+      c.app -> {[200, {}, ['']]}
+      c.clear_binds!
+    end
+    launcher = launcher(conf)
+    launcher.events.on_booted {launcher.stop}
+    launcher.run
+    Puma::Server::STAT_METHODS.each do |stat|
+      assert_includes Puma.stats, stat
+    end
+  end
+
+  def test_puma_stats_clustered
+    skip NO_FORK_MSG unless HAS_FORK
+
+    conf = Puma::Configuration.new do |c|
+      c.app -> {[200, {}, ['']]}
+      c.workers 1
+      c.clear_binds!
+    end
+    launcher = launcher(conf)
+    Thread.new do
+      sleep Puma::Const::WORKER_CHECK_INTERVAL + 1
+      status = Puma.stats[:worker_status].first[:last_status]
+      Puma::Server::STAT_METHODS.each do |stat|
+        assert_includes status, stat
+      end
+      launcher.stop
+    end
+    launcher.run
+  end
+
   private
 
   def events