Add /metrics endpoint with yabeda-prometheus #47

Merged

ldraney merged 3 commits from 19-add-metrics-endpoint into main 2026-06-01 12:28:12 +00:00

Conversation 5 Commits 3 Files changed 7 +88 -19

ldraney commented 2026-05-29 12:24:02 +00:00

Owner

Copy link

Summary

Wire up yabeda-rails, yabeda-prometheus, and yabeda-puma-plugin to expose Prometheus-format metrics at /metrics. This gives the app request counts by controller/action/status, request duration histograms, and Puma thread/worker saturation -- everything needed for Grafana dashboards and alerting.

Changes

• Gemfile -- Added yabeda-rails, yabeda-prometheus, yabeda-puma-plugin gems after lograge
• Gemfile.lock -- Updated lockfile with new gems and transitive dependencies
• config/initializers/yabeda.rb -- New initializer that calls Yabeda.configure! after app initialization
• config/routes.rb -- Mounted Yabeda::Prometheus::Exporter at /metrics before health check
• config/puma.rb -- Added plugin :yabeda to expose Puma thread/worker metrics
• config/environments/production.rb -- Added lograge.ignore_custom to suppress /metrics from request logs; updated comment on silence_healthcheck_path

Test Plan

• GET /metrics returns Prometheus text format with text/plain content type
• Metrics include rails_requests_total by method/controller/status and rails_request_duration histogram
• Puma metrics (puma_workers, puma_running_threads, etc.) present in output
• /metrics requests do not appear in lograge output
• Existing tests still pass

Review Checklist

• Gems added to main group (not dev/test)
• Lockfile updated and committed
• /metrics mounted before health check in routes
• Puma plugin activated in config/puma.rb
• Lograge configured to ignore /metrics path
• No business logic or controller changes

Related Notes

None.

Related

Closes #19
Parent: #43 (Observability & DORA metrics stack)

## Summary Wire up yabeda-rails, yabeda-prometheus, and yabeda-puma-plugin to expose Prometheus-format metrics at `/metrics`. This gives the app request counts by controller/action/status, request duration histograms, and Puma thread/worker saturation -- everything needed for Grafana dashboards and alerting. ## Changes - **Gemfile** -- Added `yabeda-rails`, `yabeda-prometheus`, `yabeda-puma-plugin` gems after lograge - **Gemfile.lock** -- Updated lockfile with new gems and transitive dependencies - **config/initializers/yabeda.rb** -- New initializer that calls `Yabeda.configure!` after app initialization - **config/routes.rb** -- Mounted `Yabeda::Prometheus::Exporter` at `/metrics` before health check - **config/puma.rb** -- Added `plugin :yabeda` to expose Puma thread/worker metrics - **config/environments/production.rb** -- Added `lograge.ignore_custom` to suppress `/metrics` from request logs; updated comment on silence_healthcheck_path ## Test Plan - `GET /metrics` returns Prometheus text format with `text/plain` content type - Metrics include `rails_requests_total` by method/controller/status and `rails_request_duration` histogram - Puma metrics (`puma_workers`, `puma_running_threads`, etc.) present in output - `/metrics` requests do not appear in lograge output - Existing tests still pass ## Review Checklist - [x] Gems added to main group (not dev/test) - [x] Lockfile updated and committed - [x] /metrics mounted before health check in routes - [x] Puma plugin activated in config/puma.rb - [x] Lograge configured to ignore /metrics path - [x] No business logic or controller changes ## Related Notes None. ## Related Closes #19 Parent: #43 (Observability & DORA metrics stack)

ldraney added 1 commit 2026-05-29 12:24:02 +00:00

Add /metrics endpoint with yabeda-prometheus for observability ... 06092dfab8

Wire up yabeda-rails, yabeda-prometheus, and yabeda-puma-plugin to expose
Prometheus-format metrics at /metrics. This gives us request counts by
controller/action/status, request duration histograms, and Puma thread/worker
saturation — everything needed for Grafana dashboards and alerting.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented 2026-05-29 12:24:49 +00:00

Author

Owner

Copy link

QA Review

Findings

1. No issues found with core implementation. The gem selection (yabeda-rails, yabeda-prometheus, yabeda-puma-plugin) is the standard stack for Rails Prometheus metrics. Gems are correctly placed in the main group, not dev/test.

