Remove OpenBSD from official support (code is kept in case) #64
Conversation
WalkthroughThe pull request modifies the GitHub Actions workflow file Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
=======================================
Coverage 55.68% 55.68%
=======================================
Files 5 5
Lines 264 264
=======================================
Hits 147 147
Misses 117 117 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
lambdalisue
changed the title
Because we cannot fix the CI issue on OpenBSD and we cannot support it. In case, we keep the code for OpenBSD but it is not tested and we cannot guarantee its correctness.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
README.md (2)
37-38: Consider adding future support status clarificationThe note about OpenBSD code existence and testing status is clear. Consider adding a brief statement about whether OpenBSD support might be reinstated in the future, given that cross-platform-actions now supports OpenBSD 7.6.
Go ([client_golang]) provides. Note that code for OpenBSD exists but is not -tested and we cannot guarantee its correctness. +tested and we cannot guarantee its correctness. Support may be reinstated in future versions.
42-44: Enhance version compatibility note clarityWhile technically accurate, the note about version compatibility could be more explicit about the impact of the change.
-> Prior to version 2.0.0, the `process_cpu_seconds_total` metric was Gauge -> instead of Counter. Enable `use-gauge-on-cpu-seconds-total` feature to use the -> previous behavior. +> Breaking change in 2.0.0: The `process_cpu_seconds_total` metric changed from Gauge +> to Counter type. To maintain compatibility with pre-2.0.0 behavior, enable the +> `use-gauge-on-cpu-seconds-total` feature.🧰 Tools
🪛 LanguageTool
[style] ~42-~42: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...arantee its correctness. > [!NOTE] > > Prior to version 2.0.0, the `process_cpu_seconds...(EN_WORDINESS_PREMIUM_PRIOR_TO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/test.yml(2 hunks)README.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~42-~42: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...arantee its correctness. > [!NOTE] > > Prior to version 2.0.0, the `process_cpu_seconds...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~58-~58: Using many exclamation marks might seem excessive (in this case: 7 exclamation marks for a text that’s 4961 characters long)
Context: ... | | x | | > [!NOTE] > > If you only need to compile th...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (3)
README.md (3)
11-13: LGTM: Support statement accurately updated
The removal of OpenBSD from the list of officially supported platforms aligns with the PR objectives while maintaining clarity about supported platforms.
60-62: LGTM: Clear documentation for non-supported platforms
The explanation of the dummy feature is clear and helpful for users working with non-supported platforms.
46-56: Verify OpenBSD support matrix accuracy
The support matrix clearly shows OpenBSD in parentheses and marks supported features. However, we should verify if the marked support status matches the actual code capabilities.
✅ Verification successful
Let's first identify the relevant files before searching for specific metrics. The previous command might have failed because we piped an empty result to xargs. Let's break this down into steps to better understand the codebase structure.
Now let's look at the actual implementation to verify the supported metrics in OpenBSD.
Support matrix accurately reflects OpenBSD implementation
The support matrix in the README.md is accurate. The OpenBSD implementation in src/collector/openbsd.rs and the test case test_collect_internal_ok_openbsd confirm that OpenBSD supports exactly the metrics marked in the table:
- ✓ process_cpu_seconds_total
- ✓ process_max_fds
- ✓ process_resident_memory_bytes
- ✓ process_start_time_seconds
The implementation explicitly documents the unsupported metrics in its TODO comment, which aligns with the blank entries in the table for:
- process_open_fds
- process_virtual_memory_bytes
- process_virtual_memory_max_bytes
- process_threads
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for OpenBSD-specific implementations of the marked metrics
rg -l "openbsd" --type rust | xargs rg "process_(cpu_seconds_total|max_fds|resident_memory_bytes|start_time_seconds)"
Length of output: 117
Script:
#!/bin/bash
# First, let's find all Rust files that might contain OpenBSD-related code
rg -l -i "openbsd|bsd" --type rust
# Then let's look for process metric implementations in those files
rg -l "process_" --type rust
Length of output: 191
Script:
#!/bin/bash
# Let's examine the OpenBSD collector implementation specifically
cat src/collector/openbsd.rs
# And check the collector trait definition to understand expected metrics
cat src/collector.rs
Length of output: 7854
Close #65
Summary by CodeRabbit
Chores
Documentation
process_cpu_seconds_totalmetric status.process_cpu_seconds_totalmetric and its previous behavior.