Conversation
Signed-off-by: georgi-l95 <glazarov95@gmail.com>
WalkthroughUpdates release workflow to extract binaries from Docker images instead of local builds, repackaging them and uploading checksums. In the observability plugin, adds a readiness endpoint, switches handler state to AppContext, updates metrics handler signature, and wires Router state accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant HTTP_Server as "HTTP Server"
participant Router
participant Plugin as "ObservabilityPlugin"
participant AppContext
Note over HTTP_Server,Router: Startup — Router state = AppContext
Client->>HTTP_Server: GET /metrics
HTTP_Server->>Router: Route request
Router->>Plugin: get_metrics(State<AppContext>)
Plugin->>AppContext: read metrics registry
Plugin-->>Client: 200 OK (metrics)
Client->>HTTP_Server: GET /readyz
HTTP_Server->>Router: Route request
Router->>Plugin: readiness_check(State<AppContext>)
Plugin->>AppContext: validate ProvidesBlockReader
alt Ready
Plugin-->>Client: 200 OK
else Not ready
Plugin-->>Client: 503 Service Unavailable
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)crates/rock-node-observability-plugin/src/lib.rs(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/rock-node-observability-plugin/src/lib.rs (4)
crates/rock-node-backfill-plugin/src/worker.rs (1)
create_test_context(759-814)crates/rock-node-subscriber-plugin/src/session.rs (1)
create_test_context(551-628)crates/rock-node-backfill-plugin/src/lib.rs (1)
create_test_context(166-224)tests/integration/src/fixtures/mock_plugins.rs (1)
create_test_context(103-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Basic Quality Checks
- GitHub Check: Security Audit
🔇 Additional comments (5)
crates/rock-node-observability-plugin/src/lib.rs (5)
10-10: LGTM!The import addition is necessary for the updated handler signatures that now consume
AppContext.
359-374: LGTM!The handler correctly extracts metrics from the
AppContextstate. This change aligns well with the new router state configuration and maintains the same error handling.
427-431: LGTM!The router configuration correctly:
- Adds the new
/readyzendpoint for readiness probing- Updates state to pass the full
AppContext(via cheap Arc clones) so both handlers can access their required fieldsThis change maintains consistency across all endpoints.
250-260: LGTM!Test correctly updated to reflect the new handler signature. The test validates that the handler executes successfully with the full
AppContext.
381-398: Verify completeness of readiness checks.The readiness probe currently only verifies
ProvidesBlockReader, but theCapabilityenum also includesProvidesVerifiedBlocks. Confirm whetherProvidesVerifiedBlocksmust be registered before marking the service as ready.
Signed-off-by: georgi-l95 <glazarov95@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
63-71: Avoid fixed container names; use ephemeral IDs to prevent collisions.Using hardcoded names (rock-node-amd64/arm64) can collide on retries. Prefer capturing the container ID.
Apply this pattern:
- docker create --name rock-node-amd64 --platform linux/amd64 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }} - docker cp rock-node-amd64:/app/rock-node release-artifacts/rock-node-amd64 - docker rm rock-node-amd64 + CID_AMD64=$(docker create --platform linux/amd64 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }}) + docker cp "${CID_AMD64}:/app/rock-node" release-artifacts/rock-node-amd64 + docker rm "${CID_AMD64}" - docker create --name rock-node-arm64 --platform linux/arm64 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }} - docker cp rock-node-arm64:/app/rock-node release-artifacts/rock-node-arm64 - docker rm rock-node-arm64 + CID_ARM64=$(docker create --platform linux/arm64 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }}) + docker cp "${CID_ARM64}:/app/rock-node" release-artifacts/rock-node-arm64 + docker rm "${CID_ARM64}"
61-62: Harden the script with strict shell options.Fail fast on errors and unset vars to avoid partial releases.
run: | - mkdir -p release-artifacts + set -euo pipefail + mkdir -p release-artifactsAlso applies to: 73-75, 86-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Security Audit
- GitHub Check: Build and Basic Quality Checks
🔇 Additional comments (1)
.github/workflows/release.yml (1)
73-81: Docs/archive name mismatch resolved.Copying to rock-node before tarring fixes the previous issue where tarballs contained arch‑suffixed filenames. Matches the release notes’ ./rock-node usage now. Good change.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (37.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
- Coverage 79.90% 79.82% -0.08%
==========================================
Files 56 56
Lines 10493 10504 +11
==========================================
+ Hits 8384 8385 +1
- Misses 2109 2119 +10
🚀 New features to boost your workflow:
|
1 participant
Description
This PR improves the release workflow.
Reviewer Checklist
Summary by CodeRabbit
New Features
Chores