2. Initializer pattern is correct. Yabeda.configure! inside after_initialize ensures all railtie-registered metrics (from yabeda-rails) are finalized before the app starts serving. This is the documented pattern.

3. Route mounting is correct. Exporter is mounted before the health check, which matches the issue spec.

4. Puma plugin activation is correct. plugin :yabeda in puma.rb will expose thread pool and worker metrics.

5. Lograge ignore_custom is defensive but harmless. Since /metrics is a mounted Rack app, it bypasses ActionController dispatch entirely, so lograge would never see these requests anyway. The ignore_custom lambda will never fire in practice. Not a bug -- just a no-op safety net. Fine to keep.

6. Note: /metrics is unauthenticated. The endpoint is publicly accessible. This is standard for Prometheus scraping (the scraper needs unauthenticated access), and the app runs on a Tailscale network, so this is acceptable. Flagging for awareness only -- no action needed.

7. Gemfile.lock includes unrelated version bumps. The lockfile has incidental updates to aws-sdk-core, json, puma, solid_cable, etc. This is normal behavior from bundle lock --update resolving the full dependency tree. Not a concern but worth noting for the reviewer.

VERDICT: APPROVE

Clean, minimal implementation that matches the issue spec exactly. No business logic changes, no unnecessary complexity.

## QA Review ### Findings **1. No issues found with core implementation.** The gem selection (yabeda-rails, yabeda-prometheus, yabeda-puma-plugin) is the standard stack for Rails Prometheus metrics. Gems are correctly placed in the main group, not dev/test. **2. Initializer pattern is correct.** `Yabeda.configure!` inside `after_initialize` ensures all railtie-registered metrics (from yabeda-rails) are finalized before the app starts serving. This is the documented pattern. **3. Route mounting is correct.** Exporter is mounted before the health check, which matches the issue spec. **4. Puma plugin activation is correct.** `plugin :yabeda` in puma.rb will expose thread pool and worker metrics. **5. Lograge ignore_custom is defensive but harmless.** Since `/metrics` is a mounted Rack app, it bypasses ActionController dispatch entirely, so lograge would never see these requests anyway. The `ignore_custom` lambda will never fire in practice. Not a bug -- just a no-op safety net. Fine to keep. **6. Note: /metrics is unauthenticated.** The endpoint is publicly accessible. This is standard for Prometheus scraping (the scraper needs unauthenticated access), and the app runs on a Tailscale network, so this is acceptable. Flagging for awareness only -- no action needed. **7. Gemfile.lock includes unrelated version bumps.** The lockfile has incidental updates to aws-sdk-core, json, puma, solid_cable, etc. This is normal behavior from `bundle lock --update` resolving the full dependency tree. Not a concern but worth noting for the reviewer. ### VERDICT: APPROVE Clean, minimal implementation that matches the issue spec exactly. No business logic changes, no unnecessary complexity.

ldraney commented 2026-05-29 12:27:59 +00:00

Author

Owner

Copy link

PR #47 Review

DOMAIN REVIEW

Tech stack identified: Ruby on Rails 8.1, Puma, RSpec, Lograge, Yabeda/Prometheus. This is a Rails infrastructure PR -- no business logic changes, purely wiring up metrics collection and exposure.

Ruby/Rails observations:

1. Initializer pattern is correct. Using Rails.application.config.after_initialize { Yabeda.configure! } ensures all Rails components are loaded before metrics registration. This is the documented yabeda-rails pattern.

2. Route mounting order is correct. /metrics is mounted before the health check and application routes, which means it will match first without falling through to any controller-level middleware (e.g., allow_browser). Since Yabeda::Prometheus::Exporter is a Rack app mounted directly in the router, it bypasses ApplicationController and its allow_browser filter -- this is the right behavior for a scrape endpoint.

3. Puma plugin placement is correct. plugin :yabeda is added after :tmp_restart and before :solid_queue, which is a reasonable position. The yabeda-puma-plugin collects thread/worker metrics from Puma's internal stats.

4. Lograge ignore_custom is correct. Using event.payload[:path] == "/metrics" filters metrics scrape requests from structured logs, preventing Prometheus's 15s scrape interval from flooding Loki. The comment on line 43 of production.rb was updated to reflect the new exclusion -- good.

