chore(proto): sync from resq-proto@3f31a3a#77
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Protobuf definition file protos/health.proto to define the ResQDiagnosticService and its associated messages and enums for detailed health reporting. The review feedback suggests prefixing enum values with their enum names to prevent collisions, changing the latency_ms field type from double to uint32 to avoid precision issues, and adding a disk_bytes_total field to ResourceMetrics for completeness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| enum SystemState { | ||
| // Guards against proto3's zero-default masking failures: an unset | ||
| // overall_state / state must not read as HEALTHY. | ||
| SYSTEM_STATE_UNSPECIFIED = 0; | ||
| HEALTHY = 1; | ||
| DEGRADED = 2; | ||
| UNHEALTHY = 3; | ||
| } |
There was a problem hiding this comment.
Enum values should be prefixed with the enum name to prevent name collisions in languages like C++ or Go, adhering to the Google Protobuf Style Guide.
| enum SystemState { | |
| // Guards against proto3's zero-default masking failures: an unset | |
| // overall_state / state must not read as HEALTHY. | |
| SYSTEM_STATE_UNSPECIFIED = 0; | |
| HEALTHY = 1; | |
| DEGRADED = 2; | |
| UNHEALTHY = 3; | |
| } | |
| enum SystemState { | |
| // Guards against proto3's zero-default masking failures: an unset | |
| // overall_state / state must not read as HEALTHY. | |
| SYSTEM_STATE_UNSPECIFIED = 0; | |
| SYSTEM_STATE_HEALTHY = 1; | |
| SYSTEM_STATE_DEGRADED = 2; | |
| SYSTEM_STATE_UNHEALTHY = 3; | |
| } |
References
- Enum value names should be in UPPER_CASE_WITH_UNDERSCORES and should be prefixed with the enum name to avoid collisions in languages that do not scope enums. (link)
| enum DependencyType { | ||
| DEPENDENCY_TYPE_UNSPECIFIED = 0; | ||
| DATABASE = 1; | ||
| CACHE = 2; | ||
| MESSAGE_BROKER = 3; | ||
| PEER_SERVICE = 4; | ||
| INTERNAL_ENGINE = 5; | ||
| } |
There was a problem hiding this comment.
Enum values should be prefixed with the enum name to prevent name collisions in languages like C++ or Go, adhering to the Google Protobuf Style Guide.
| enum DependencyType { | |
| DEPENDENCY_TYPE_UNSPECIFIED = 0; | |
| DATABASE = 1; | |
| CACHE = 2; | |
| MESSAGE_BROKER = 3; | |
| PEER_SERVICE = 4; | |
| INTERNAL_ENGINE = 5; | |
| } | |
| enum DependencyType { | |
| DEPENDENCY_TYPE_UNSPECIFIED = 0; | |
| DEPENDENCY_TYPE_DATABASE = 1; | |
| DEPENDENCY_TYPE_CACHE = 2; | |
| DEPENDENCY_TYPE_MESSAGE_BROKER = 3; | |
| DEPENDENCY_TYPE_PEER_SERVICE = 4; | |
| DEPENDENCY_TYPE_INTERNAL_ENGINE = 5; | |
| } |
References
- Enum value names should be in UPPER_CASE_WITH_UNDERSCORES and should be prefixed with the enum name to avoid collisions in languages that do not scope enums. (link)
| string name = 1; | ||
| DependencyType type = 2; | ||
| SystemState state = 3; | ||
| double latency_ms = 4; |
There was a problem hiding this comment.
Using double for latency can introduce floating-point precision issues. It is recommended to use an integer type like uint32 (representing milliseconds) or google.protobuf.Duration for representing durations and latencies in Protobuf.
| double latency_ms = 4; | |
| uint32 latency_ms = 4; |
| message ResourceMetrics { | ||
| double cpu_usage_percent = 1; | ||
| uint64 memory_bytes_used = 2; | ||
| uint64 memory_bytes_total = 3; | ||
| uint64 disk_bytes_free = 4; | ||
| } |
There was a problem hiding this comment.
Adding disk_bytes_total allows calculating the percentage of disk space remaining or used, similar to how memory metrics provide both used and total bytes.
| message ResourceMetrics { | |
| double cpu_usage_percent = 1; | |
| uint64 memory_bytes_used = 2; | |
| uint64 memory_bytes_total = 3; | |
| uint64 disk_bytes_free = 4; | |
| } | |
| message ResourceMetrics { | |
| double cpu_usage_percent = 1; | |
| uint64 memory_bytes_used = 2; | |
| uint64 memory_bytes_total = 3; | |
| uint64 disk_bytes_free = 4; | |
| uint64 disk_bytes_total = 5; | |
| } |
Audit Results for PR #77I have audited the changes in this PR, which primarily involve the addition of and an update to . Findings for1. Security: Information DisclosureThe message and the field in expose internal system details (CPU, memory, disk usage, and potentially internal error messages).
2. Logic: Completeness of Resource MetricsThe message currently includes but lacks a corresponding field.
3. Documentation: Metric UnitsThe unit for is not explicitly defined.
4. Standards: gRPC Health CheckingThe PR introduces a custom for health checks.
Other Observations
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Audit Results for PR #77I have audited the changes in this PR, which primarily involve the addition of Findings for
|
1 participant
Automated sync from resq-proto@3f31a3a4c4999b3ea5d0f3555c25d72ac6eabc50\n\nBSR revision: buf.build/resq-software/resq-proto:9b0f82297aca4f669603fd516e9e6263