chore: reduce the interval to 30s - #93
Conversation
InftyAI-Agent
added
needs-triage
f82fa0a to
e8644d1
Compare
Signed-off-by: kerthcet <kerthcet@gmail.com>
e8644d1 to
76b1629
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It mixes an unrelated large log-shipping design doc into an interval-change PR, and the updated test hard-codes tick count instead of deriving it from accrualInterval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Reduces the cost accrual checkpoint cadence from 60s to 30s to tighten the baseline→first-charge visibility window for Prometheus increase() queries, and updates documentation/tests accordingly.
Changes:
- Set
accrualIntervalto 30s and makeaccrualTimeouttrack the interval. - Update cost-accrual tests and metrics documentation for the new 30s cadence and revised rounding-error bounds.
- Add a new design doc (
docs/log-shipping.md) describing a planned log shipping architecture.
File summaries
| File | Description |
|---|---|
| internal/controller/cost_accrual.go | Changes accrual write cadence to 30s and updates interval-related rationale/comments. |
| internal/controller/cost_accrual_test.go | Adjusts tests for the new 30s cadence and updates expected rounding-error tolerance. |
| docs/metrics.md | Updates operator-facing documentation to reflect 30s accrual windows and related scrape guidance. |
| docs/log-shipping.md | Introduces an (unimplemented) design document for future CloudWatch log shipping. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes a production write cadence and associated timeout behavior, which can have non-trivial operational impact under real apiserver latency and fleet sizes.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
|
/lgtm |
InftyAI-Agent
added
lgtm
InftyAI-Agent
left a comment
InftyAI-Agent
left a comment
There was a problem hiding this comment.
Approved: PR has both lgtm and approved labels
What this PR does / why we need it
Which issue(s) this PR fixes
Fixes #
Special notes for your reviewer
Does this PR introduce a user-facing change?