5. Gem placement is correct. All three yabeda gems are in the main Gemfile group (not dev/test), which is required since metrics must be available in production. The gems are placed after lograge with a clear comment block.

6. Lockfile is consistent. The Gemfile.lock includes yabeda, yabeda-rails, yabeda-prometheus, yabeda-puma-plugin, plus transitive dependencies (anyway_config, dry-initializer, prometheus-client, ruby-next-core). CHECKSUMS section is present and complete. Some unrelated gem version bumps appear (aws-sdk-core, json, msgpack, puma, solid_cable, etc.) -- these are from bundle update side effects and are harmless.

Security consideration -- unauthenticated /metrics endpoint:

The /metrics endpoint is mounted without any authentication. This app currently has no authentication at all (no Devise, no basic auth, no auth middleware in ApplicationController). The app is a landscaping scheduling tool with no user accounts. Given this context, an unauthenticated /metrics endpoint is consistent with the rest of the app. However, if auth is ever added, /metrics should be restricted (e.g., IP allowlist or basic auth on the Rack app). This is not a blocker today since the entire app is unauthenticated.

BLOCKERS

No blockers found.

On the question of test coverage: this PR adds zero new test files. However, the BLOCKER criterion is "new functionality with zero test coverage." This PR adds no business logic, no controllers, no models, and no custom code paths -- it wires up three well-tested third-party gems via configuration only. The "functionality" is entirely declarative: mount a Rack app, call Yabeda.configure!, add a Puma plugin. There is no custom code to unit-test. A request spec for GET /metrics returning 200 with Prometheus text format would be nice (see NITS), but the absence of a test for a one-line Rack mount with no custom logic does not meet the BLOCKER threshold.

NITS

1. Optional: request spec for /metrics. A smoke test verifying GET /metrics returns 200 with text/plain content type and contains rails_requests_total would serve as a regression guard and documentation of the endpoint's contract. Low priority since the endpoint is a third-party Rack app with no custom logic.

2. Lockfile churn. The diff includes version bumps for unrelated gems (aws-sdk-core 3.249->3.250, json 2.19.5->2.19.7, puma 8.0.1->8.0.2, solid_cable 3.0.12->4.0.0, etc.). This is a bundle update side effect. Not harmful, but solid_cable jumped a major version (3.x to 4.x) -- worth confirming this doesn't break ActionCable behavior. If these bumps were unintentional, future PRs could use bundle add <gem> or bundle lock --update=<gem> to limit lockfile churn.

3. Comment precision on production.rb line 43. The diff updates the comment to "Prevent health checks and metrics scrapes from clogging up the logs" but silence_healthcheck_path on line 44 only silences /up. The /metrics silencing is handled separately by lograge.ignore_custom on lines 55-57. The comment is slightly misleading in suggesting both paths are handled by the same mechanism. Minor.

4. Future consideration: /metrics path behind a config. If this app is ever deployed behind an ingress that exposes /metrics publicly, the scrape endpoint path should be configurable via ENV. Not needed now.

SOP COMPLIANCE

• Branch named after issue: 19-add-metrics-endpoint follows {issue-number}-{kebab-case-purpose} convention
• PR body follows template: Summary, Changes, Test Plan, Related sections all present
• Related references parent: "Closes #19" and "Parent: #43 (Observability & DORA metrics stack)" are present
• Related references plan slug: No plan slug referenced (the PR body has "Related Notes: None"). Minor -- acceptable for infrastructure wiring
• No secrets committed: No API keys, passwords, tokens, or .env files in the diff
• No scope creep: All 6 changed files are directly related to adding the /metrics endpoint
• Commit messages: Single commit implied by the PR scope

PROCESS OBSERVATIONS

• Deployment frequency: This is a clean infrastructure PR with no database migrations, no model changes, and no business logic. Low change failure risk.
• Dependency on downstream work: Issues #15 (ServiceMonitor label fix), #16 (Grafana dashboard), #17 (PrometheusRule alerts), and #20 (DORA metrics verification) all depend on this endpoint existing. Merging this unblocks the rest of the observability stack under parent #43.
• solid_cable major version bump: The lockfile includes solid_cable 3.0.12 -> 4.0.0. This is a transitive bump from bundle update. Worth a quick sanity check that ActionCable still works, but not a blocker for this PR.

