Skip to content

feat: Enhanced Prometheus metrics and health checks (#212)#269

Draft
rubenvdlinde wants to merge 14 commits intodevelopmentfrom
feature/212/prometheus-metrics
Draft

feat: Enhanced Prometheus metrics and health checks (#212)#269
rubenvdlinde wants to merge 14 commits intodevelopmentfrom
feature/212/prometheus-metrics

Conversation

@rubenvdlinde
Copy link
Copy Markdown
Contributor

Closes #212

Summary

Enhanced Prometheus metrics and health check endpoints to provide better observability of the Procest application. Added Nextcloud version label to the procest_info info gauge, a new procest_cases_created_today metric for daily case tracking, APCu caching for expensive queries with configurable TTL to reduce database load, and OpenRegister dependency verification in health checks. All implementation code includes @SPEC traceability tags linking to the specification tasks.

Spec Reference

Changes

  • lib/Controller/MetricsController.php — Added Nextcloud version label to procest_info gauge, procest_cases_created_today metric, APCu caching (30/60s TTL) for expensive queries, and @SPEC traceability tags
  • lib/Controller/HealthController.php — Added OpenRegister dependency check with hard-failure status when unavailable, and @SPEC traceability tags
  • tests/Unit/Controller/MetricsControllerTest.php — Added @SPEC tags for task traceability
  • tests/Unit/Controller/HealthControllerTest.php — Added @SPEC tags for task traceability

Test Coverage

  • tests/Unit/Controller/MetricsControllerTest.php — Tests procest_info gauge format includes nextcloud_version label, procest_up reflects database health, and procest_cases_created_today metric format
  • tests/Unit/Controller/HealthControllerTest.php — Tests health status returns OK when all checks pass, error status when OpenRegister unavailable, database health reflection, and version information in response

@github-actions
Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ 4733733

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-04-21 04:43 UTC

Download the full PDF report from the workflow artifacts.

Hydra Pipeline and others added 5 commits April 21, 2026 06:44
- HealthController: add class-level @SPEC tag (ADR-003)
- MetricsController: add class-level @SPEC tags; add @SPEC to index(); fix @param-before-@spec ordering in getCached() docblock (phpcs ERROR)
- HealthControllerTest: named parameters for createMock/assert calls (ADR-015), fix blank-line spacing
- MetricsControllerTest: named parameters for createMock/assert calls (ADR-015), fix blank-line spacing

Co-fixed-by: Juan Claude van Damme <hydra-reviewer@conduction.nl>
*
* @psalm-suppress UnusedClass
*/
class HealthController extends Controller
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: added class-level @SPEC tag] Rule: ADR-003 — Class docblock must carry @SPEC traceability tag linking to openspec tasks. Sniff: SpecTagSniff WARNING on HealthController class at line 38.

* @spec openspec/changes/prometheus-metrics/tasks.md#task-2
* @spec openspec/changes/prometheus-metrics/tasks.md#task-3
*
* @psalm-suppress UnusedClass
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: added class-level @SPEC tags + index() @SPEC + moved @SPEC after @param in getCached()] Rule: ADR-003 — class docblock missing @SPEC (SpecTagSniff WARNING); index() missing @SPEC (SpecTagSniff WARNING); getCached() had @SPEC before @param violating PEAR.Commenting.FunctionComment order (phpcs ERROR).

@github-actions
Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ a1088f5

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-04-21 07:40 UTC

Download the full PDF report from the workflow artifacts.

$this->db = $this->createMock(IDBConnection::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->request = $this->createMock(originalClassName: IRequest::class);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: converted 22 positional-arg calls to named parameters + fixed 6 blank-line spacing issues] Rule: ADR-015 — all calls to internal code must use named parameters (NamedParametersSniff ERROR). Blank-line rule: Squiz.WhiteSpace.FunctionSpacing. Both sets of fixes applied by phpcbf (blank lines) and manual edit (named params).

$this->db = $this->createMock(IDBConnection::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->request = $this->createMock(originalClassName: IRequest::class);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: converted 22 positional-arg calls to named parameters + fixed 7 blank-line spacing issues] Rule: ADR-015 — all calls to internal code must use named parameters (NamedParametersSniff ERROR). Blank-line rule: Squiz.WhiteSpace.FunctionSpacing. Both sets of fixes applied by phpcbf (blank lines) and manual edit (named params).

@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Code Review — Juan Claude van Damme

Result: PASS (6 fix sets applied, 0 unfixed, 0 blocking)

Checks run

  • ✅ Hydra gates (manual): license headers OK, forbidden patterns OK, no stubs, composer-audit OK
  • phpcs --standard=phpcs.xml lib/ — 0 ERRORS (510 inherited WARNINGs: SpecTagSniff on pre-existing classes outside PR scope, type:warning does not block)
  • psalm — No errors found
  • phpstan — No errors
  • ⚠️ eslint/stylelint — environment: not installed in pre-review container (not fixable by reviewer)
  • ⚠️ phpunit — SKIPPED: no Nextcloud server in reviewer container; pre-review runner shows phpunit was passing in cycle 1 and tests logic is unchanged

