-
Notifications
You must be signed in to change notification settings - Fork 0
feat(#13): expose Prometheus metrics via /metrics endpoint #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| import threading | ||
| import time | ||
|
|
||
| from flask import g, request | ||
| from prometheus_client import ( | ||
| CollectorRegistry, | ||
| Counter, | ||
| Gauge, | ||
| Histogram, | ||
| generate_latest, | ||
| CONTENT_TYPE_LATEST, | ||
| ) | ||
|
|
||
| registry = CollectorRegistry() | ||
|
|
||
| REQUEST_COUNT = Counter( | ||
| "http_requests_total", | ||
| "Total HTTP request count", | ||
| ["method", "endpoint", "status"], | ||
| registry=registry, | ||
| ) | ||
|
|
||
| REQUEST_DURATION = Histogram( | ||
| "http_request_duration_seconds", | ||
| "HTTP request duration in seconds", | ||
| ["method", "endpoint"], | ||
| registry=registry, | ||
| ) | ||
|
|
||
| REQUEST_DURATION_MAX = Gauge( | ||
| "http_request_duration_max_seconds", | ||
| "Maximum HTTP request duration in seconds", | ||
| ["method", "endpoint"], | ||
| registry=registry, | ||
| ) | ||
|
|
||
| REQUEST_DURATION_MIN = Gauge( | ||
| "http_request_duration_min_seconds", | ||
| "Minimum HTTP request duration in seconds", | ||
| ["method", "endpoint"], | ||
| registry=registry, | ||
| ) | ||
|
|
||
| # Thread-safe tracking of observed min/max values per (method, endpoint). | ||
| _extremes_lock = threading.Lock() | ||
| _extremes: dict[tuple[str, str], tuple[float, float]] = {} | ||
|
|
||
|
|
||
| def init_metrics(app): | ||
| """Register Prometheus metrics hooks and /metrics endpoint on the app.""" | ||
|
|
||
| @app.before_request | ||
| def _start_timer(): | ||
| if request.path == "/metrics": | ||
| return | ||
| g.start_time = time.monotonic() | ||
|
|
||
| @app.after_request | ||
| def _record_metrics(response): | ||
| if request.path == "/metrics": | ||
| return response | ||
|
|
||
| start = g.pop("start_time", None) | ||
| if start is None: | ||
| return response | ||
|
|
||
| duration = time.monotonic() - start | ||
| rule = request.url_rule | ||
| endpoint = rule.rule if rule is not None else "unmatched" | ||
| method = request.method | ||
| status = str(response.status_code) | ||
|
|
||
| REQUEST_COUNT.labels(method=method, endpoint=endpoint, status=status).inc() | ||
| REQUEST_DURATION.labels(method=method, endpoint=endpoint).observe(duration) | ||
|
|
||
| key = (method, endpoint) | ||
| with _extremes_lock: | ||
| cur = _extremes.get(key) | ||
| if cur is None: | ||
| _extremes[key] = (duration, duration) | ||
| REQUEST_DURATION_MAX.labels(method=method, endpoint=endpoint).set( | ||
| duration | ||
| ) | ||
| REQUEST_DURATION_MIN.labels(method=method, endpoint=endpoint).set( | ||
| duration | ||
| ) | ||
| else: | ||
| cur_min, cur_max = cur | ||
| new_min = min(cur_min, duration) | ||
| new_max = max(cur_max, duration) | ||
| _extremes[key] = (new_min, new_max) | ||
| if new_max != cur_max: | ||
| REQUEST_DURATION_MAX.labels(method=method, endpoint=endpoint).set( | ||
| new_max | ||
| ) | ||
| if new_min != cur_min: | ||
| REQUEST_DURATION_MIN.labels(method=method, endpoint=endpoint).set( | ||
| new_min | ||
| ) | ||
|
|
||
| return response | ||
|
|
||
| @app.route("/metrics") | ||
| def metrics(): | ||
| return generate_latest(registry), 200, {"Content-Type": CONTENT_TYPE_LATEST} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| flask>=3.0 | ||
| prometheus_client>=0.20 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,36 @@ | ||
| import re | ||
|
|
||
| import pytest | ||
| from app import app, reset_items | ||
| from metrics import ( | ||
| REQUEST_COUNT, | ||
| REQUEST_DURATION, | ||
| REQUEST_DURATION_MAX, | ||
| REQUEST_DURATION_MIN, | ||
| _extremes, | ||
| ) | ||
|
|
||
| _ALL_METRICS = [ | ||
| REQUEST_COUNT, | ||
| REQUEST_DURATION, | ||
| REQUEST_DURATION_MAX, | ||
| REQUEST_DURATION_MIN, | ||
| ] | ||
|
|
||
|
|
||
| def _reset_collectors(): | ||
| """Clear all prometheus metric values between tests.""" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] test-inadequate The _reset_collectors function clears prometheus_client metric state by accessing the internal _metrics attribute (metric._metrics.clear()). This is an undocumented internal API that could break on library upgrades. The dependency is pinned only with a floor (>=0.20). Suggested fix: Create a fresh CollectorRegistry per test, or pin the dependency more tightly (e.g., >=0.20,<1.0). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
| for metric in _ALL_METRICS: | ||
| metric._metrics.clear() | ||
| _extremes.clear() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
|
|
||
|
|
||
| @pytest.fixture | ||
| def client(): | ||
| app.config["TESTING"] = True | ||
| with app.test_client() as client: | ||
| reset_items() | ||
| _reset_collectors() | ||
| yield client | ||
|
|
||
|
|
||
|
|
@@ -63,3 +87,75 @@ def test_delete_item(client): | |
| def test_delete_item_not_found(client): | ||
| resp = client.delete("/items/999") | ||
| assert resp.status_code == 404 | ||
|
|
||
|
|
||
| def test_metrics_endpoint_returns_200(client): | ||
| resp = client.get("/metrics") | ||
| assert resp.status_code == 200 | ||
| assert resp.content_type.startswith("text/plain") | ||
|
|
||
|
|
||
| def test_metrics_contains_help_and_type(client): | ||
| resp = client.get("/metrics") | ||
| body = resp.data.decode() | ||
| assert "# HELP" in body | ||
| assert "# TYPE" in body | ||
|
|
||
|
|
||
| def test_metrics_tracks_request_count(client): | ||
| client.get("/items") | ||
| resp = client.get("/metrics") | ||
| body = resp.data.decode() | ||
| assert "http_requests_total{" in body | ||
| assert 'endpoint="/items"' in body | ||
| assert 'method="GET"' in body | ||
| assert 'status="200"' in body | ||
|
|
||
|
|
||
| def test_metrics_tracks_request_duration(client): | ||
| client.get("/items") | ||
| resp = client.get("/metrics") | ||
| body = resp.data.decode() | ||
| assert "http_request_duration_seconds" in body | ||
| assert 'endpoint="/items"' in body | ||
|
|
||
|
|
||
| def test_metrics_tracks_status_codes(client): | ||
| client.post("/items", json={}) | ||
| resp = client.get("/metrics") | ||
| body = resp.data.decode() | ||
| assert 'status="400"' in body | ||
|
|
||
|
|
||
| def test_metrics_excludes_metrics_endpoint(client): | ||
| # Hit /metrics a few times | ||
| client.get("/metrics") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] test-inadequate 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. |
||
| client.get("/metrics") | ||
| resp = client.get("/metrics") | ||
| body = resp.data.decode() | ||
| assert 'endpoint="/metrics"' not in body | ||
|
|
||
|
|
||
| def test_metrics_tracks_min_max_duration(client): | ||
| client.get("/items") | ||
| resp = client.get("/metrics") | ||
| body = resp.data.decode() | ||
| assert "http_request_duration_max_seconds" in body | ||
| assert "http_request_duration_min_seconds" in body | ||
|
|
||
| # Verify values are positive floats and min <= max | ||
| max_match = re.search( | ||
| r'http_request_duration_max_seconds\{.*endpoint="/items".*\}\s+([\d.e+-]+)', | ||
| body, | ||
| ) | ||
| min_match = re.search( | ||
| r'http_request_duration_min_seconds\{.*endpoint="/items".*\}\s+([\d.e+-]+)', | ||
| body, | ||
| ) | ||
| assert max_match is not None, "max duration metric not found for /items" | ||
| assert min_match is not None, "min duration metric not found for /items" | ||
| max_val = float(max_match.group(1)) | ||
| min_val = float(min_match.group(1)) | ||
| assert max_val > 0, "max duration should be positive" | ||
| assert min_val > 0, "min duration should be positive" | ||
| assert min_val <= max_val, "min duration should be <= max duration" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.