api: Fix missing endpoint in OpenAPI #7214
Conversation
📝 WalkthroughWalkthroughThe changes refactor the REST API by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant Middleware
participant RootModule
Client->>API_Router: GET /metrics
API_Router->>Middleware: Authenticate, Cap Websockets, Record
Middleware->>RootModule: metrics()
RootModule->>RootModule: collect_hopr_metrics()
RootModule-->>Middleware: Metrics String or Error
Middleware-->>API_Router: Response
API_Router-->>Client: 200 OK (metrics) or 422 (error)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
Refactor and relocate the Prometheus metrics endpoint while reorganizing middleware to improve modularity and update API route definitions.
- Metrics endpoint moved from the node module to a new root module.
- Middleware reorganized into preconditions and prometheus submodules.
- API routes updated to reflect the new module structure.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hoprd/rest-api/src/root.rs | Added new metrics endpoint with Prometheus support in the root module. |
| hoprd/rest-api/src/node.rs | Removed the metrics function and its associated logic. |
| hoprd/rest-api/src/middleware/mod.rs | Created preconditions and prometheus submodules for middleware. |
| hoprd/rest-api/src/lib.rs | Updated API routes and middleware wiring to use the new modules. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
hoprd/rest-api/src/root.rs (1)
32-37: Review HTTP status code usage for error handling.The current implementation returns HTTP 422 (Unprocessable Entity) for metrics collection failures. Consider if HTTP 503 (Service Unavailable) might be more appropriate when metrics collection fails due to system issues rather than client input problems.
- Err(error) => (StatusCode::UNPROCESSABLE_ENTITY, error).into_response(), + Err(error) => (StatusCode::SERVICE_UNAVAILABLE, error).into_response(),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
hoprd/rest-api/src/lib.rs(6 hunks)hoprd/rest-api/src/middleware/mod.rs(1 hunks)hoprd/rest-api/src/node.rs(0 hunks)hoprd/rest-api/src/root.rs(1 hunks)
💤 Files with no reviewable changes (1)
- hoprd/rest-api/src/node.rs
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Docs / Rust docs
- GitHub Check: Build Candidate Binaries (hopli)
- GitHub Check: tests-unit-nightly
- GitHub Check: tests-unit
- GitHub Check: Cargo Audit
- GitHub Check: Linter
- GitHub Check: zizmor
🔇 Additional comments (9)
hoprd/rest-api/src/middleware/mod.rs (1)
1-2: LGTM! Clean module organization.The module declarations follow Rust conventions with appropriate
pub(crate)visibility for internal middleware components.hoprd/rest-api/src/root.rs (3)
5-9: LGTM! Proper feature-gated implementation.The conditional compilation correctly handles the prometheus feature case with appropriate error mapping.
11-14: LGTM! Consistent fallback implementation.The fallback implementation properly handles the case when prometheus feature is disabled or during tests.
16-31: OpenAPI documentation looks comprehensive.The endpoint documentation includes proper response codes, security requirements, and descriptions. The tag assignment to "Node" category maintains consistency with other node-related endpoints.
hoprd/rest-api/src/lib.rs (5)
7-7: LGTM! Clean module organization.The addition of
middlewareandrootmodules aligns well with the refactoring objectives to improve code organization.Also applies to: 11-11
107-107: Consistent OpenAPI documentation update.The path update from
node::metricstoroot::metricscorrectly reflects the code reorganization.
253-253: Route endpoint correctly updated.The metrics route now properly references the new
root::metricshandler.
254-261: Improved middleware application pattern.The refactoring to use
axum::middleware::from_fn_with_stateprovides better type safety and clearer middleware composition compared to direct function usage. This follows Axum best practices for middleware application.Also applies to: 317-324
272-272: Consistent middleware pattern across routes.The
axum::middleware::from_fnusage for the prometheus recording middleware maintains consistency with the other middleware applications and follows Axum conventions.Also applies to: 335-335
This pull request refactors the
hoprd/rest-apimodule to reorganize middleware and metrics-related functionality. The changes primarily involve moving Prometheus metrics handling from thenodemodule to a newrootmodule, restructuring middleware imports, and updating related API routes.Middleware Refactoring and Organization:
hoprd/rest-api/src/lib.rs: Removed direct references tomiddlewareand reorganized imports to useaxum::middlewareandmiddlewaresubmodules (preconditionsandprometheus). [1] [2] [3] [4] [5]hoprd/rest-api/src/middleware/mod.rs: Addedpreconditionsandprometheusas submodules undermiddlewarefor better modularity.Metrics Handling Relocation:
hoprd/rest-api/src/node.rs: Removed themetricsfunction and associated logic for Prometheus metrics from thenodemodule.hoprd/rest-api/src/root.rs: Added a newmetricsfunction to therootmodule, replicating the Prometheus metrics functionality previously in thenodemodule. This change includes updated API route definitions and error handling.API Route Updates:
hoprd/rest-api/src/lib.rs: Updated API routes to reflect the relocation of themetricsendpoint fromnode::metricstoroot::metrics. [1] [2]Notes
Closes #7143