feat(alertRules): eval records drawer to inspect per-cycle evaluation raw data - #2239
feat(alertRules): eval records drawer to inspect per-cycle evaluation raw data#2239710leo wants to merge 2 commits into
Conversation
… raw data Adds an 'Eval records' row action on the alert rules list that opens a drawer showing each evaluation cycle of the rule: query results (final promql + raw series samples), judge outcomes (anomaly/recover points) and the event funnel (fired/pending/muted/pipeline-drop/inhibited), backed by GET /api/n9e/alert-rule/:id/eval-records. Supports time range filter and cursor-based load-more; i18n for zh_CN/zh_HK/en_US/ja_JP/ru_RU.
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a localized evaluation-records drawer to the alert-rules list. The drawer retrieves paginated records, displays query, anomaly, event, and node-error details, and supports time ranges, refresh, datasource filters, and expanded rows. ChangesEvaluation records
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AlertRulesList
participant EvalRecordsDrawer
participant getAlertRuleEvalRecords
AlertRulesList->>EvalRecordsDrawer: open selected alert rule
EvalRecordsDrawer->>getAlertRuleEvalRecords: request records
getAlertRuleEvalRecords-->>EvalRecordsDrawer: return records and node errors
EvalRecordsDrawer-->>AlertRulesList: display evaluation details
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@src/pages/alertRules/List/EvalRecordsDrawer/index.tsx`:
- Around line 247-249: Update the request failure handler in the
EvalRecordsDrawer query flow so that when before is not set, it clears records
along with hasMore, nodeErrs, and queriedRange. Preserve the existing behavior
for failures with before set.
- Around line 228-252: Update fetchData to maintain a monotonically increasing
request ID across calls and capture the ID for each request. In the then, catch,
and finally handlers, apply records, errors, range, and loading-state updates
only when the captured ID is still the latest, so obsolete responses cannot
overwrite current data or loading state.
- Around line 353-365: Update EvalRecordsDrawer’s onClose handling to invalidate
pending requests and reset records, nodeErrs, hasMore, queriedRange, loading,
and the temporary range before invoking onClose. Keep the existing close
behavior and ensure the reset occurs whenever the drawer closes, since
destroyOnClose does not unmount this component.
- Around line 89-107: Update both column render callbacks in the eval records
table to read the row from the second render argument, then access its points
field for the latest value and timestamp. Keep the existing formatting, tooltip,
and fallback behavior unchanged.
- Line 69: Update the table callbacks in the EvalRecordsDrawer to remove any
annotations and use the existing row types: EvalSeriesSample for series
rowKey/render callbacks, EvalAnomalyBrief for anomaly callbacks, and
EvalEventTrail for event callbacks. Preserve the current callback behavior while
ensuring each table’s API response type is enforced through its presentation
props.
- Around line 397-402: Update the curl command rendering in the
EvalRecordsNodeErr mapping to use authenticated HTTPS instead of http:// when
interpolating e.instance, preserving the existing query parameters and
credentials. If arbitrary instances cannot safely support HTTPS, constrain the
generated endpoint to loopback and enforce that deployment assumption rather
than exposing BasicAuth over cleartext.
🪄 Autofix
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 Plus
Run ID: a7feda5f-f7b7-4085-ae50-e805bd61f06d
📒 Files selected for processing (9)
src/pages/alertRules/List/EvalRecordsDrawer/index.tsxsrc/pages/alertRules/List/EvalRecordsDrawer/style.lesssrc/pages/alertRules/List/ListNG.tsxsrc/pages/alertRules/locale/en_US.tssrc/pages/alertRules/locale/ja_JP.tssrc/pages/alertRules/locale/ru_RU.tssrc/pages/alertRules/locale/zh_CN.tssrc/pages/alertRules/locale/zh_HK.tssrc/pages/alertRules/services.ts
| <Table | ||
| className='mt-1' | ||
| size='small' | ||
| rowKey={(r: any) => JSON.stringify(r.labels)} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/pages/alertRules/List/EvalRecordsDrawer/index.tsx"
if [ -f "$file" ]; then
echo "== file exists =="
wc -l "$file"
echo "== outline =="
ast-grep outline "$file" || true
echo "== relevant lines =="
cat -n "$file" | sed -n '1,230p'
else
echo "missing $file"
fd -a 'index.tsx$' . | sed -n '1,80p'
fi
echo "== any occurrences in file =="
rg -n '\bany\b|Eval(Series|Anomaly|Event|Records|Sample|Trail|Brief)' "$file" || trueRepository: n9e/fe
Length of output: 12306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== services files =="
fd -a 'services\.ts$|types\.ts$' src/pages/alertRules | sort
echo "== alertRules services.ts outline/lines =="
services="src/pages/alertRules/services.ts"
if [ -f "$services" ]; then
wc -l "$services"
ast-grep outline "$services" || true
sed -n '1,260p' "$services" | cat -n
fi
echo "== eval-related type definitions across repo =="
rg -n "interface Eval(QueryRecord|AnomalyBrief|EventTrail|SeriesSample)|type Eval(QueryRecord|AnomalyBrief|EventTrail|SeriesSample)|export .*Eval(QueryRecord|AnomalyBrief|EventTrail|SeriesSample)" src || trueRepository: n9e/fe
Length of output: 7028
Remove the any annotations from table render prop parameters.
Pass the existing row types into these callbacks: use EvalSeriesSample for the series rowKey/render callbacks, EvalAnomalyBrief for anomalies, and EvalEventTrail for events. This keeps the API response contract checked through the presentation layer and removes new any annotations that violate the TypeScript guideline.
🤖 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 `@src/pages/alertRules/List/EvalRecordsDrawer/index.tsx` at line 69, Update the
table callbacks in the EvalRecordsDrawer to remove any annotations and use the
existing row types: EvalSeriesSample for series rowKey/render callbacks,
EvalAnomalyBrief for anomaly callbacks, and EvalEventTrail for event callbacks.
Preserve the current callback behavior while ensuring each table’s API response
type is enforced through its presentation props.
Source: Coding guidelines
| render: (record: any) => { | ||
| const last = _.last(record.points as [number, number][]); | ||
| if (!last) return '-'; | ||
| return ( | ||
| <Tooltip | ||
| title={_.map(record.points as [number, number][], (p) => `${moment.unix(p[0]).format('MM-DD HH:mm:ss')} → ${formatValue(p[1])}`).join('\n')} | ||
| overlayClassName='eval-records-points-tooltip' | ||
| > | ||
| <span>{formatValue(last[1])}</span> | ||
| </Tooltip> | ||
| ); | ||
| }, | ||
| }, | ||
| { | ||
| title: t('eval_records.point_time'), | ||
| width: 160, | ||
| render: (record: any) => { | ||
| const last = _.last(record.points as [number, number][]); | ||
| return last ? moment.unix(last[0]).format('YYYY-MM-DD HH:mm:ss') : '-'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Read the row from the second render argument.
These columns omit dataIndex, but Table still passes (value, record, ...) to the render callback. Read the row from the second argument, not the first, when accessing record.points or related fields.
🤖 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 `@src/pages/alertRules/List/EvalRecordsDrawer/index.tsx` around lines 89 - 107,
Update both column render callbacks in the eval records table to read the row
from the second render argument, then access its points field for the latest
value and timestamp. Keep the existing formatting, tooltip, and fallback
behavior unchanged.
| {_.map(_.uniqBy(nodeErrs, 'instance'), (e, i) => ( | ||
| <pre key={i} className='eval-records-node-err-url'> | ||
| {`curl -u <user>:<pass> 'http://${e.instance}/v1/n9e/eval-records?rule_id=${rid}&datasource_id=${e.datasource_id}&from=${queriedRange?.from || ''}&to=${ | ||
| queriedRange?.to || '' | ||
| }'`} | ||
| </pre> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file outline ---\n'
ast-grep outline src/pages/alertRules/List/EvalRecordsDrawer/index.tsx --view expanded 2>/dev/null | sed -n '1,220p' || true
printf '\n--- relevant EvalRecordsDrawer section ---\n'
sed -n '1,440p' src/pages/alertRules/List/EvalRecordsDrawer/index.tsx
printf '\n--- EvalRecordsNodeErr type/services ---\n'
sed -n '116,145p' src/pages/alertRules/services.ts
printf '\n--- static verifier: curl generated URL structure ---\n'
python3 - <<'PY'
import re
from pathlib import Path
p = Path('src/pages/alertRules/List/EvalRecordsDrawer/index.tsx')
text = p.read_text()
m = re.search(r"curl -u <user>:<pass> 'http://\$\{e\.instance\}/v1/n9e/eval-records\?rule_id=\$\{rid\}&datasource_id=\$\{e\.datasource_id\}&from=\$\{queriedRange\?\.from \|\| ''\}&to=\$\{ queriedRange\?\.to \|\| '' \}'", text, re.S)
print('matches_plain_http_template=', bool(m))
if bool(m):
print('plain_http_token_present=', 'http://${e.instance}' in text)
print('https_url_token_present=', 'https://'.upper() in text or 'https://' in text)
PY
printf '\n--- loopback/http check for e.instance in generated command ---\n'
python3 - <<'PY'
from pathlib import Path
text = Path('src/pages/alertRules/List/EvalRecordsDrawer/index.tsx').read_text()
start = text.index("curl -u <user>:<pass>")
end = text.index('</pre>', start)
cmd = text[start:end]
print(cmd)
print('starts_with_https=', cmd.strip().lstrip().startswith("curl -u <user>:<pass> 'https://"))
print('starts_with_http=', cmd.strip().lstrip().startswith("curl -u <user>:<pass> 'http://"))
print('mentions_loopback_or_localhost=', bool(re.search(r'(127\.0\.0\.1|localhost|loopback)', cmd)))
PYRepository: n9e/fe
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file outline ---\n'
ast-grep outline src/pages/alertRules/List/EvalRecordsDrawer/index.tsx --view expanded 2>/dev/null | sed -n '1,220p' || true
printf '\n--- relevant EvalRecordsDrawer section ---\n'
sed -n '1,440p' src/pages/alertRules/List/EvalRecordsDrawer/index.tsx
printf '\n--- EvalRecordsNodeErr type/services ---\n'
sed -n '116,145p' src/pages/alertRules/services.ts
printf '\n--- static verifier: curl generated URL structure ---\n'
python3 - <<'PY'
import re
from pathlib import Path
p = Path('src/pages/alertRules/List/EvalRecordsDrawer/index.tsx')
text = p.read_text()
m = re.search(r"curl -u <user>:<pass> 'http://\$\{e\.instance\}/v1/n9e/eval-records\?rule_id=\$\{rid\}&datasource_id=\$\{e\.datasource_id\}&from=\$\{queriedRange\?\.from \|\| ''\}&to=\$\{ queriedRange\?\.to \|\| '' \}'", text, re.S)
print('matches_plain_http_template=', bool(m))
if bool(m):
print('plain_http_token_present=', 'http://${e.instance}' in text)
print('https_url_token_present=', 'https://'.upper() in text or 'https://' in text)
print('mentions_loopback_or_localhost=', bool(re.search(r'(127\.0\.0\.1|localhost|loopback)', text)))
PYRepository: n9e/fe
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- relevant EvalRecordsDrawer section ---'
sed -n '1,440p' src/pages/alertRules/List/EvalRecordsDrawer/index.tsx
echo '--- EvalRecordsNodeErr type/services ---'
sed -n '116,145p' src/pages/alertRules/services.ts
echo '--- static verifier: curl generated URL structure ---'
python3 - <<'PY'
from pathlib import Path
import re
path = Path('src/pages/alertRules/List/EvalRecordsDrawer/index.tsx')
text = path.read_text()
curl_start = text.index('curl -u <user>:<pass>')
curl_section = text[curl_start:text.index('</pre>', curl_start)]
print('plain_http_token_present=', 'http://${e.instance}' in text)
print('https_url_token_present=', 'https://' in text)
print('mentions_loopback_or_localhost=', bool(re.search(r'(127\.0\.0\.1|localhost|loopback)', text)))
print('generated_section=', repr(curl_section[:300]))
PYRepository: n9e/fe
Length of output: 17569
Sensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External
Reachability path
● Entry
src/pages/alertRules/List/ListNG.tsx:49
Props
│
▼
● Sink
src/pages/alertRules/List/EvalRecordsDrawer/index.tsx
Send the generated curl command over authenticated HTTPS.
EvalRecordsNodeErr.instance can be an arbitrary node address, but the generated command sends curl -u <user>:<pass> over http://${e.instance}, exposing BasicAuth credentials in clear transport. If the endpoint is only safe locally, generate a loopback URL and enforce/document that deployment constraint.
🤖 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 `@src/pages/alertRules/List/EvalRecordsDrawer/index.tsx` around lines 397 -
402, Update the curl command rendering in the EvalRecordsNodeErr mapping to use
authenticated HTTPS instead of http:// when interpolating e.instance, preserving
the existing query parameters and credentials. If arbitrary instances cannot
safely support HTTPS, constrain the generated endpoint to loopback and enforce
that deployment assumption rather than exposing BasicAuth over cleartext.
…k only persisted event hashes
Summary by CodeRabbit