feat(#13): expose Prometheus metrics via /metrics endpoint

[fullsend-ai-coder]

Add prometheus_client instrumentation to the Flask app with before_request/after_request hooks that track:

• http_requests_total (Counter): request count per method,
  endpoint, and HTTP status code
• http_request_duration_seconds (Histogram): request duration
  per method and endpoint, enabling average calculation via
  sum/count
• http_request_duration_max_seconds (Gauge): maximum observed
  request duration per method and endpoint
• http_request_duration_min_seconds (Gauge): minimum observed
  request duration per method and endpoint

The /metrics endpoint itself is excluded from instrumentation to avoid recursive inflation of counters.

Added prometheus_client>=0.20 to requirements.txt and pyproject.toml. Added 7 new tests covering the metrics endpoint response, request counting, duration tracking, status code labels, min/max gauges, and the /metrics exclusion behavior.

---

Closes #13

Post-script verification

• Branch is not main/master (agent/13-prometheus-metrics)
• Secret scan passed (gitleaks — 47f7f1511df8ad7036c082d9b2179085cd9ec107..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

Review

Findings

Low

• [edge-case] test_app.py:25 — The _reset_collectors function clears metric state by accessing the private _metrics attribute of prometheus_client collector objects (metric._metrics.clear()) and directly imports/clears the private _extremes dict. This internal API is not part of the prometheus_client public contract and could break with a future library version, silently breaking test isolation.
  Remediation: Consider adding a public reset_metrics() function in metrics.py that encapsulates clearing both the prometheus collectors and _extremes, similar to how reset_items() exists in app.py.

• [naming-convention] test_app.py:22 — The _reset_collectors helper function uses an underscore prefix, which is marginally inconsistent with the reset_items() pattern in app.py. A leading underscore reasonably signals "not a test function" to pytest discovery, but removing it would be more consistent.
  Remediation: Consider removing the underscore prefix for consistency, or adding a comment explaining the convention.

• [missing-doc] README.md:14 — The /metrics endpoint is listed in the endpoints table but the README does not describe what specific Prometheus metrics are exposed (http_requests_total, http_request_duration_seconds, etc.) or the new prometheus_client dependency.
  Remediation: Consider adding a brief note about the available metrics, either inline in the endpoint description or in a short section.

Previous run

Review

Findings

Medium

• [data-exposure] metrics.py:99 — The /metrics endpoint is registered without any authentication or access control. It exposes internal operational information including all route patterns, HTTP methods, response status codes, request counts, and request duration statistics. An attacker can enumerate every endpoint in the application and observe traffic patterns.
  Remediation: Protect the /metrics endpoint with authentication (e.g., a bearer token, basic-auth, or IP-allowlist), or serve metrics on a separate internal-only port/bind address not reachable from untrusted networks.

• [unbounded-cardinality] metrics.py:73 — The method label is taken directly from request.method, which can be any arbitrary string sent by a client (e.g., FOOBAR). Flask's after_request hook fires even for 405 responses, so an attacker can send requests with thousands of distinct HTTP method strings to create unbounded Prometheus time-series, exhausting memory in the metrics registry.
  Remediation: Normalize the method value to a fixed allow-list of known HTTP methods (GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS) and map everything else to a single sentinel like OTHER.

Low

• [test-fragility] test_app.py:22 — _reset_collectors() resets metric state by clearing the internal _metrics dict on each prometheus_client collector (e.g., REQUEST_COUNT._metrics.clear()). This attribute is not part of the prometheus_client public API. The dependency is pinned as >=0.20 with no upper bound, so a future version could rename or restructure this internal, causing all seven new metrics tests to fail with AttributeError.

• [architectural-pattern] metrics.py — The PR introduces a new module-level architecture pattern (separate metrics.py with init_metrics(app) function) that establishes precedent for how future cross-cutting concerns should be organized. Consider documenting this pattern to guide future development.

Previous run (2)

Review

Findings

Low

• [test-inadequate] test_app.py:23 — The _reset_collectors function clears prometheus_client metric state by accessing the internal _metrics attribute (metric._metrics.clear()). This is an undocumented internal API. With the open-ended version pin (>=0.20), a future prometheus_client release could change this internal structure, causing silent test pollution or noisy AttributeError failures. Consider creating a fresh CollectorRegistry per test or pinning the dependency more tightly (e.g., >=0.20,<1.0).

• [data-exposure] app.py:96 — The /metrics endpoint is served without authentication or access control. Prometheus metrics reveal internal endpoint paths, request volumes, timing characteristics, and HTTP status code distributions. For this demo application this is acceptable, but in a production deployment the endpoint should be restricted — e.g., served on a separate internal-only port, gated behind authentication, or limited via network policy to the Prometheus scraper.

Previous run (3)

Review

Findings

High

• [race-condition] app.py:62 — The min/max gauge update uses a non-atomic check-then-set pattern (current_max = max_gauge._value.get() followed by max_gauge.set(duration)) that is racy under multi-threaded WSGI servers. Two concurrent requests can both read current_max, both decide their duration is larger, and one silently overwrites the other. Additionally, ._value.get() accesses a private prometheus_client internal — there is no public API to read a Gauge's current value, which makes this pattern fragile across library upgrades.
  Remediation: Remove the manual min/max Gauges and rely on Histogram quantiles via PromQL, or protect the read-compare-set block with a threading lock per label set. If min/max Gauges are retained, maintain a separate dict for current extremes rather than reading back from the Gauge's private state.

Medium

• [unbounded-cardinality] app.py:55 — endpoint = request.path includes dynamic path segments (e.g. /items/1, /items/2, ...). Each unique path creates a distinct time-series across all four metrics, causing unbounded cardinality growth — a well-known Prometheus anti-pattern that leads to high memory usage and slow /metrics scrapes.
  Remediation: Use request.url_rule (yields the route template, e.g. /items/<int:item_id>) instead of request.path. Fall back to a constant like "unmatched" when url_rule is None (404s on unknown paths).
  See also: [data-exposure] finding at this location.

• [data-exposure] app.py:62 — The /metrics endpoint is unauthenticated and, combined with the unbounded cardinality above, allows an external attacker to enumerate all requested URL paths, observe per-route timing, and inflate memory via a cardinality-bomb (sending requests to many distinct paths).
  Remediation: Restrict /metrics access via authentication, IP allowlist, or serve metrics on a separate internal-only port. Bounding cardinality (see [unbounded-cardinality]) mitigates the DoS vector.
  See also: [unbounded-cardinality] finding at this location.

• [missing-doc] README.md:5 — The Endpoints table lists five routes but omits the newly added GET /metrics endpoint. A reader consulting this table would not know the metrics endpoint exists.
  Remediation: Add a row to the Endpoints table: | GET | /metrics | Prometheus metrics |.

Low

• [edge-case] app.py:63 — The min/max logic uses current == 0.0 as a sentinel for "not yet observed," relying on the Gauge's default value. This conflates "never observed" with a (theoretical) zero-duration request and is fragile if the Gauge default ever changes.

• [test-inadequate] test_app.py:120 — test_metrics_tracks_min_max_duration only asserts that metric names appear in the output; it does not verify that values are positive or that min ≤ max, so the compare-and-set logic is untested.

• [test-inadequate] test_app.py:8 — _reset_collectors() accesses REGISTRY._names_to_collectors and collector._metrics, both private APIs that may silently break on prometheus_client upgrades without causing test failures.

• [data-exposure] app.py:73 — The /metrics endpoint serves the global REGISTRY, which includes default process-level collectors (process_cpu_seconds_total, process_resident_memory_bytes, etc.), revealing internal runtime details.

[Allda]

/fs-fix

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 7 of 8 review findings. Fixed race condition with threading lock, bounded cardinality via url_rule, isolated metrics via custom registry, added README docs, improved test assertions. Disagreed with /metrics auth requirement as out of scope for this PR.

Fixed (7):

1. race-condition: non-atomic check-then-set in min/max gauge update using private API: Replaced the racy check-then-set pattern and private ._value.get() access with a threading lock protecting a separate _extremes dict that tracks current min/max values per (method, endpoint) key
2. unbounded-cardinality: request.path includes dynamic path segments: Changed endpoint label from request.path to request.url_rule.rule (route template), with fallback to 'unmatched' when url_rule is None
3. missing-doc: README endpoints table omits /metrics: Added GET /metrics row to the README endpoints table
4. edge-case: min/max logic uses 0.0 as sentinel for not-yet-observed: Addressed as part of the race-condition fix: the _extremes dict uses None (key absence) as the sentinel for not-yet-observed, eliminating the 0.0 ambiguity
5. test-inadequate: min/max test only checks metric names exist without verifying values: Added regex-based assertions to verify min/max values are positive floats and that min <= max
6. test-inadequate: _reset_collectors accesses REGISTRY._names_to_collectors private API: Replaced REGISTRY internal access with direct imports of the four metric objects and clearing their _metrics dicts, plus clearing the _extremes dict
7. data-exposure: /metrics serves global REGISTRY exposing process-level collectors: Created a custom CollectorRegistry and registered all metrics with it. The /metrics endpoint now serves generate_latest(registry) instead of the global REGISTRY, excluding process-level collectors

Disagreed (1):

1. data-exposure: /metrics endpoint is unauthenticated: Adding authentication or IP allowlisting to /metrics is out of scope for this PR which introduces basic metrics instrumentation. The unbounded-cardinality fix mitigates the DoS vector. Auth should be addressed in a follow-up PR.

Tests: passed

_{Updated by fullsend fix agent}

[Allda]

Please move the Prometheus-related code into a separate Python module.

[Allda]

/fs-fix

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Moved all Prometheus-related code into a dedicated metrics.py module per human request. All 15 tests pass, lint clean.

Fixed (1):

1. move Prometheus-related code into a separate Python module: Extracted all Prometheus instrumentation (registry, metric objects, _extremes tracking, before/after request hooks, /metrics endpoint) from app.py into a new metrics.py module. The app.py now imports and calls init_metrics(app). Test imports updated to import metric objects from metrics instead of app.

Tests: passed

_{Updated by fullsend fix agent}

[ascerra]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:50 AM UTC · Completed 12:04 PM UTC

Commit: 724fbf2 · View workflow run →

Superseded by updated review

[fullsend-ai-review]

[low] edge-case

The _reset_collectors function clears metric state by accessing the private _metrics attribute of prometheus_client collector objects (metric._metrics.clear()) and directly imports/clears the private _extremes dict. This internal API is not part of the prometheus_client public contract and could break with a future library version, silently breaking test isolation.

Suggested fix: Consider adding a public reset_metrics() function in metrics.py that encapsulates clearing both the prometheus collectors and _extremes, similar to how reset_items() exists in app.py.

[fullsend-ai-review]

[low] naming-convention

The _reset_collectors helper function uses an underscore prefix, which is marginally inconsistent with the reset_items() pattern in app.py.

Suggested fix: Consider removing the underscore prefix for consistency, or adding a comment explaining the convention.

[fullsend-ai-review]

[low] missing-doc

The /metrics endpoint is listed in the endpoints table but the README does not describe what specific Prometheus metrics are exposed (http_requests_total, http_request_duration_seconds, etc.) or the new prometheus_client dependency.

Suggested fix: Consider adding a brief note about the available metrics, either inline in the endpoint description or in a short section.