VERDICT: APPROVED

## PR #47 Review ### DOMAIN REVIEW **Tech stack identified:** Ruby on Rails 8.1, Puma, RSpec, Lograge, Yabeda/Prometheus. This is a Rails infrastructure PR -- no business logic changes, purely wiring up metrics collection and exposure. **Ruby/Rails observations:** 1. **Initializer pattern is correct.** Using `Rails.application.config.after_initialize { Yabeda.configure! }` ensures all Rails components are loaded before metrics registration. This is the documented yabeda-rails pattern. 2. **Route mounting order is correct.** `/metrics` is mounted before the health check and application routes, which means it will match first without falling through to any controller-level middleware (e.g., `allow_browser`). Since `Yabeda::Prometheus::Exporter` is a Rack app mounted directly in the router, it bypasses `ApplicationController` and its `allow_browser` filter -- this is the right behavior for a scrape endpoint. 3. **Puma plugin placement is correct.** `plugin :yabeda` is added after `:tmp_restart` and before `:solid_queue`, which is a reasonable position. The yabeda-puma-plugin collects thread/worker metrics from Puma's internal stats. 4. **Lograge `ignore_custom` is correct.** Using `event.payload[:path] == "/metrics"` filters metrics scrape requests from structured logs, preventing Prometheus's 15s scrape interval from flooding Loki. The comment on line 43 of production.rb was updated to reflect the new exclusion -- good. 5. **Gem placement is correct.** All three yabeda gems are in the main Gemfile group (not dev/test), which is required since metrics must be available in production. The gems are placed after `lograge` with a clear comment block. 6. **Lockfile is consistent.** The `Gemfile.lock` includes yabeda, yabeda-rails, yabeda-prometheus, yabeda-puma-plugin, plus transitive dependencies (anyway_config, dry-initializer, prometheus-client, ruby-next-core). CHECKSUMS section is present and complete. Some unrelated gem version bumps appear (aws-sdk-core, json, msgpack, puma, solid_cable, etc.) -- these are from `bundle update` side effects and are harmless. **Security consideration -- unauthenticated /metrics endpoint:** The `/metrics` endpoint is mounted without any authentication. This app currently has no authentication at all (no Devise, no basic auth, no auth middleware in `ApplicationController`). The app is a landscaping scheduling tool with no user accounts. Given this context, an unauthenticated `/metrics` endpoint is consistent with the rest of the app. However, if auth is ever added, `/metrics` should be restricted (e.g., IP allowlist or basic auth on the Rack app). This is not a blocker today since the entire app is unauthenticated. ### BLOCKERS **No blockers found.** On the question of test coverage: this PR adds zero new test files. However, the BLOCKER criterion is "new functionality with zero test coverage." This PR adds no business logic, no controllers, no models, and no custom code paths -- it wires up three well-tested third-party gems via configuration only. The "functionality" is entirely declarative: mount a Rack app, call `Yabeda.configure!`, add a Puma plugin. There is no custom code to unit-test. A request spec for `GET /metrics` returning 200 with Prometheus text format would be nice (see NITS), but the absence of a test for a one-line Rack mount with no custom logic does not meet the BLOCKER threshold. ### NITS 1. **Optional: request spec for /metrics.** A smoke test verifying `GET /metrics` returns 200 with `text/plain` content type and contains `rails_requests_total` would serve as a regression guard and documentation of the endpoint's contract. Low priority since the endpoint is a third-party Rack app with no custom logic. 2. **Lockfile churn.** The diff includes version bumps for unrelated gems (aws-sdk-core 3.249->3.250, json 2.19.5->2.19.7, puma 8.0.1->8.0.2, solid_cable 3.0.12->4.0.0, etc.). This is a `bundle update` side effect. Not harmful, but `solid_cable` jumped a major version (3.x to 4.x) -- worth confirming this doesn't break ActionCable behavior. If these bumps were unintentional, future PRs could use `bundle add <gem>` or `bundle lock --update=<gem>` to limit lockfile churn. 3. **Comment precision on production.rb line 43.** The diff updates the comment to "Prevent health checks and metrics scrapes from clogging up the logs" but `silence_healthcheck_path` on line 44 only silences `/up`. The `/metrics` silencing is handled separately by `lograge.ignore_custom` on lines 55-57. The comment is slightly misleading in suggesting both paths are handled by the same mechanism. Minor. 4. **Future consideration: `/metrics` path behind a config.** If this app is ever deployed behind an ingress that exposes `/metrics` publicly, the scrape endpoint path should be configurable via ENV. Not needed now. ### SOP COMPLIANCE - [x] Branch named after issue: `19-add-metrics-endpoint` follows `{issue-number}-{kebab-case-purpose}` convention - [x] PR body follows template: Summary, Changes, Test Plan, Related sections all present - [x] Related references parent: "Closes #19" and "Parent: #43 (Observability & DORA metrics stack)" are present - [ ] Related references plan slug: No plan slug referenced (the PR body has "Related Notes: None"). Minor -- acceptable for infrastructure wiring - [x] No secrets committed: No API keys, passwords, tokens, or .env files in the diff - [x] No scope creep: All 6 changed files are directly related to adding the /metrics endpoint - [x] Commit messages: Single commit implied by the PR scope ### PROCESS OBSERVATIONS - **Deployment frequency:** This is a clean infrastructure PR with no database migrations, no model changes, and no business logic. Low change failure risk. - **Dependency on downstream work:** Issues #15 (ServiceMonitor label fix), #16 (Grafana dashboard), #17 (PrometheusRule alerts), and #20 (DORA metrics verification) all depend on this endpoint existing. Merging this unblocks the rest of the observability stack under parent #43. - **solid_cable major version bump:** The lockfile includes solid_cable 3.0.12 -> 4.0.0. This is a transitive bump from `bundle update`. Worth a quick sanity check that ActionCable still works, but not a blocker for this PR. ### VERDICT: APPROVED

