feat: add Parseable branding to metrics onboarding scripts - #1741
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe ingest scripts now show colored setup banners before configuration and completion messages after Fluent Bit starts. Setup restarts an existing Fluent Bit process with the updated configuration. PowerShell validates the Fluent Bit process path and reports startup status. ChangesIngest setup UI
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🤖 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 `@scripts/ingest.ps1`:
- Line 398: Update Start-FluentBit to signal whether it actually started or
reloaded Fluent Bit, and gate Write-SetupComplete on that status so early
returns for an already-running process do not report completion for the new
configuration.
- Line 78: Update the success message in the ingest script to replace the
Unicode ✓ character with the ASCII text [OK]. Preserve the existing color
variables and keep the script BOM-less so the pwsh entry point remains usable.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 80328faa-70d7-4b9a-a1cc-e8f2cde749cb
📒 Files selected for processing (2)
scripts/ingest.ps1scripts/ingest.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/ingest.ps1 (1)
300-303: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
ShouldProcesssupport toRestart-FluentBit.PSScriptAnalyzer reports
PSUseShouldProcessForStateChangingFunctionsat Line 300. AddSupportsShouldProcessand guard the stop/start sequence.Proposed refactor
function Restart-FluentBit { + [CmdletBinding(SupportsShouldProcess)] + param() + if (-not $PSCmdlet.ShouldProcess("Fluent Bit", "Restart")) { + return + } Stop-FluentBit Start-Sleep -Seconds 2 [void](Start-FluentBit) }🤖 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 `@scripts/ingest.ps1` around lines 300 - 303, Update Restart-FluentBit to declare SupportsShouldProcess and wrap the Stop-FluentBit, delay, and Start-FluentBit sequence in a ShouldProcess guard, preserving the existing restart behavior when confirmation is approved.Source: Linters/SAST tools
🤖 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 `@scripts/ingest.ps1`:
- Around line 397-400: Update the automatic restart block guarded by
Test-FluentBitRunning to validate that the PID-file process is the expected
Fluent Bit executable or matches its persisted start time before calling
Stop-FluentBit. If validation fails, remove the stale PID file and do not stop
the unrelated process; preserve the existing restart flow for validated Fluent
Bit processes.
---
Nitpick comments:
In `@scripts/ingest.ps1`:
- Around line 300-303: Update Restart-FluentBit to declare SupportsShouldProcess
and wrap the Stop-FluentBit, delay, and Start-FluentBit sequence in a
ShouldProcess guard, preserving the existing restart behavior when confirmation
is approved.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1f0cf594-25ea-437e-9472-9d3ac6547e50
📒 Files selected for processing (2)
scripts/ingest.ps1scripts/ingest.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/ingest.sh
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ingest.ps1 (1)
301-308: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the
$falsestartup result.
Start-FluentBitreturns$falsewhen Fluent Bit is already running.Restart-FluentBitdiscards this result, andSetup-FluentBitreturns normally without a completion or failure message. Add a failure branch that exits non-zero or throws.🤖 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 `@scripts/ingest.ps1` around lines 301 - 308, Update Restart-FluentBit to capture the result of Start-FluentBit and handle a $false return explicitly by throwing or exiting with a non-zero status. Preserve the existing stop and delay sequence, while ensuring Setup-FluentBit receives a clear failure instead of returning normally when Fluent Bit does not start.
🤖 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 `@scripts/ingest.ps1`:
- Around line 89-101: Update the process identity check around Get-Process and
the PID-file cleanup so an inaccessible or null executable path does not delete
the PID for a live Fluent Bit process. Use Win32_Process.ExecutablePath or
enforce a 64-bit host to obtain the path, and remove $PID_FILE only when the
process is confirmed absent; preserve the existing path comparison for valid
identity checks.
---
Outside diff comments:
In `@scripts/ingest.ps1`:
- Around line 301-308: Update Restart-FluentBit to capture the result of
Start-FluentBit and handle a $false return explicitly by throwing or exiting
with a non-zero status. Preserve the existing stop and delay sequence, while
ensuring Setup-FluentBit receives a clear failure instead of returning normally
when Fluent Bit does not start.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
2 participants
PR description:
What changed
Summary by CodeRabbit
User Experience
Bug Fixes