Wave-33 C05: OTLP metrics export soft stub - #276
Conversation
Add docs/feature stub and SelfCheck for unpaid OTLP metrics push while keeping default Prometheus GET /metrics unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
Next review available in: 12 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| let script = repo_root().join("scripts/otlp-metrics-check.ps1"); | ||
| assert!(script.is_file(), "expected OTLP metrics check script at {}", script.display()); | ||
|
|
||
| let output = Command::new("pwsh") |
There was a problem hiding this comment.
WARNING: Integration test hardcodes pwsh dependency
The Command::new("pwsh") call assumes PowerShell Core is installed on the developer machine. While GitHub ubuntu-latest runners have pwsh, many Linux/macOS environments do not. This makes cargo test --test otlp_metrics fail outside CI, breaking the hermetic SelfCheck promise for local development.
Consider gating this test behind a Windows-only cfg or providing a fallback (e.g., skip with a helpful message if pwsh is unavailable, or use bash for a portable subset of checks).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| #[test] | ||
| fn stub_acknowledged_parses_truthy_values() { |
There was a problem hiding this comment.
WARNING: Unit test leaks env state on panic
The stub_acknowledged_parses_truthy_values test uses std::env::set_var / remove_var without panic-safe cleanup. If any assert! between lines 51 and 60 panics, SL_OTLP_METRICS remains set in the process environment for subsequent tests, causing flaky test failures in the same binary.
Wrap the env mutations in a guard (e.g., a small RAII helper that restores the previous value on drop) to guarantee cleanup even on panic.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| set. If neither variable is set, the feature-enabled binary uses the same local | ||
| fmt logs and `RUST_LOG` filtering as the default build. | ||
|
|
||
| OTLP **metrics** push is a soft stub (`--features otel-metrics`, optional |
There was a problem hiding this comment.
SUGGESTION: README docs omit accepted env values
The README documents SL_OTLP_METRICS=1 as the acknowledgment value, but otel_metrics.rs:15-17 also accepts "true" and "yes" (case-insensitive). Operators reading the README won't discover these alternatives, creating a doc/code mismatch.
Update the README to mention all accepted truthy values, or narrow the parser to only "1" to match the documented contract.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Review post-merge (PR is already MERGED) Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 68.5K · Output: 8.6K · Cached: 211.3K |
1 participant
Summary
GET /metricsRED path unchangedTest plan
pwsh ./scripts/otlp-metrics-check.ps1 -SelfCheckcargo check --manifest-path crates/sl-daemon/Cargo.toml --locked --features otel-metricscargo test --locked --test otlp_metricsGET /metricsstill renderssl_http_requests_totalwithout the featureMade with Cursor