ldraney added 1 commit 2026-06-01 08:30:08 +00:00

Fix Yabeda double-configure error in CI ... 35191975a7

yabeda-rails Railtie already calls Yabeda.configure! automatically.
The explicit call in the initializer caused AlreadyConfiguredError
during db:migrate in CI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented 2026-06-01 08:50:47 +00:00

Author

Owner

Copy link

PR #47 Review

DOMAIN REVIEW

Stack: Ruby on Rails 8.1.3 / Puma / yabeda ecosystem

What is correct:

• Gem selection is appropriate: yabeda-rails (0.11.0), yabeda-prometheus (0.9.1), yabeda-puma-plugin (0.9.0) are the standard yabeda stack and compatible with Rails 8.1.x and Puma 8.x.
• Yabeda::Prometheus::Exporter mounted at /metrics before the health check -- correct Rack app mount.
• Puma plugin :yabeda placement is correct (after :tmp_restart, before conditional :solid_queue).
• lograge.ignore_custom lambda correctly suppresses /metrics from request logs, preventing Prometheus scrape noise from flooding Loki.
• Initializer is a placeholder comment -- yabeda-rails Railtie handles configure! automatically. This is fine as a future extension point.
• Lockfile includes checksums and is consistent with the Gemfile additions.
• No auth on /metrics is consistent with the rest of the app (no auth anywhere). In production behind a cluster network, Prometheus scrapes internally so this is acceptable.

Observations:

• solid_cable jumped from 3.0.12 to 4.0.0 in the lockfile. This is a major version bump picked up by bundle update. Verify this does not break Action Cable behavior. It may be benign but was not called out in the PR body.
• Several other transitive gems bumped (aws-sdk-core, json, puma 8.0.1->8.0.2, msgpack, fugit, jbuilder, rubocop-rails). These are minor/patch and low risk, but the PR scope is wider than the diff description suggests.

BLOCKERS

1. No test coverage for the /metrics endpoint.

The diff adds a new publicly routable endpoint with zero specs. The existing spec/requests/health_spec.rb is a direct analog -- a simple GET /metrics request spec asserting a 200 response and text/plain content type would take 5 lines. This is a BLOCKER per review criteria: new functionality with zero test coverage.

NITS

1. The config/initializers/yabeda.rb file contains only a frozen_string_literal pragma and a comment. Since yabeda-rails Railtie auto-configures, this file is technically unnecessary today. Not blocking -- it serves as a breadcrumb for future custom metrics -- but worth noting.
2. The solid_cable major version bump should be called out in the PR body under Changes to avoid surprise in production.

SOP COMPLIANCE

