diff --git a/Gemfile.saas b/Gemfile.saas index 52251a26d5..8a86f3c58d 100644 --- a/Gemfile.saas +++ b/Gemfile.saas @@ -11,9 +11,9 @@ gem "console1984", bc: "console1984" gem "audits1984", bc: "audits1984" # Attachment processing in an unprivileged sibling container -gem "hotcell-client", "0.4.1" -gem "activestorage-hotcell-client", "0.4.1" -gem "mini_magick", require: false # activestorage-hotcell-client loads the ImageMagick transformer even when unused +gem "hotcell-client", "0.6.0" +gem "activestorage-hotcell-client", "0.6.0" +gem "yabeda-hotcell", "0.6.0" # Native push notifications (iOS/Android) gem "action_push_native" diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 08e7ae132c..2345820a06 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -224,9 +224,9 @@ GEM activemodel (>= 7.0) activemodel-serializers-xml (~> 1.0) activesupport (>= 7.0) - activestorage-hotcell-client (0.4.1) + activestorage-hotcell-client (0.6.0) activestorage (>= 8.2.0.alpha) - hotcell-client (= 0.4.1) + hotcell-client (= 0.6.0) addressable (2.9.0) public_suffix (>= 2.0.2, < 8.0) ansi (1.6.0) @@ -344,10 +344,10 @@ GEM herb (0.10.4-x86_64-darwin) herb (0.10.4-x86_64-linux-gnu) herb (0.10.4-x86_64-linux-musl) - hotcell-client (0.4.1) + hotcell-client (0.6.0) activesupport (>= 7.1) - hotcell-core (= 0.4.1) - hotcell-core (0.4.1) + hotcell-core (= 0.6.0) + hotcell-core (0.6.0) http-2 (1.1.1) httpx (1.7.1) http-2 (>= 1.0.0) @@ -404,8 +404,6 @@ GEM net-smtp marcel (2.1.0) matrix (0.4.3) - mini_magick (5.3.3) - logger mini_mime (1.1.5) minitest (6.0.6) drb (~> 2.0) @@ -707,6 +705,9 @@ GEM yabeda (~> 0.8) yabeda-gc (0.4.0) yabeda (~> 0.6) + yabeda-hotcell (0.6.0) + hotcell-client (= 0.6.0) + yabeda (>= 0.12) yabeda-http_requests (0.3.0) anyway_config (>= 1.3, < 3.0) sniffer @@ -742,7 +743,7 @@ DEPENDENCIES action_push_native actionpack-xml_parser activeresource - activestorage-hotcell-client (= 0.4.1) + activestorage-hotcell-client (= 0.6.0) audits1984! autotuner aws-sdk-s3 @@ -758,14 +759,13 @@ DEPENDENCIES fizzy-saas! geared_pagination (~> 1.2) gvltools - hotcell-client (= 0.4.1) + hotcell-client (= 0.6.0) image_processing (~> 2.1) importmap-rails jbuilder kamal letter_opener lexxy (= 0.9.32) - mini_magick minitest-reporters mission_control-jobs mittens @@ -807,6 +807,7 @@ DEPENDENCIES yabeda-actioncable yabeda-activejob! yabeda-gc + yabeda-hotcell (= 0.6.0) yabeda-http_requests yabeda-prometheus-mmap yabeda-puma-plugin @@ -829,7 +830,7 @@ CHECKSUMS activerecord (8.2.0.alpha) activeresource (6.2.0) sha256=818ca60d3cd1ff7bbb7277f41250ad4d61a7cdbe30fbf52b2145e1b66ed13900 activestorage (8.2.0.alpha) - activestorage-hotcell-client (0.4.1) sha256=46e09962ea839044241b026153c2c27b2f786ad929d4f8e493bcde1965244e1c + activestorage-hotcell-client (0.6.0) sha256=3c303c601d65586f938a360bfceb773c009501ad7bdc9e5ab106fa962223c22a activesupport (8.2.0.alpha) addressable (2.9.0) sha256=7fdf6ac3660f7f4e867a0838be3f6cf722ace541dd97767fa42bc6cfa980c7af ansi (1.6.0) sha256=ac9ea0c0ea8d32fb4e271348e609963ac78882f34b73836c2a02b3622e666658 @@ -899,8 +900,8 @@ CHECKSUMS herb (0.10.4-x86_64-darwin) sha256=50d522b19732e38a9c6f332e1ffaf70054ad2eca9395dd3bb81dde1f73cb2e67 herb (0.10.4-x86_64-linux-gnu) sha256=36591472b92e0f601e512b108bf25dbb761f32636703cb79d3f5388425476b1b herb (0.10.4-x86_64-linux-musl) sha256=3b5fc05c0224f156bb05993703c309d53c52a1385d37c65a956cb4d40695d70c - hotcell-client (0.4.1) sha256=45ad9c7e083a01df861d9906aa758efbd5dc1e2cf3fb6ba887b92bf57c14a019 - hotcell-core (0.4.1) sha256=5a5fff74264074122113a65080d09fe35d73bee6bed5c11e26562d050616eab6 + hotcell-client (0.6.0) sha256=836bb52c35621204c204e409e2aa0391be5c44ff3d98dcd2f47b4e78a5bcda0c + hotcell-core (0.6.0) sha256=de72cc17afd3e1bf09179021fa193e2b38e491fbaa23a7f1663e0b86a6a4f62b http-2 (1.1.1) sha256=1141a5a03c2f4e6b8d2fa62394de581e1ff6387711cd7ed577212e9d95562bba httpx (1.7.1) sha256=ae380461539203c92f3ee65c13b22dfccb0d975156426869f57820505cacdf56 i18n (1.15.2) sha256=00f9eb62412fe593b2a65a97daa75300d37abb8f7202ec748e94b6d46a9dd1b5 @@ -923,7 +924,6 @@ CHECKSUMS mail (2.9.1) sha256=06574eca475253d6c18145dd70af80d0eb970182d55053497c5f4d797ea160e8 marcel (2.1.0) sha256=f8b0327111dc7e516331eeefe052e981ce50e2bd826c1042a53cdf047fcc3512 matrix (0.4.3) sha256=a0d5ab7ddcc1973ff690ab361b67f359acbb16958d1dc072b8b956a286564c5b - mini_magick (5.3.3) sha256=b3beed8a8cef36bd03666365f14b89cd344da0ecd373af53e617c71bfd426aab mini_mime (1.1.5) sha256=8681b7e2e4215f2a159f9400b5816d85e9d8c6c6b491e96a12797e798f8bccef minitest (6.0.6) sha256=153ea36d1d987a62942382b61075745042a2b3123b1cd48f4c3675af9cc7d6f1 minitest-reporters (1.8.0) sha256=8ce5280fb73ad3178ae525454df169b6f28c1b38b1d088ea91815d3a370ba384 @@ -1059,6 +1059,7 @@ CHECKSUMS yabeda-actioncable (0.2.2) sha256=dcfaf23d36a7c9f7a939aac82e34f5db6bb83cdde26f0a3d6e21816410edb579 yabeda-activejob (0.6.0) yabeda-gc (0.4.0) sha256=bdc50e133b6c5ba27071f00d63f9184d0338acec3d0f20403900427b9b718ff3 + yabeda-hotcell (0.6.0) sha256=97723f5192d30432b5a0927c1ef27d04e9ea33f242302bab56fad5f3be130bc8 yabeda-http_requests (0.3.0) sha256=f2705a6e41dbe2ebcc1911abdfce22c1d3d89c2f067201f0e636bda91adcf927 yabeda-prometheus-mmap (0.4.0) sha256=1a66120756d6f931f03a7784e08e79060d71681ff83a9f5287df2ff756e9e2c9 yabeda-puma-plugin (0.9.0) sha256=b78673ecc7ee30bc50691ddc41b7022c1c1801843900d5101418f4a14b550bc8 diff --git a/saas/README.md b/saas/README.md index ff2c798279..1915a2f82c 100644 --- a/saas/README.md +++ b/saas/README.md @@ -68,11 +68,11 @@ In SaaS mode `bin/dev` boots a cell beside the server, through `saas/Procfile.de bin/dev # the cell boots and carries attachment processing, as in production ``` -`/hotcellz` says whether the cell is reachable. It asks the control socket only — `describe` and `metrics`, which the supervisor answers inline with no fork — and replies `OK` or `FAIL` with a 200 or a 503. It is unauthenticated so a monitor can poll it, and it says nothing else, because a stranger has no business knowing the cell's inventory or how loaded it is. +`/hotcellz` says whether the cell is reachable. Served by the gem's `HotCell::HealthController`, it asks the control socket only — `describe` and `metrics`, which the supervisor answers inline with no fork — and replies `OK` or `FAIL` with a 200 or a 503. It is unauthenticated so a monitor can poll it, and it says nothing else, because a stranger has no business knowing the cell's inventory or how loaded it is. -`/hotcellz/test` is staff-only and returns the full diagnosis as JSON, including two round trips over the work socket, the socket that carries real files. The two prove different halves of it: `example.echo` reads its descriptor directly, while `example.reopen` re-opens it by name, which is what every operation that hands a tool a filename does. A cell whose group is wrong answers echo perfectly and fails reopen — so echo alone will tell you a broken cell is healthy. Each round trip forks a worker, which is why this is the half behind authentication. +`/hotcellz/test` is the gem's `HotCell::DiagnosticsController`, staff-only because its parent is `AdminController`. It returns the full diagnosis as JSON, including two round trips over the work socket, the socket that carries real files. The two prove different halves of it: `health.echo` reads its descriptor directly, while `health.reopen` re-opens it by name, which is what every operation that hands a tool a filename does. A cell whose group is wrong answers echo perfectly and fails reopen — so echo alone will tell you a broken cell is healthy. Each round trip forks a worker, which is why this is the half behind authentication. -**A monitor therefore cannot see a broken work socket.** Only the round trips can, and they are staff-only. That is deliberate: it is a configuration error rather than something that degrades on its own, so run `/hotcellz/test` — or the `Cell.diagnostics(work: true)` runner — by hand whenever the configuration changes. +**A monitor therefore cannot see a broken work socket.** Only the round trips can, and they are staff-only. That is deliberate: it is a configuration error rather than something that degrades on its own, so run `/hotcellz/test` — or `HotCell.diagnose(work: true).as_json` in a console — by hand whenever the configuration changes. Because the dev cell shells out to your laptop rather than to the image, every tool in `saas/hotcell/Dockerfile` needs a host equivalent. `bin/setup` installs them; if you add a tool to the image, add it to `.mise.toml` and the `Brewfile` too, or that operation will fail in development only. diff --git a/saas/app/controllers/hotcellz_controller.rb b/saas/app/controllers/hotcellz_controller.rb deleted file mode 100644 index 426e319699..0000000000 --- a/saas/app/controllers/hotcellz_controller.rb +++ /dev/null @@ -1,34 +0,0 @@ -# Two actions, because the two questions cost different things to answer. `show` is unauthenticated so a -# monitor can poll it, and asks only the control socket, which the supervisor answers inline with no fork. -# `test` crosses the work socket, which forks a worker per round trip — the staff gate bounds who may -# spend one. The trade: a broken work socket is invisible to a monitor, so a person runs `test` whenever -# the configuration changes. -class HotcellzController < ApplicationController - allow_unauthenticated_access - disallow_account_scope - - before_action :ensure_staff_access, only: :test - - def show - reachable = healthy? Fizzy::Saas::Cell.diagnostics - - render plain: reachable ? "OK" : "FAIL", status: reachable ? :ok : :service_unavailable - end - - def test - diagnostics = Fizzy::Saas::Cell.diagnostics(work: true) - - render json: diagnostics, status: healthy?(diagnostics) ? :ok : :service_unavailable - end - - private - # Authorization#ensure_staff reads Current.identity.staff?, and this controller is reachable with no - # identity at all. Signed out is refused rather than sent to a login page: a prober wants an answer. - def ensure_staff_access - head :forbidden unless Current.identity&.staff? - end - - def healthy?(diagnostics) - diagnostics.values_at(:describe, :metrics, :echo, :reopen).compact.all? { it[:ok] } - end -end diff --git a/saas/config/deploy.yml b/saas/config/deploy.yml index cb2c7b074e..f1bf38704c 100644 --- a/saas/config/deploy.yml +++ b/saas/config/deploy.yml @@ -74,7 +74,7 @@ accessories: # would make what a host runs depend on when it last rebooted. saas/hotcell/bin/build writes this line; # saas/hotcell/bin/push publishes the tag. Until push has run the pull fails loudly, which is the right # way for an unbuilt image to fail. - image: registry.37signals.com/basecamp/fizzy-hotcell:2f4ddc90eae3 + image: registry.37signals.com/basecamp/fizzy-hotcell:d8c4c8004de0 roles: [ web, jobs ] # a descriptor cannot cross machines, so a cell lives on its caller's host # An accessory's own key, not one of the options below. Kamal always emits a --network of its own, # defaulting to `kamal`, so the same setting under `options:` is a second --network flag and Docker diff --git a/saas/config/routes.rb b/saas/config/routes.rb index 6da3014584..1cf75c451d 100644 --- a/saas/config/routes.rb +++ b/saas/config/routes.rb @@ -7,8 +7,8 @@ # Beside "up", because it answers the same kind of question and is polled the same way. The work-socket # round trips are a separate action because they fork a worker, and only staff may spend one. - get "hotcellz", to: "hotcellz#show" - get "hotcellz/test", to: "hotcellz#test" + get "hotcellz", to: "hot_cell/health#show" + get "hotcellz/test", to: "hot_cell/diagnostics#show" namespace :admin do mount Audits1984::Engine, at: "/console" diff --git a/saas/hotcell/Gemfile b/saas/hotcell/Gemfile index 857127ba4c..efc8dec5af 100644 --- a/saas/hotcell/Gemfile +++ b/saas/hotcell/Gemfile @@ -9,5 +9,5 @@ source "https://rubygems.org" -gem "hotcell-server", "0.4.1" -gem "activestorage-hotcell-server", "0.4.1" +gem "hotcell-server", "0.6.0" +gem "activestorage-hotcell-server", "0.6.0" diff --git a/saas/hotcell/Gemfile.lock b/saas/hotcell/Gemfile.lock index eca02cd98c..aa689d8e33 100644 --- a/saas/hotcell/Gemfile.lock +++ b/saas/hotcell/Gemfile.lock @@ -1,9 +1,9 @@ GEM remote: https://rubygems.org/ specs: - activestorage-hotcell-server (0.4.1) - hotcell-server (= 0.4.1) - image_processing (>= 2.1.0) + activestorage-hotcell-server (0.6.0) + hotcell-server (= 0.6.0) + image_processing (>= 2.2.0) mini_magick (>= 5.4.0) ruby-vips (>= 2.2.1) ffi (1.17.4) @@ -17,10 +17,10 @@ GEM ffi (1.17.4-x86_64-darwin) ffi (1.17.4-x86_64-linux-gnu) ffi (1.17.4-x86_64-linux-musl) - hotcell-core (0.4.1) - hotcell-server (0.4.1) - hotcell-core (= 0.4.1) - image_processing (2.1.0) + hotcell-core (0.6.0) + hotcell-server (0.6.0) + hotcell-core (= 0.6.0) + image_processing (2.2.0) logger (1.7.0) mini_magick (5.4.0) logger @@ -42,11 +42,11 @@ PLATFORMS x86_64-linux-musl DEPENDENCIES - activestorage-hotcell-server (= 0.4.1) - hotcell-server (= 0.4.1) + activestorage-hotcell-server (= 0.6.0) + hotcell-server (= 0.6.0) CHECKSUMS - activestorage-hotcell-server (0.4.1) sha256=00a7d7691d2f03ca3dd4ad455fc1cf79b61cb7d37974f737e6e08e8ee8660f77 + activestorage-hotcell-server (0.6.0) sha256=ee795c1053543f3c4b7716ff7308f86e92e838db5c9f8bbe871c0318fdc77ccb bundler (4.0.18) sha256=02d9a17429de1847b4e0c9f27a9ee4b20c0a74c0a641b4e77195d6019e3618ac ffi (1.17.4) sha256=bcd1642e06f0d16fc9e09ac6d49c3a7298b9789bcb58127302f934e437d60acf ffi (1.17.4-aarch64-linux-gnu) sha256=b208f06f91ffd8f5e1193da3cae3d2ccfc27fc36fba577baf698d26d91c080df @@ -59,9 +59,9 @@ CHECKSUMS ffi (1.17.4-x86_64-darwin) sha256=aa70390523cf3235096cf64962b709b4cfbd5c082a2cb2ae714eb0fe2ccda496 ffi (1.17.4-x86_64-linux-gnu) sha256=9d3db14c2eae074b382fa9c083fe95aec6e0a1451da249eab096c34002bc752d ffi (1.17.4-x86_64-linux-musl) sha256=3fdf9888483de005f8ef8d1cf2d3b20d86626af206cbf780f6a6a12439a9c49e - hotcell-core (0.4.1) sha256=5a5fff74264074122113a65080d09fe35d73bee6bed5c11e26562d050616eab6 - hotcell-server (0.4.1) sha256=9badf6200106b878c2280ebd0ec4d655f977f3a4e24684466a6c8054e05a3e20 - image_processing (2.1.0) sha256=ca3814b1b909fc5fab68b261c50eaffef11e29a6907143e9cf1efc14b6dc6f47 + hotcell-core (0.6.0) sha256=de72cc17afd3e1bf09179021fa193e2b38e491fbaa23a7f1663e0b86a6a4f62b + hotcell-server (0.6.0) sha256=48196afcfdbf1ac1fc14e3436421192d6876f65f1d7b22716bfeed3dff4a4222 + image_processing (2.2.0) sha256=e0cb9892a4395ac2b7bb2fac5c6f5bc63361f2f18ed08773e21fce5b111c6974 logger (1.7.0) sha256=196edec7cc44b66cfb40f9755ce11b392f21f7967696af15d274dde7edff0203 mini_magick (5.4.0) sha256=f120af581d9ed4ec52c57f35a67a605d112bb1d8f582d1415b147fda42d11d78 ruby-vips (2.3.0) sha256=e685ec02c13969912debbd98019e50492e12989282da5f37d05f5471442f5374 diff --git a/saas/hotcell/bin/build b/saas/hotcell/bin/build index 679d22805c..f894b7b44b 100755 --- a/saas/hotcell/bin/build +++ b/saas/hotcell/bin/build @@ -32,7 +32,7 @@ step: saas/hotcell/bin/push. To move the cell to a new hotcell version, update the gem in the application first: - BUNDLE_GEMFILE=Gemfile.saas bundle update --conservative hotcell-client activestorage-hotcell-client hotcell-core + BUNDLE_GEMFILE=Gemfile.saas bundle update --conservative hotcell-client activestorage-hotcell-client yabeda-hotcell hotcell-core saas/hotcell/bin/build then commit the two lockfiles, the deploy.yml pin, and anything else, together. diff --git a/saas/hotcell/operations/echo.rb b/saas/hotcell/operations/echo.rb deleted file mode 100644 index 972e1ee070..0000000000 --- a/saas/hotcell/operations/echo.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -# Copied from the hotcell repository's examples/operations/echo.rb. /hotcellz calls it: one round trip -# through the caller's own descriptors proves SCM_RIGHTS passing end to end, and it is the only check -# there that touches the work socket. -module Examples - class Echo < HotCell::Operation - operation "example.echo" - - def perform(inputs, outputs) - bytes = outputs.first.to_io.write(inputs.first.to_io.read) - - { bytes: bytes, staged: inputs.first.staged? } - end - end -end diff --git a/saas/hotcell/operations/health.rb b/saas/hotcell/operations/health.rb new file mode 100644 index 0000000000..24e21fb711 --- /dev/null +++ b/saas/hotcell/operations/health.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +# health.echo and health.reopen, which /hotcellz/test sends over the work socket. +require "hot_cell/health_operations" diff --git a/saas/hotcell/operations/reopen.rb b/saas/hotcell/operations/reopen.rb deleted file mode 100644 index da16402052..0000000000 --- a/saas/hotcell/operations/reopen.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -# Copied from the hotcell repository's examples/operations/reopen.rb. The same round trip as echo, but -# re-opening both descriptors by name — `/dev/fd/N` on Linux, the file's own path on macOS — a fresh open, -# rechecked against the cell's uid, which is what every operation that hands a tool a filename does. A cell -# missing the shared group answers echo perfectly and fails only this. It re-opens in both directions -# because reading and writing are different permissions, and shipped operations do each. -module Examples - class Reopen < HotCell::Operation - operation "example.reopen" - - def perform(inputs, outputs) - source, = inputs - destination, = outputs - - bytes = File.open(source.fd_path, "rb") do |input| - File.open(destination.fd_path, "wb") { |output| output.write(input.read) } - end - - { bytes: bytes, staged: source.staged? || destination.staged? } - end - end -end diff --git a/saas/lib/fizzy/saas/cell.rb b/saas/lib/fizzy/saas/cell.rb index 82f11143e8..5fefb69390 100644 --- a/saas/lib/fizzy/saas/cell.rb +++ b/saas/lib/fizzy/saas/cell.rb @@ -1,7 +1,5 @@ # The engine loads during Bundler.require, before it reaches these gems, so Gemfile order cannot be # relied on to have defined HotCell by the time this file is read. -require "socket" -require "tmpdir" require "hot_cell/client" require "active_storage/hot_cell/client" @@ -25,9 +23,6 @@ class UnprocessableAttachment < StandardError; end # UnprocessableAttachment — the inheritance graph is the classification. class ProcessingUnavailable < StandardError; end - # A /hotcellz check that answered, but answered badly. - class CheckFailed < StandardError; end - class << self # SECRET_KEY_BASE_DUMMY is asset precompilation in the Dockerfile, which boots the production # environment with no cell. @@ -75,91 +70,6 @@ def active_storage_configuration previewers: [ ActiveStorage::HotCell::Client::Previewers::Pdf::Mutool, ActiveStorage::HotCell::Client::Previewers::Video::FFmpeg ] } end - - # What /hotcellz reports. Every check answers rather than raises: the page's job is to be the last - # step of booting an accessory, the moment a cell most likely cannot answer. Control socket only by - # default; `work: true` is opt-in because each round trip takes a queue slot and forks a worker. - # `at` and `host` are spelled the way the cell's own log lines spell them, so a reading lines up - # against Loki. - def diagnostics(work: false) - checks = { describe: reporting { cell.describe or raise CheckFailed, "the cell did not answer; see metrics" }, - metrics: reporting { answered cell.metrics } } - - checks.merge! echo: reporting { echo }, reopen: reporting { reopen } if work - - { at: Time.now.utc.iso8601(3), host: Socket.gethostname, root: root }.merge(checks) - end - - # A fixed payload through example.echo, which reads the descriptor directly. - def echo(message = "hotcell") - round_trip Echo, message - end - - # The same payload through example.reopen, which re-opens the input by name as every operation that - # hands a tool a filename does — so a cell missing the shared group answers echo perfectly and - # fails only this. - def reopen(message = "hotcell") - round_trip Reopen, message - end - - private - # The client verifies access modes, so the input must be "rb" and the output "wb"; - # Tempfile.create yields "r+", which it rejects. - def round_trip(client, message) - Dir.mktmpdir do |directory| - source = File.join(directory, "in") - destination = File.join(directory, "out") - File.write source, message - - result = File.open(source, "rb") do |input| - File.open(destination, "wb") { |output| client.perform_in_hotcell input, output } - end - - verified result, File.read(destination), message - end - end - - # Staged means the worker read a copy rather than the caller's descriptor, so the round trip - # proved nothing about the thing it exists to prove. - def verified(result, returned, message) - raise CheckFailed, "the cell returned #{returned.bytesize} bytes, not the #{message.bytesize} " \ - "sent" unless returned == message - raise CheckFailed, "the input was staged onto the worker's scratch, so this did not cross a " \ - "descriptor" if result[:staged] - - result.merge(echoed: true) - end - - # The registered cell answers enabled? rather than the module: a cell registered with an explicit - # dir: is reachable whatever HOTCELL_ROOT says. - def reporting - if cell.enabled? - { ok: true, result: yield } - else - { ok: false, error: "HOTCELL_ROOT is unset, so no cell is configured" } - end - rescue CheckFailed => error - { ok: false, error: error.message } - rescue => error - { ok: false, error: "#{error.class}: #{error.message}" } - end - - def answered(response) - raise CheckFailed, "no answer from the control socket" if response.nil? - raise CheckFailed, response.failure.to_s unless response.ok? - - response.result - end - end - - class Echo < ::HotCell::Client - hotcell NAME - operation "example.echo" - end - - class Reopen < ::HotCell::Client - hotcell NAME - operation "example.reopen" end end end diff --git a/saas/lib/fizzy/saas/engine.rb b/saas/lib/fizzy/saas/engine.rb index d44f5a09dc..ec0f10a399 100644 --- a/saas/lib/fizzy/saas/engine.rb +++ b/saas/lib/fizzy/saas/engine.rb @@ -52,6 +52,7 @@ class Engine < ::Rails::Engine # Before active_storage.configs, which is where Active Storage reads these settings off config. initializer "fizzy_saas.hotcell", before: "active_storage.configs" do |app| Cell.register! + ::HotCell.diagnostics_controller_parent = "AdminController" app.config.active_storage.merge! Cell.active_storage_configuration end diff --git a/saas/lib/yabeda/hot_cell.rb b/saas/lib/yabeda/hot_cell.rb deleted file mode 100644 index 8bc5e9162d..0000000000 --- a/saas/lib/yabeda/hot_cell.rb +++ /dev/null @@ -1,99 +0,0 @@ -module Yabeda - # Two namespace traps here fail silently: inside `module Yabeda`, `Rails` resolves to Yabeda::Rails, so - # error reporting must say ::Rails; and `hotcell` is a DSL method that exists only inside - # Yabeda.configure, so factored-out methods must say Yabeda.hotcell or they record nothing. - module HotCell - def self.install! - Yabeda.configure do - group :hotcell - - counter :requests, comment: "Calls through perform_in_hotcell, by outcome", - tags: %i[ cell operation code cause ] - histogram :perform, comment: "Time the cell spent performing", unit: :seconds, - tags: %i[ cell operation ], buckets: [ 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30, 120 ] - - gauge :up, comment: "1 when the local cell answers its control socket", - tags: %i[ cell ], aggregation: :most_recent - gauge :running, comment: "Workers busy right now", tags: %i[ cell ], aggregation: :most_recent - gauge :queued, comment: "Connections waiting for a worker", tags: %i[ cell ], aggregation: :most_recent - gauge :queue_high_water, comment: "Deepest the queue has been since boot", - tags: %i[ cell ], aggregation: :most_recent - gauge :cancelled, comment: "Callers that gave up before the cell answered (a floor)", - tags: %i[ cell ], aggregation: :most_recent - gauge :killed, comment: "Workers killed since boot, by cause", - tags: %i[ cell cause ], aggregation: :most_recent - gauge :uptime_seconds, comment: "Seconds since the supervisor booted", - tags: %i[ cell ], aggregation: :most_recent - - collect { Yabeda::HotCell.collect_stats } - end - - subscribe_to_performs - end - - def self.collect_stats - ::HotCell.cells.each_value do |cell| - next unless cell.enabled? - - response = cell.metrics - Yabeda.hotcell.up.set({ cell: cell.name }, response&.ok? ? 1 : 0) - next unless response&.ok? - - set_counters cell, response.result - end - rescue => error - # A scrape must not fail because a cell is misbehaving. - ::Rails.error.report error, handled: true - end - - # The subscriber raises into whoever called instrument, so an unguarded bug here would arrive as a - # failed conversion rather than as missing metrics. - def self.subscribe_to_performs - ActiveSupport::Notifications.subscribe "perform.hot_cell" do |event| - record_perform event - rescue => error - ::Rails.error.report error, handled: true - end - end - - # Deliberately no Sentry report from here: the raise the caller sees already reaches Sentry, so a - # report here was a second copy under a second name. - def self.record_perform(event) - labels = { cell: event.payload[:cell], operation: event.payload[:operation] } - code = event.payload[:code] - - # cause is empty rather than absent: a sometimes-missing label is a separate Prometheus series. - Yabeda.hotcell.requests.increment(labels.merge(code: code || "ok", cause: event.payload[:cause].to_s)) - Yabeda.hotcell.perform.measure(labels, (event.payload[:perform_ms] || 0) / 1000.0) - log_perform event, labels, code - end - - # The line carries two clocks on purpose: `perform_ms` is what the cell measured, `duration_ms` is - # what this process waited, and their difference is the queue and the socket. `Rails.logger.info` - # rather than `logger.struct`, because `struct` lives on the per-request proxy and a subscriber - # has none. - private_class_method def self.log_perform(event, labels, code) - duration_ms = event.duration.round(1) - - ::Rails.logger.info " HotCell (#{duration_ms}ms) " + labels.merge( - code: code || "ok", - cause: event.payload[:cause], - stderr: event.payload[:stderr], - perform_ms: event.payload[:perform_ms], - duration_ms: duration_ms, - bytes_in: event.payload[:bytes_in], - bytes_out: event.payload[:bytes_out]).to_json - end - - private_class_method def self.set_counters(cell, counters) - tags = { cell: cell.name } - - Yabeda.hotcell.running.set(tags, counters[:running]) - Yabeda.hotcell.queued.set(tags, counters[:queued]) - Yabeda.hotcell.queue_high_water.set(tags, counters[:queue_high_water]) - Yabeda.hotcell.cancelled.set(tags, counters[:cancelled]) - Yabeda.hotcell.uptime_seconds.set(tags, counters[:uptime_s]) - counters[:killed_by].each { |cause, count| Yabeda.hotcell.killed.set(tags.merge(cause: cause), count) } - end - end -end diff --git a/saas/test/controllers/hotcellz_controller_test.rb b/saas/test/controllers/hotcellz_controller_test.rb deleted file mode 100644 index a23b65ec6a..0000000000 --- a/saas/test/controllers/hotcellz_controller_test.rb +++ /dev/null @@ -1,93 +0,0 @@ -require "test_helper" - -class HotcellzControllerTest < ActionDispatch::IntegrationTest - HOTCELLZ = "/hotcellz" - HOTCELLZ_TEST = "/hotcellz/test" - - test "answers without a session" do - get HOTCELLZ - - assert_response :service_unavailable - assert_equal "text/plain", response.media_type - assert_equal "FAIL", response.body - end - - test "tells an unauthenticated caller nothing but whether the cell answered" do - get HOTCELLZ - - assert_no_match(/hotcell|running|queued|uptime|HOTCELL_ROOT/i, response.body) - end - - test "runs no work-socket round trip" do - Fizzy::Saas::Cell.stubs(:echo).raises("echo must not run here") - Fizzy::Saas::Cell.stubs(:reopen).raises("reopen must not run here") - - get HOTCELLZ - - assert_response :service_unavailable - end - - test "refuses the test action without a session" do - get HOTCELLZ_TEST - - assert_response :forbidden - end - - test "refuses the test action to a signed-in identity that is not staff" do - sign_in_as :mike - - untenanted { get HOTCELLZ_TEST } - - assert_response :forbidden - end - - test "reports every check to staff" do - sign_in_as :david - - untenanted { get HOTCELLZ_TEST } - - assert_equal "application/json", response.media_type - assert_equal %w[ at host root describe metrics echo reopen ], response.parsed_body.keys - end - - test "stamps the reading with subsecond resolution" do - sign_in_as :david - - untenanted { get HOTCELLZ_TEST } - - assert_match(/\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z\z/, response.parsed_body["at"]) - end - - test "names the host that answered" do - sign_in_as :david - - untenanted { get HOTCELLZ_TEST } - - assert_equal Socket.gethostname, response.parsed_body["host"] - end - - test "reports an unconfigured cell rather than raising" do - sign_in_as :david - - untenanted { get HOTCELLZ_TEST } - - assert_response :service_unavailable - assert_equal [ false, false, false, false ], checks.map { it["ok"] } - assert_match "HOTCELL_ROOT is unset", response.parsed_body["echo"]["error"] - end - - test "reports a cell that will not answer rather than raising" do - HotCell.cell(Fizzy::Saas::Cell::NAME).stubs(:directory).returns("tmp/no-cell-here") - sign_in_as :david - - untenanted { get HOTCELLZ_TEST } - - assert_equal [ false, false, false, false ], checks.map { it["ok"] } - assert_match "Errno::ENOENT", response.parsed_body["echo"]["error"] - end - - private - def checks - response.parsed_body.values_at("describe", "metrics", "echo", "reopen") - end -end diff --git a/saas/test/integration/hotcellz_test.rb b/saas/test/integration/hotcellz_test.rb new file mode 100644 index 0000000000..2ea3c4bb2f --- /dev/null +++ b/saas/test/integration/hotcellz_test.rb @@ -0,0 +1,44 @@ +require "test_helper" + +class HotcellzTest < ActionDispatch::IntegrationTest + HOTCELLZ = "/hotcellz" + HOTCELLZ_TEST = "/hotcellz/test" + + test "answers without a session" do + get HOTCELLZ + + assert_response :service_unavailable + assert_equal "text/plain", response.media_type + assert_equal "FAIL", response.body + end + + test "tells an unauthenticated caller nothing but whether the cell answered" do + get HOTCELLZ + + assert_no_match(/hotcell|running|queued|uptime|HOTCELL_ROOT/i, response.body) + end + + test "sends a caller without a session to sign in before the test action" do + untenanted do + get HOTCELLZ_TEST + assert_redirected_to new_session_path + end + end + + test "refuses the test action to a signed-in identity that is not staff" do + sign_in_as :mike + + untenanted { get HOTCELLZ_TEST } + + assert_response :forbidden + end + + test "reports every check to staff, the work-socket round trips included" do + sign_in_as :david + + untenanted { get HOTCELLZ_TEST } + + assert_equal "application/json", response.media_type + assert_equal %w[ describe metrics echo reopen ], response.parsed_body.dig("cells", Fizzy::Saas::Cell::NAME).keys + end +end diff --git a/saas/test/lib/cell_test.rb b/saas/test/lib/cell_test.rb index 4c01558817..07bdd380e7 100644 --- a/saas/test/lib/cell_test.rb +++ b/saas/test/lib/cell_test.rb @@ -23,27 +23,6 @@ class Fizzy::Saas::CellTest < ActiveSupport::TestCase end end - test "diagnostics puts nothing on the cell's queue unless asked" do - Cell.stubs(:echo).raises("echo must not run here") - Cell.stubs(:reopen).raises("reopen must not run here") - - assert_equal %i[ at host root describe metrics ], Cell.diagnostics.keys - end - - test "work asks for the round trips as well" do - assert_equal %i[ at host root describe metrics echo reopen ], Cell.diagnostics(work: true).keys - end - - test "a round trip that returns different bytes is not ok" do - assert_raises(Cell::CheckFailed) { Cell.send :round_trip, silent_client, "hotcell" } - end - - test "a round trip whose input was staged is not ok" do - error = assert_raises(Cell::CheckFailed) { Cell.send :round_trip, staging_client, "hotcell" } - - assert_match "staged", error.message - end - test "the cell is registered even with no root, so callers get an answer rather than an UnregisteredCell" do with_env "HOTCELL_ROOT" => nil do Cell.register! @@ -110,21 +89,6 @@ class Fizzy::Saas::CellTest < ActiveSupport::TestCase end private - def silent_client - Class.new do - def self.perform_in_hotcell(input, output) = { bytes: 0, staged: false } - end - end - - def staging_client - Class.new do - def self.perform_in_hotcell(input, output) - output.write File.read(input.path) - { bytes: 7, staged: true } - end - end - end - def hotcell_classes ->(klass) { klass.name.start_with?("ActiveStorage::HotCell::Client::") } end diff --git a/saas/test/lib/hotcell_telemetry_test.rb b/saas/test/lib/hotcell_telemetry_test.rb new file mode 100644 index 0000000000..4f342920d7 --- /dev/null +++ b/saas/test/lib/hotcell_telemetry_test.rb @@ -0,0 +1,33 @@ +require "test_helper" +require "yabeda/testing" + +class HotcellTelemetryTest < ActiveSupport::TestCase + OPERATION = "active_storage.transformers.image.vips" + + setup do + Yabeda::TestAdapter.instance.reset! + @log = ActiveSupport::Logger.new(@output = StringIO.new) + Rails.logger.broadcast_to @log + end + + teardown { Rails.logger.stop_broadcasting_to @log } + + test "a call writes one log line" do + perform + + assert_equal 1, @output.string.scan(/^ HotCell \(/).size + end + + test "a call is counted by outcome" do + perform + + labels = { cell: Fizzy::Saas::Cell::NAME, operation: OPERATION, code: "ok", cause: "" } + assert_equal 1, Yabeda::TestAdapter.instance.counters[Yabeda.hotcell.requests][labels] + end + + private + def perform + ActiveSupport::Notifications.instrument "perform.hot_cell", + cell: Fizzy::Saas::Cell::NAME, operation: OPERATION, perform_ms: 250 + end +end diff --git a/saas/test/lib/yabeda/hot_cell_test.rb b/saas/test/lib/yabeda/hot_cell_test.rb deleted file mode 100644 index 7cfdf6c61b..0000000000 --- a/saas/test/lib/yabeda/hot_cell_test.rb +++ /dev/null @@ -1,171 +0,0 @@ -require "test_helper" -require "yabeda/testing" - -# Asserting values rather than that the block ran, because both of this file's namespace traps are silent: -# a bare `Rails` raises inside the rescue, and a bare `hotcell` records nothing. assert_nothing_raised -# would pass against either. -class Yabeda::HotCellTest < ActiveSupport::TestCase - COUNTERS = { uptime_s: 41, running: 2, queued: 3, queue_high_water: 7, cancelled: 1, - requests: {}, killed_by: { memory: 5, deadline: 2 } } - - setup do - Yabeda::TestAdapter.instance.reset! - @log = StringIO.new - Rails.logger.broadcast_to ActiveSupport::Logger.new(@log) - end - - teardown { Fizzy::Saas::Cell.register! } - - test "publishes the cell's counters" do - stub_cell_metrics COUNTERS - - Yabeda::HotCell.collect_stats - - assert_equal 1, gauge(:up) - assert_equal 2, gauge(:running) - assert_equal 3, gauge(:queued) - assert_equal 7, gauge(:queue_high_water) - assert_equal 1, gauge(:cancelled) - assert_equal 41, gauge(:uptime_seconds) - end - - test "publishes kills by cause" do - stub_cell_metrics COUNTERS - - Yabeda::HotCell.collect_stats - - assert_equal 5, gauge(:killed, cause: :memory) - assert_equal 2, gauge(:killed, cause: :deadline) - end - - test "a cell that does not answer is down rather than missing" do - cell = registered_cell - cell.stubs(:enabled?).returns(true) - cell.stubs(:metrics).returns(nil) - - Yabeda::HotCell.collect_stats - - assert_equal 0, gauge(:up) - assert_nil gauge(:running) - end - - test "an unregistered cell publishes nothing rather than raising" do - registered_cell.stubs(:enabled?).returns(false) - - Yabeda::HotCell.collect_stats - - assert_nil gauge(:up) - end - - test "a scrape does not fail because a cell is misbehaving" do - registered_cell.stubs(:enabled?).raises(StandardError.new("boom")) - - assert_nothing_raised { Yabeda::HotCell.collect_stats } - end - - test "counts a successful call as ok and measures what the cell spent" do - Yabeda::HotCell.record_perform perform_event(perform_ms: 250) - - assert_equal 1, counter(:requests, code: "ok", cause: "") - assert_equal 0.25, histogram(:perform) - end - - test "counts a failure under its own code" do - Yabeda::HotCell.record_perform perform_event(code: "capacity") - - assert_equal 1, counter(:requests, code: "capacity", cause: "") - end - - test "counts a kill under its cause" do - Yabeda::HotCell.record_perform perform_event(code: "killed", cause: "fsize") - - assert_equal 1, counter(:requests, code: "killed", cause: "fsize") - end - - test "logs what each call cost" do - Yabeda::HotCell.record_perform perform_event(perform_ms: 250, duration_ms: 310, bytes_in: 4096, bytes_out: 512) - - assert_match(/^ HotCell \(310\.0ms\) \{/, @log.string) - assert_equal "active_storage.transformers.image.vips", logged["operation"] - assert_equal "ok", logged["code"] - assert_equal 250, logged["perform_ms"] - assert_equal 310, logged["duration_ms"] - assert_equal 4096, logged["bytes_in"] - assert_equal 512, logged["bytes_out"] - end - - test "logs a failure under its own code" do - Yabeda::HotCell.record_perform perform_event(code: "capacity") - - assert_equal "capacity", logged["code"] - end - - test "logs why a worker was killed" do - Yabeda::HotCell.record_perform perform_event(code: "killed", cause: "fsize") - - assert_equal "killed", logged["code"] - assert_equal "fsize", logged["cause"] - end - - test "logs what a failed tool wrote to stderr" do - Yabeda::HotCell.record_perform perform_event(code: "killed", cause: "signal", stderr: "libgomp: Thread creation failed") - - assert_equal "libgomp: Thread creation failed", logged["stderr"] - end - - test "does not report a failure to Sentry, because the raise already does" do - Rails.error.expects(:report).never - - Yabeda::HotCell.record_perform perform_event(code: "killed", cause: "fsize") - Yabeda::HotCell.record_perform perform_event(code: "capacity") - Yabeda::HotCell.record_perform perform_event(code: "unreadable") - end - - test "a metrics bug arrives as missing metrics rather than as a failed conversion" do - Yabeda.stubs(:hotcell).raises(StandardError.new("boom")) - - assert_nothing_raised do - ActiveSupport::Notifications.instrument("perform.hot_cell") { } - end - end - - private - def registered_cell - HotCell.cell Fizzy::Saas::Cell::NAME - end - - def stub_cell_metrics(counters) - cell = registered_cell - cell.stubs(:enabled?).returns(true) - cell.stubs(:metrics).returns(stub(ok?: true, result: counters)) - end - - # duration_ms is measured by the event itself, not carried in the payload, so faking it takes the - # same start/finish the real event has. - def perform_event(code: nil, perform_ms: 0, cause: nil, stderr: nil, duration_ms: 0, bytes_in: nil, bytes_out: nil) - start = Time.now - ActiveSupport::Notifications::Event.new("perform.hot_cell", start, start + duration_ms / 1000.0, nil, - { cell: Fizzy::Saas::Cell::NAME, operation: "active_storage.transformers.image.vips", - code: code, cause: cause, stderr: stderr, perform_ms: perform_ms, bytes_in: bytes_in, bytes_out: bytes_out }) - end - - def logged - JSON.parse @log.string[/^ HotCell \([\d.]+ms\) (\{.*\})$/, 1] - end - - def gauge(metric, **tags) - Yabeda::TestAdapter.instance.gauges[Yabeda.hotcell.public_send(metric)][tags.merge(cell: Fizzy::Saas::Cell::NAME)] - end - - def counter(metric, **tags) - Yabeda::TestAdapter.instance.counters[Yabeda.hotcell.public_send(metric)][default_labels.merge(tags)] - end - - def histogram(metric) - Yabeda::TestAdapter.instance.histograms[Yabeda.hotcell.public_send(metric)][default_labels] - end - - def default_labels - { cell: Fizzy::Saas::Cell::NAME, operation: "active_storage.transformers.image.vips" } - end -end