feat: rewrite telemetery and configs - #18
Merged
Merged
Conversation
📝 WalkthroughWalkthroughTelemetry configuration replaces host-signal capture with ChangesTelemetry mode migration
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
internal/report/report.go (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated rule-hit rendering loop across
report.goandrender.go.Both files copy-pasted the identical rule-hit formatting loop when adding the telemetry-disabled branch. Extracting a small
appendRuleHitLineshelper in each file eliminates the duplication.
internal/report/report.go#L354-372: extract a localappendRuleHitLines(&lines, runReport.RuleHits)helper and call it in both the disabled-telemetry branch (lines 356-362) and the default branch (lines 366-372).internal/render/render.go#L109-117: extract the same local helper and call it in both the disabled-telemetry branch (lines 111-117) and the default branch (lines 121-127).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/report/report.go` at line 1, Extract the duplicated rule-hit formatting loops into an appendRuleHitLines helper in both report.go and render.go. Update the telemetry-disabled and default branches in the report and render flows to call the respective helper with the output lines and RuleHits, preserving the existing formatting and branch behavior.
354-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated rule-hit rendering into a helper.
The rule-hit formatting loop at lines 356-362 is identical to lines 366-372. The same duplication appears in
internal/render/render.go(lines 111-117 vs 121-127). Extracting a small helper eliminates the copy-paste in both files.♻️ Proposed helper extraction
func BuildSummary(runReport Report) string { var lines []string // ... existing code ... lines = append(lines, "", "## Why This Verdict", "") if runReport.Diagnostic != nil { lines = append(lines, "- "+runReport.Diagnostic.Summary) } else if runReport.Telemetry.Status == TelemetryStatusDisabled { lines = append(lines, "- Kernel-level telemetry was disabled, so this comparison requires human review.") - for _, hit := range runReport.RuleHits { - line := fmt.Sprintf("- %s matched `%s`", hit.Category, hit.RuleID) - if strings.TrimSpace(hit.Reason) != "" { - line += ": " + hit.Reason - } - lines = append(lines, line) - } + appendRuleHitLines(&lines, runReport.RuleHits) } else if len(runReport.RuleHits) == 0 { lines = append(lines, "- No rules matched. The recommendation comes from the raw diff outcome.") } else { - for _, hit := range runReport.RuleHits { - line := fmt.Sprintf("- %s matched `%s`", hit.Category, hit.RuleID) - if strings.TrimSpace(hit.Reason) != "" { - line += ": " + hit.Reason - } - lines = append(lines, line) - } + appendRuleHitLines(&lines, runReport.RuleHits) } // ... } + +func appendRuleHitLines(lines *[]string, hits []RuleHit) { + for _, hit := range hits { + line := fmt.Sprintf("- %s matched `%s`", hit.Category, hit.RuleID) + if strings.TrimSpace(hit.Reason) != "" { + line += ": " + hit.Reason + } + *lines = append(*lines, line) + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/report/report.go` around lines 354 - 372, The rule-hit rendering loop is duplicated across the telemetry-disabled and normal branches in the report generation flow, and similarly in render generation. Extract the shared hit-formatting logic into a small helper in each relevant package, then call it from both branches while preserving the existing category, rule ID, and optional reason formatting.internal/render/render.go (1)
109-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRule-hit rendering loop duplicated from lines 121-127.
This is the same copy-paste pattern flagged in
internal/report/report.go. Extracting a shared helper (or a localappendRuleHitLines) would eliminate the duplication in both files.♻️ Proposed local helper for render.go
} else if runReport.Telemetry.Status == report.TelemetryStatusDisabled { lines = append(lines, "- Kernel-level telemetry was disabled, so this comparison requires human review.") - for _, hit := range runReport.RuleHits { - line := fmt.Sprintf("- %s matched `%s`", hit.Category, hit.RuleID) - if strings.TrimSpace(hit.Reason) != "" { - line += ": " + hit.Reason - } - lines = append(lines, line) - } + appendRuleHitLines(&lines, runReport.RuleHits) } else if len(runReport.RuleHits) == 0 { lines = append(lines, "- No rules matched. This output mirrors the report-level verdict and recommendation.") } else { - for _, hit := range runReport.RuleHits { - line := fmt.Sprintf("- %s matched `%s`", hit.Category, hit.RuleID) - if strings.TrimSpace(hit.Reason) != "" { - line += ": " + hit.Reason - } - lines = append(lines, line) - } + appendRuleHitLines(&lines, runReport.RuleHits) } +func appendRuleHitLines(lines *[]string, hits []report.RuleHit) { + for _, hit := range hits { + line := fmt.Sprintf("- %s matched `%s`", hit.Category, hit.RuleID) + if strings.TrimSpace(hit.Reason) != "" { + line += ": " + hit.Reason + } + *lines = append(*lines, line) + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/render/render.go` around lines 109 - 117, Extract the duplicated RuleHits formatting loop from the telemetry-disabled branch and the corresponding nearby branch into a shared helper or local appendRuleHitLines function, then call it from both locations. Preserve the existing output format, including the optional trimmed Reason suffix, and apply the same deduplication pattern in internal/report/report.go.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/diagrams/limier-operation-flow.scene-spec.json`:
- Line 317: Make the telemetry requirement conditional in both diagram
artifacts: update the callout label in
docs/diagrams/limier-operation-flow.scene-spec.json at lines 317-317 to say
process-execution telemetry is included when enabled, and synchronize the
rendered text and originalText in docs/diagrams/limier-operation-flow.excalidraw
at lines 3083-3089 with the same wording.
In `@docs/diagrams/limier-operation-flow.verification.json`:
- Line 68: Restore the minimumEditableElements threshold to its previous value
of 18 for this diagram. Do not lower the verification gate to 2 unless the
intended requirement is documented and covered by tests.
In `@docs/guide/getting-started.md`:
- Around line 12-17: Update the prerequisites and kernel telemetry warning in
the getting-started guide to require Linux with bpftrace, cgroup v2, and eBPF
support. Extend the fallback condition to recommend telemetry.mode: off when any
of these prerequisites are unavailable, while preserving the existing
output-only and good_to_go behavior.
In `@docs/launch-readiness.md`:
- Line 41: Update the reviewer journey near the hosted/self-hosted telemetry
guidance to remove stdout/stderr-only and bpftrace comparisons. Describe the
behavior using the documented telemetry.mode values: required must enforce
kernel telemetry availability with fail-closed behavior, while off must
represent output-only review without kernel telemetry; keep the existing
launch-readiness context intact.
---
Nitpick comments:
In `@internal/render/render.go`:
- Around line 109-117: Extract the duplicated RuleHits formatting loop from the
telemetry-disabled branch and the corresponding nearby branch into a shared
helper or local appendRuleHitLines function, then call it from both locations.
Preserve the existing output format, including the optional trimmed Reason
suffix, and apply the same deduplication pattern in internal/report/report.go.
In `@internal/report/report.go`:
- Line 1: Extract the duplicated rule-hit formatting loops into an
appendRuleHitLines helper in both report.go and render.go. Update the
telemetry-disabled and default branches in the report and render flows to call
the respective helper with the output lines and RuleHits, preserving the
existing formatting and branch behavior.
- Around line 354-372: The rule-hit rendering loop is duplicated across the
telemetry-disabled and normal branches in the report generation flow, and
similarly in render generation. Extract the shared hit-formatting logic into a
small helper in each relevant package, then call it from both branches while
preserving the existing category, rule ID, and optional reason formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 578dab85-aaa4-4c4c-ba2c-3333613172f2
⛔ Files ignored due to path filters (2)
docs/diagrams/limier-operation-flow.preview.pngis excluded by!**/*.pngdocs/diagrams/limier-operation-flow.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.agents/skills/limier-cli/SKILL.mdREADME.mdcmd/ci.gocmd/run.godocs/diagrams/limier-operation-flow.excalidrawdocs/diagrams/limier-operation-flow.scene-spec.jsondocs/diagrams/limier-operation-flow.verification.jsondocs/guide/ci-and-deploy.mddocs/guide/getting-started.mddocs/guide/review-your-own-project.mddocs/guide/understand-results.mddocs/launch-readiness.mddocs/reference/cli.mddocs/reference/scenario-file.mdinternal/analysis/analysis.gointernal/collector/collector.gointernal/collector/factory_linux.gointernal/collector/factory_nonlinux.gointernal/limier/run.gointernal/limier/run_test.gointernal/preset/assets/scenarios/cargo-ci.ymlinternal/preset/assets/scenarios/npm-ci.ymlinternal/preset/assets/scenarios/pip-ci.ymlinternal/render/render.gointernal/render/render_test.gointernal/render/testdata/build-summary.golden.mdinternal/render/testdata/github-comment.golden.mdinternal/render/testdata/gitlab-note.golden.mdinternal/render/testdata/inspect-conclusive.golden.mdinternal/render/testdata/inspect-inconclusive.golden.mdinternal/report/report.gointernal/report/report_test.gointernal/scenario/scenario.gointernal/scenario/scenario_test.goscenarios/npm.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/collector/factory_linux.go (1)
167-220: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFixed 250ms drain grace is a blind heuristic, not a deterministic completion signal.
waitForBpftraceDrainalways sleeps a flatbpftraceDrainGrace(250ms) before signalingstop(), regardless of whether bpftrace has actually finished flushing. This is a tradeoff: too short risks dropping trailing events for slow/loaded hosts (silently degrading telemetry evidence used for "required" mode gating); applied on every step, it also adds up to real latency for CI pipelines with many steps.Consider whether a deterministic signal (e.g., an explicit "step done" marker printed by the script once the traced cgroup's steady-state is reached) is feasible instead of/in addition to the fixed sleep, or document why 250ms was chosen as sufficient across environments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/collector/factory_linux.go` around lines 167 - 220, Replace the fixed-delay waitForBpftraceDrain heuristic used by bpftraceStepCapture.Finish with a deterministic completion signal from the bpftrace script, such as an explicit step-done marker after the traced cgroup reaches steady state, and wait for that signal before calling stop. Ensure the signal is handled with context cancellation and preserve the existing error propagation and event-draining behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/collector/factory_linux.go`:
- Around line 167-220: Replace the fixed-delay waitForBpftraceDrain heuristic
used by bpftraceStepCapture.Finish with a deterministic completion signal from
the bpftrace script, such as an explicit step-done marker after the traced
cgroup reaches steady state, and wait for that signal before calling stop.
Ensure the signal is handled with context cancellation and preserve the existing
error propagation and event-draining behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cefe70b5-922a-4216-82dc-95685628e7c7
📒 Files selected for processing (1)
internal/collector/factory_linux.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
telemetry.modeto scenarios (requireddefault) andtelemetry: { mode: off }for output-only comparisons.--telemetry-mode {required|off}forlimier runandlimier ci github.Bug Fixes
rerunguidance; output-only mode always requires human review.Documentation
telemetry.modesemantics.Tests