Repository navigation
Conversation
JuaniRios
changed the title
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Limit details: You’ve used all 4 included reviews currently available. Your 59 included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. WalkthroughThe change adds file-log event counting and two 24-hour reliability metrics, labeled by level and by level and target. The counter records events accepted by the configured file-log filter, seeds prior entries asynchronously, and maintains one-minute buckets. Reliability metrics are omitted until seeding completes. The change also updates tracing setup, log-file scanning, metric documentation, and parity comparison rules. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The new reliability metrics count file-log errors and warnings, becoming available after seeding. Persistent read failures may leave partial historical counts, as documented and logged; no actionable merge-blocking risk is identified. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 67.65% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 68 functions across 11 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly summarizes the main change: native counting of error and warning logs for liq reliability metrics. The issue reference and minor spacing issue do not prevent understanding. |
| Description check | |
The description directly explains the log-counting implementation, metrics, seeding behavior, known differences, testing, and remaining verification work. |
Full details: Docstring Coverage
Explanation
Docstring coverage is 67.65% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 68 functions across 11 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
a8677c5 to
123d4a8
Compare
eaf0749 to
410233c
Compare
123d4a8 to
0eb74db
Compare
|
@rain-marvin review |
|
🔎 Reviewing |
![rain-marvin[bot]](https://avatars.githubusercontent.com/in/5077017?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR makes the bot count its own error and warning log lines as it writes them. A tracing layer sits behind the file log's filter and feeds one-minute buckets per level and target. The bot then publishes liq_reliability_log_count_24h, liq_log_target_count_24h and log_events_total itself, so the exporter no longer rescans up to 50,000 log entries every minute. After each start, a one-time background seed adds the previous process's last 24 h from the files, and it takes only entries strictly before activation. The /logs scanner is split into visitor helpers that the seed shares, which also fixes an endless loop on a persistent read error.
Overall this is sound. The activation boundary prevents double counting. The seed state machine is bounded (two retries, then it keeps a partial read) and runs off the collectors' path. Nothing logs while a lock is held. The /logs and /performance/reliability behaviour is preserved apart from the documented fix. A concern that the bucket mutex could stall logging threads during window() or merge() was checked independently and dismissed: the maps are bounded at about 1,440 minutes per target, and the lock is held for microseconds to low milliseconds. I found no upgrade or shared-stall risk. What remains is two minor points (log counts freeze while the SQLite loaders fail, and no positive test that the production setup attaches the counting layer) and two documentation nits.
claude-opus-5-5 · high · 1 h 2 min
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
410233c to
2d275dc
Compare
0eb74db to
045776e
Compare
JuaniRios
changed the title
This branch has not been deployed
1 participant
Part of RAI-3091.
Counts error and warning logs as the bot writes them, for
liq_reliability_log_count_24handliq_log_target_count_24h. Item 3 of the RAI-2998 stack, part 3 of 3.Live effect: the reliability family gains the two log-count names, and
/metricsgainslog_events_total{level,target}. After each start a background task reads the last 24 h of log files once · Risk: low to medium (a new layer runs on every file log event, and the/logsscanner was refactored) · Ships: on the next bot releaseWhy
The exporter got these counts by rescanning up to 50,000 log entries through
/performance/reliabilityevery minute. Counting as events happen is cheaper and has no cap.Decisions
st0x_configgets aLogEventSinktrait. The counting layer is attached only on the file-logging branch, with the file layer's own filter. So it counts what the file log writes, and no dependency TRACE callsite is enabled for it.compare.pywith evidence: no 50k cap (decision J11), the first minute of the window counts whole, lines an operator'sst0x-clirun appends to the same files are counted only after the next restart, and a line the lossy non-blocking writer drops is still counted.liq_reliability_log_count_24hcompares keys only, andliq_log_target_count_24his ignored. A target can exist on one side only./logsfile scanner is split into visitor helpers that the seed shares./logsand/performance/reliabilitybehave the same, except one bug fix. A persistent read error, for example a directory namedst0x-hedge.log.*, used to loop forever on that file. Now it stops reading that file.Proof
golden.py --check,test_compare.py, andgen-t0-liquidity.py --checkpass. The reliability golden now includes the log names.log_events_total.st0x-hedgeandst0x-configsuites on this head: 5283 passed. One failure wasapi::tests::a_fee_bumped_copy_of_the_burn_is_adopted_as_its_replacement(anvil "max fee per gas less than block base fee"). It's unrelated and passed 3 of 3 times on rerun./performance/reliability, and the seed time on production-size log files. Those need a release.Merge conditions
Same as the stack below.