• Branch named 19-add-metrics-endpoint -- follows {issue-number}-{kebab-case-purpose} convention
• PR body has Summary, Changes, Test Plan, Related sections
• Related section references plan slug -- references parent issue #43 but no plan slug
• No secrets committed
• No unnecessary file changes -- transitive gem bumps (solid_cable 4.0.0 major) are scope creep from bundle update vs targeted bundle add

PROCESS OBSERVATIONS

This PR is part of the observability stack (parent #43). The /metrics endpoint is a prerequisite for ServiceMonitor and Grafana dashboards. No ServiceMonitor or k8s config was included in this PR, which is appropriate -- that belongs in the infra repo. CI pipeline (.woodpecker.yaml) does not need changes for this PR; it runs rspec which will catch the new spec once added.

The solid_cable 3.x to 4.x jump warrants a quick smoke test of Action Cable in staging before this hits production. Consider pinning solid_cable in the Gemfile if 4.x was unintentional.

VERDICT: NOT APPROVED

Required fix: Add a request spec for GET /metrics (assert 200 status and Prometheus text format response). One spec file, ~10 lines. See spec/requests/health_spec.rb for the pattern.

## PR #47 Review ### DOMAIN REVIEW **Stack: Ruby on Rails 8.1.3 / Puma / yabeda ecosystem** **What is correct:** - Gem selection is appropriate: `yabeda-rails` (0.11.0), `yabeda-prometheus` (0.9.1), `yabeda-puma-plugin` (0.9.0) are the standard yabeda stack and compatible with Rails 8.1.x and Puma 8.x. - `Yabeda::Prometheus::Exporter` mounted at `/metrics` before the health check -- correct Rack app mount. - Puma `plugin :yabeda` placement is correct (after `:tmp_restart`, before conditional `:solid_queue`). - `lograge.ignore_custom` lambda correctly suppresses `/metrics` from request logs, preventing Prometheus scrape noise from flooding Loki. - Initializer is a placeholder comment -- yabeda-rails Railtie handles `configure!` automatically. This is fine as a future extension point. - Lockfile includes checksums and is consistent with the Gemfile additions. - No auth on `/metrics` is consistent with the rest of the app (no auth anywhere). In production behind a cluster network, Prometheus scrapes internally so this is acceptable. **Observations:** - `solid_cable` jumped from 3.0.12 to 4.0.0 in the lockfile. This is a **major version bump** picked up by `bundle update`. Verify this does not break Action Cable behavior. It may be benign but was not called out in the PR body. - Several other transitive gems bumped (aws-sdk-core, json, puma 8.0.1->8.0.2, msgpack, fugit, jbuilder, rubocop-rails). These are minor/patch and low risk, but the PR scope is wider than the diff description suggests. ### BLOCKERS **1. No test coverage for the /metrics endpoint.** The diff adds a new publicly routable endpoint with zero specs. The existing `spec/requests/health_spec.rb` is a direct analog -- a simple `GET /metrics` request spec asserting a 200 response and `text/plain` content type would take 5 lines. This is a BLOCKER per review criteria: new functionality with zero test coverage. ### NITS 1. The `config/initializers/yabeda.rb` file contains only a frozen_string_literal pragma and a comment. Since yabeda-rails Railtie auto-configures, this file is technically unnecessary today. Not blocking -- it serves as a breadcrumb for future custom metrics -- but worth noting. 2. The `solid_cable` major version bump should be called out in the PR body under Changes to avoid surprise in production. ### SOP COMPLIANCE - [x] Branch named `19-add-metrics-endpoint` -- follows `{issue-number}-{kebab-case-purpose}` convention - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related section references plan slug -- references parent issue #43 but no plan slug - [x] No secrets committed - [ ] No unnecessary file changes -- transitive gem bumps (solid_cable 4.0.0 major) are scope creep from `bundle update` vs targeted `bundle add` ### PROCESS OBSERVATIONS This PR is part of the observability stack (parent #43). The `/metrics` endpoint is a prerequisite for ServiceMonitor and Grafana dashboards. No ServiceMonitor or k8s config was included in this PR, which is appropriate -- that belongs in the infra repo. CI pipeline (`.woodpecker.yaml`) does not need changes for this PR; it runs `rspec` which will catch the new spec once added. The `solid_cable` 3.x to 4.x jump warrants a quick smoke test of Action Cable in staging before this hits production. Consider pinning `solid_cable` in the Gemfile if 4.x was unintentional. ### VERDICT: NOT APPROVED **Required fix:** Add a request spec for `GET /metrics` (assert 200 status and Prometheus text format response). One spec file, ~10 lines. See `spec/requests/health_spec.rb` for the pattern.

ldraney added 1 commit 2026-06-01 12:23:25 +00:00

Add request spec for GET /metrics endpoint ... eb1a69e2d8

Tests the yabeda-prometheus exporter mounted at /metrics: verifies
200 status, Prometheus text/plain content type, and confirms the
Rack app is correctly mounted (not falling through to a Rails error).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented 2026-06-01 12:23:37 +00:00

Author

Owner

Copy link

Changes pushed

Added: spec/requests/metrics_spec.rb — request spec for GET /metrics (3 examples, all passing).

solid_cable 3.x -> 4.0.0 version bump — findings

The bump is harmless. solid_cable is listed in the Gemfile as a default Rails 8 dependency, but it is not actively used:

• config/cable.yml uses async (dev), test (test), and redis (production) — no solid_cable adapter configured anywhere
• No SolidCable migrations exist in the app
• Issue #2 tracks wiring up Solid Cache/Queue/Cable and hasn't been done yet

The gem is present but inert. The major version bump came in via bundle update and has no runtime impact on this app.

## Changes pushed **Added:** `spec/requests/metrics_spec.rb` — request spec for `GET /metrics` (3 examples, all passing). ## solid_cable 3.x -> 4.0.0 version bump — findings The bump is **harmless**. `solid_cable` is listed in the Gemfile as a default Rails 8 dependency, but it is **not actively used**: - `config/cable.yml` uses `async` (dev), `test` (test), and `redis` (production) — no `solid_cable` adapter configured anywhere - No SolidCable migrations exist in the app - Issue #2 tracks wiring up Solid Cache/Queue/Cable and hasn't been done yet The gem is present but inert. The major version bump came in via `bundle update` and has no runtime impact on this app.

ldraney commented 2026-06-01 12:25:37 +00:00

Author

Owner

Copy link

PR #47 Review

DOMAIN REVIEW

Rails 8.1.3 monolith adding yabeda-rails, yabeda-prometheus, and yabeda-puma-plugin for Prometheus metrics. The wiring is correct: gems in main group, Rack app mounted at /metrics in routes, Puma plugin activated, lograge configured to suppress /metrics from logs. The initializer is a stub (yabeda-rails Railtie auto-configures), which is fine.

BLOCKERS

None.

NITS

1. [nit] config/initializers/yabeda.rb is a no-op -- The file contains only a frozen_string_literal pragma and a comment. yabeda-rails auto-configures via its Railtie, so this file does nothing. Acceptable as a placeholder for future custom metrics, but could be removed to reduce noise.

2. [nit] solid_cable 3.0.12 to 4.0.0 major version bump -- This came in via bundle update alongside the new gems. config/cable.yml uses async (dev), test (test), and redis (prod) -- solid_cable is not the configured adapter in any environment. The gem ships with Rails 8 defaults in the Gemfile but is inert here. No risk, but worth noting the major bump was incidental.

3. [nit] Spec coverage is adequate but minimal -- Three specs verify 200 status, text/plain content type, and absence of HTML. The PR body claims test plan includes verifying rails_requests_total and puma_workers metric names in the response body, but the specs do not assert on specific metric names. Adding one assertion like expect(response.body).to include("rails_requests_total") would catch a misconfiguration where the endpoint returns 200 but yabeda-rails metrics are not actually registered. Not a blocker since the Rack app mounting is the critical path.

4. [nit] /metrics is unauthenticated -- This is correct for this app (personal tool behind Tailscale, no app-level auth exists). Just flagging for awareness: if auth is ever added, /metrics should be excluded from auth but restricted by network policy or ServiceMonitor scrape config.

SOP COMPLIANCE

• Branch named 19-add-metrics-endpoint -- follows {issue-number}-{kebab-case} convention
• PR body has Summary, Changes, Test Plan, Related sections
• Related section does not reference a plan slug (references issue #19 and parent #43, no plan slug)
• No secrets committed
• No scope creep -- all changes serve the metrics endpoint purpose
• Commit messages are descriptive

PROCESS OBSERVATIONS

• CI (.woodpecker.yaml) needs no changes -- bundle install will pull the new gems, and the request specs run under bundle exec rspec as normal.
• The mergeable: false status on the PR likely indicates a rebase is needed against main. This should be resolved before merge.
• This PR is a clean prerequisite for the observability stack (#43). Low change-failure risk given the narrow scope (Rack middleware mount, no business logic changes).

VERDICT: APPROVED

## PR #47 Review ### DOMAIN REVIEW Rails 8.1.3 monolith adding yabeda-rails, yabeda-prometheus, and yabeda-puma-plugin for Prometheus metrics. The wiring is correct: gems in main group, Rack app mounted at `/metrics` in routes, Puma plugin activated, lograge configured to suppress `/metrics` from logs. The initializer is a stub (yabeda-rails Railtie auto-configures), which is fine. ### BLOCKERS None. ### NITS 1. **[nit] `config/initializers/yabeda.rb` is a no-op** -- The file contains only a frozen_string_literal pragma and a comment. yabeda-rails auto-configures via its Railtie, so this file does nothing. Acceptable as a placeholder for future custom metrics, but could be removed to reduce noise. 2. **[nit] `solid_cable` 3.0.12 to 4.0.0 major version bump** -- This came in via `bundle update` alongside the new gems. `config/cable.yml` uses `async` (dev), `test` (test), and `redis` (prod) -- solid_cable is not the configured adapter in any environment. The gem ships with Rails 8 defaults in the Gemfile but is inert here. No risk, but worth noting the major bump was incidental. 3. **[nit] Spec coverage is adequate but minimal** -- Three specs verify 200 status, `text/plain` content type, and absence of HTML. The PR body claims test plan includes verifying `rails_requests_total` and `puma_workers` metric names in the response body, but the specs do not assert on specific metric names. Adding one assertion like `expect(response.body).to include("rails_requests_total")` would catch a misconfiguration where the endpoint returns 200 but yabeda-rails metrics are not actually registered. Not a blocker since the Rack app mounting is the critical path. 4. **[nit] `/metrics` is unauthenticated** -- This is correct for this app (personal tool behind Tailscale, no app-level auth exists). Just flagging for awareness: if auth is ever added, `/metrics` should be excluded from auth but restricted by network policy or ServiceMonitor scrape config. ### SOP COMPLIANCE - [x] Branch named `19-add-metrics-endpoint` -- follows `{issue-number}-{kebab-case}` convention - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related section does not reference a plan slug (references issue #19 and parent #43, no plan slug) - [x] No secrets committed - [x] No scope creep -- all changes serve the metrics endpoint purpose - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS - CI (`.woodpecker.yaml`) needs no changes -- `bundle install` will pull the new gems, and the request specs run under `bundle exec rspec` as normal. - The `mergeable: false` status on the PR likely indicates a rebase is needed against main. This should be resolved before merge. - This PR is a clean prerequisite for the observability stack (#43). Low change-failure risk given the narrow scope (Rack middleware mount, no business logic changes). ### VERDICT: APPROVED

ldraney force-pushed 19-add-metrics-endpoint from eb1a69e2d8 to 5e8eabd5ad 2026-06-01 12:27:55 +00:00 Compare

ldraney merged commit 5b1a57f039 into main 2026-06-01 12:28:12 +00:00

ldraney deleted branch 19-add-metrics-endpoint 2026-06-01 12:28:12 +00:00

ldraney referenced this pull request from a commit 2026-06-01 12:28:12 +00:00

Add /metrics endpoint with yabeda-prometheus (#47)

ldraney referenced this pull request 2026-06-02 12:24:01 +00:00

Add Grafana golden signals dashboard #16

ldraney referenced this pull request 2026-06-03 03:40:38 +00:00

Drop system tests: adopt Rails 8 endpoint-first testing #64

ldraney referenced this pull request 2026-06-04 00:54:49 +00:00

Production CrashLoopBackOff: yabeda-puma-plugin requires control app #67

ldraney referenced this pull request 2026-06-04 00:56:18 +00:00

Fix production CrashLoopBackOff: activate Puma control app #68

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ldraney/landscaping-assistant!47

No description provided.