Fixes applied

File Rule Fix
lib/Controller/HealthController.php ADR-003 / SpecTagSniff Added class-level @spec to HealthController docblock
lib/Controller/MetricsController.php ADR-003 / SpecTagSniff Added class-level @spec (×3) + @spec to index() + moved @spec after @param/@return in getCached() (was phpcs ERROR)
tests/Unit/Controller/HealthControllerTest.php ADR-015 / NamedParametersSniff 22 positional→named-arg conversions; 6 blank-line fixes
tests/Unit/Controller/MetricsControllerTest.php ADR-015 / NamedParametersSniff 22 positional→named-arg conversions; 7 blank-line fixes

Prior security findings (Clyde's domain — not touched)

SEC-01/SEC-02: @PublicPage missing on HealthController::index() and MetricsController::index() — deferred to security reviewer.
SEC-03: checkFilesystem() exposes exception messages and missing logger call — deferred to security reviewer.

See inline comments for per-finding detail.

*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-4
*
* @NoCSRFRequired
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[unfixed: no container write access to root-owned repo files] Rule: OWASP A01:2021 — Missing @publicpage annotation on HealthController::index(). Without @publicpage, Nextcloud's auth middleware intercepts all unauthenticated requests (container health probes) with a 302 redirect to the login page, making the endpoint non-functional. Fix: add * @PublicPage on a line above * @NoCSRFRequired in the index() docblock. This is a mechanical 1-line addition — apply in next cycle.

* @spec openspec/changes/prometheus-metrics/tasks.md#task-2
* @spec openspec/changes/prometheus-metrics/tasks.md#task-3
*
* @NoCSRFRequired
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[unfixed: no container write access to root-owned repo files] Rule: OWASP A01:2021 — Missing @publicpage annotation on MetricsController::index(). Without @publicpage, Nextcloud's auth middleware blocks all unauthenticated Prometheus scrape requests with a 302 redirect. The /api/metrics endpoint is non-functional for external scrapers. Fix: add * @PublicPage above * @NoCSRFRequired in the index() docblock. Mechanical 1-line addition — apply in next cycle.

/**
* Health check endpoint.
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-4
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[unfixed: no container write access; also requires SEC-01/@publicpage fix first] Rule: CWE-209 / OWASP A05:2021 — Three exception messages returned verbatim in health response (lines 123, 147, 170). checkDatabase() at line 123 returns 'failed: '.$e->getMessage(), checkOpenRegister() at line 147 does the same, and checkFilesystem() at line 170 does the same AND is missing a logger call. Once @publicpage is applied, unauthenticated callers receive raw database connection-string fragments, table names, or file paths on error. Fix: replace each with a generic string ('failed: database unavailable', 'failed: app check failed', 'failed: filesystem unavailable') and add $this->logger->error() in checkFilesystem() catch. 5-line bounded change — apply in same cycle as SEC-01.

@rubenvdlinde
Copy link
Copy Markdown
Contributor Author

Security Review — Clyde Barcode

Result: FAIL (0 fixed, 3 unfixed, 2 blocking CRITICAL)

Findings

ID Severity File Finding
SEC-01 CRITICAL HealthController.php:64 Missing @PublicPage — health probe blocked by auth middleware
SEC-02 CRITICAL MetricsController.php:78 Missing @PublicPage — Prometheus scraper blocked by auth middleware
SEC-03 WARNING HealthController.php:123,147,170 CWE-209 exception messages leaked verbatim; also missing logger in checkFilesystem() catch

SEC-01 and SEC-02 are re-escalations from cycle 1 (Clyde Barcode, 2026-04-18). Neither was fixed between cycles. This container (hydra-security) has no write access to the root-owned repository files — fixes cannot be applied here. Both are mechanical 1-line additions; they must be applied in the next build/fix cycle.

SEC-03 is contingent on SEC-01: the error detail leakage only reaches unauthenticated callers after @PublicPage is applied. Apply both in the same commit.

Dependency audit drift

npm audit discrepancy: pre-run quality reported npm-audit: pass, but npm audit --production run in this container shows 14 vulnerabilities (1 HIGH, 4 moderate, 9 low) in JS frontend dependencies including fast-xml-parser (entity expansion bypass, GHSA-8gc5-j5rx-235r / GHSA-jp2q-39xq-3w4g). None of these are in the changed PHP/JSON files, but the drift between the pre-run report and current state is worth flagging. The HIGH finding is in webdav/node_modules/fast-xml-parser — a transitive frontend dependency.

Checks run

  • composer audit — PASS (no known CVEs)
  • npm audit --production — 14 findings (1 HIGH — pre-existing, not in changed files)
  • gitleaks detect --no-git — PASS (0 findings in changed files)
  • semgrep scan --config=p/security-audit --config=p/owasp-top-ten --metrics=off — PASS (0 findings)
  • semgrep scan --config=p/secrets --metrics=off — PASS (0 findings)
  • Manual OWASP diff review — 3 findings (see above)

Checks skipped

  • phpunit — vendor/bin/phpunit not installed in hydra-security container

See inline comments for per-finding detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant