Conversation
…FIXED (TegoLabs#270) * TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
…FIXED (TegoLabs#270) * TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
- Add docker-compose.devnet.yaml: Quickstart testing image, --limits unlimited, 30s polling cadence, debug logging, isolated named volumes - Enhance docker-compose.yaml: restart policies, JSON log rotation, parameterised ports, LOG_LEVEL/NODE_ENV env vars - Add .env.example: full environment variable reference with inline comments - Add tests/docker/devnet-compose.test.ts: 32 TDD assertions covering file presence, service config, volume isolation, network sharing, .env.example, and compose merge compatibility - Update .dockerignore: exclude compose files, systemd/, docs/, templates/ - Update .gitignore: allow .env.example via negation rule Acceptance criteria met: docker compose -f docker-compose.yaml -f docker-compose.devnet.yaml up boots daemon and mock RPC environment successfully. All 530 tests pass, 63 docker-specific tests, 5 skipped TODOs.
…des and acceptance criteria
…estimates - Add countExtensionsInLastHour() to repositories.ts to query extension_history for the past 60-minute window (issue TegoLabs#142) - Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue TegoLabs#142) - Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue TegoLabs#142) - Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue TegoLabs#142) - Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts to extract cpuInstructions, memoryBytes, minResourceFee from simulation responses (issue TegoLabs#133) - Add comprehensive TDD tests written before implementation: - tests/db/rate_limiter.test.ts: countExtensionsInLastHour edge cases - tests/core/rate_limiter.test.ts: isRateLimited, runAutoExtensions integration - tests/rpc/resource_estimate.test.ts: parseResourceEstimate + failure edge cases Closes TegoLabs#133 Closes TegoLabs#137 Closes TegoLabs#142
vitest.config.ts only globs tests/**/*.test.ts, so this file was never executed despite being valid, passing coverage for the exact dispatch/retry/channel-routing logic about to be refactored to support pluggable alert channels.
Central registration point for alert channel plugins. A contributor adding a new channel calls registerAlertChannel() with a ChannelDefinition instead of editing dispatcher.ts's channel map, the CLI's --type if/else chain, and a DB CHECK constraint.
Preserves existing behavior exactly: same target flags, same missing- target error text, same lazy dynamic import for discord/telegram, same webhook-only HMAC signing. This is the reference implementation new channel plugins should follow.
Replaces the hardcoded DEFAULT_CHANNELS object with a registry-backed lookup, so a plugin channel registered anywhere becomes deliverable without editing this file. Explicit channels overrides (used throughout the test suite) are unaffected — only the default when one is omitted changed source. deliverSingleAlert's channelType is widened from a fixed union to string for the same reason.
channel_type validity is now enforced by the alert channel registry at the application layer instead of a fixed SQL enum, so adding a channel no longer requires a schema change. The CHECK now only guards against an empty string.
…ration The SCHEMA comment-stripper (`--.*\n`) silently failed to match comments ending in \r\n, since JS's `.` excludes all line terminators including \r. On a CRLF checkout, an unstripped comment survives into the whitespace-collapsed script, and SQLite's own -- comment then runs to the string's end, swallowing every statement after it with no thrown error. Switched to `--[^\n]*\n`, which matches either line ending. Latent since schema.sql had no comments before this change. Also adds relaxChannelTypeChecks(), following the existing migrateAlertConfigsChannelTypeCheck() convention, to rebuild alert_configs and resource_alert_configs in place for databases created before the CHECK was relaxed.
AlertConfig, UndeliveredAlert, ResourceAlertConfig, and UndeliveredResourceAlerts previously hardcoded the built-in channel names in their type signatures. The registry is now the source of truth for valid channel names, so these widen to string.
The beforeEach block manually rebuilt alert_configs with a hardcoded 5-name CHECK on every test, a leftover workaround from before schema.sql had these columns natively. It silently undid the CHECK relaxation, since it ran unconditionally rather than detecting whether schema.sql already had the change. getDatabaseForTesting() already execs the current schema.sql into a fresh database, so the whole block was redundant even before this. Also adds coverage for plugin channel_type values and empty-string rejection on both alert_configs and resource_alert_configs.
Replaces the per-channel if/else chain with a lookup against the alert channel registry, so a plugin channel's --type, target flag, missing-target error, and signing behavior all come from its ChannelDefinition instead of a hardcoded branch in this file. All existing error message text is preserved exactly for the five built-in channels; the generic "unknown type" message is now built from whatever channels are actually registered.
…a dashboard (TegoLabs#336) - Move src/alerts/pagerduty.test.ts to tests/alerts/ and delete the orphaned file - Create devops/grafana/sorokeep-dashboard.json with TTL health, extension cost, alert delivery, and daemon activity panels - Update README roadmap to link Grafana dashboard Closes TegoLabs#356, TegoLabs#336
|
@Salauayo Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details🛑 Comments failed to post (1)
🔇 Additional comments (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change adds a Prometheus-backed Grafana dashboard and links it from the README roadmap. It also adds Vitest coverage for PagerDuty trigger, resolve, request wiring, and HTTP error behavior. ChangesObservability dashboard
PagerDuty alert tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 5
🤖 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 `@devops/grafana/sorokeep-dashboard.json`:
- Around line 134-219: Update the Prometheus expressions for the identified
panels—“TTL Distribution by Contract,” “Entries Below Threshold,” “Minimum TTL
Remaining,” and the System Overview statistics—to apply the contract selector
using the existing contract_id regex filter. Preserve the per-contract grouping
and aggregations while adding `{contract_id=~"$contract"}` consistently so the
dashboard selector affects every listed panel.
- Around line 220-286: Update the Prometheus targets for the TTL Remaining Over
Time panel and the sibling panel around the other referenced occurrence to
include the `$network` template variable in their label selectors, alongside the
existing `$contract` filter. Apply the filter only to metrics exposing the
`network` label so changing the network dashboard variable affects both panels.
- Around line 470-487: Update the Grafana targets for the 5-minute total
legends, including the target near sorokeep_extension_cost_xlm_total and the
corresponding target near lines 543-552, to use Prometheus increase() over [5m]
instead of rate(). Preserve the existing legend formats, refIds, and dashboard
structure.
- Around line 624-633: The success-rate target uses the undefined
sorokeep_alert_delivery_success_total metric. Update the target expression in
the success-rate panel to calculate successful deliveries from
sorokeep_alert_delivery_total using its existing status="success" label, while
retaining the total delivery denominator and percentage calculation.
In `@tests/alerts/pagerduty.test.ts`:
- Around line 72-82: Strengthen the “uses resolve event_action for
alert_resolved” test by asserting mockFetch was called exactly once, and verify
the request URL and routing key match the expected PagerDuty destination and
“resolve-key” value, matching the trigger test’s coverage.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 489c2511-bb4d-401c-8d8a-dbbd6a605735
📒 Files selected for processing (3)
README.mddevops/grafana/sorokeep-dashboard.jsontests/alerts/pagerduty.test.ts
📜 Review details
🔇 Additional comments (8)
tests/alerts/pagerduty.test.ts (2)
1-3: LGTM!Also applies to: 8-29, 31-38, 49-70, 84-89
5-6: 🩺 Stability & AvailabilityNo lifecycle change needed.
vitest.config.tsdoes not enableunstubGlobals, and the file’safterEachrestoresfetchat module-file teardown, soglobalThis.fetchis not left mocked after the test suite.> Likely an incorrect or invalid review comment.devops/grafana/sorokeep-dashboard.json (5)
1-74: LGTM!
89-219: LGTM!
288-354: LGTM!Also applies to: 367-887, 888-1105
890-959: LGTM!
1107-1150: LGTM!Also applies to: 1151-1163
README.md (1)
663-665: LGTM!
| { | ||
| "id": 3, | ||
| "title": "Minimum TTL Remaining", | ||
| "type": "gauge", | ||
| "gridPos": { "h": 6, "w": 4, "x": 4, "y": 1 }, | ||
| "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, | ||
| "fieldConfig": { | ||
| "defaults": { | ||
| "color": { "mode": "thresholds" }, | ||
| "mappings": [], | ||
| "thresholds": { | ||
| "mode": "absolute", | ||
| "steps": [ | ||
| { "color": "red", "value": null }, | ||
| { "color": "yellow", "value": 500 }, | ||
| { "color": "green", "value": 2000 } | ||
| ] | ||
| }, | ||
| "unit": "short" | ||
| }, | ||
| "overrides": [] | ||
| }, | ||
| "options": { | ||
| "reduceOptions": { | ||
| "calcs": ["min"], | ||
| "fields": "", | ||
| "values": false | ||
| }, | ||
| "showThresholdLabels": false, | ||
| "showThresholdMarkers": true | ||
| }, | ||
| "targets": [ | ||
| { | ||
| "expr": "min(sorokeep_ttl_remaining_ledgers)", | ||
| "legendFormat": "Min TTL", | ||
| "interval": "", | ||
| "exemplar": true, | ||
| "refId": "A" | ||
| } | ||
| ], | ||
| "description": "Lowest remaining TTL (in ledgers) across all tracked contracts." | ||
| }, | ||
| { | ||
| "id": 4, | ||
| "title": "TTL Distribution by Contract", | ||
| "type": "bargauge", | ||
| "gridPos": { "h": 6, "w": 8, "x": 8, "y": 1 }, | ||
| "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, | ||
| "fieldConfig": { | ||
| "defaults": { | ||
| "color": { "mode": "thresholds" }, | ||
| "mappings": [], | ||
| "thresholds": { | ||
| "mode": "absolute", | ||
| "steps": [ | ||
| { "color": "red", "value": null }, | ||
| { "color": "yellow", "value": 500 }, | ||
| { "color": "green", "value": 2000 } | ||
| ] | ||
| }, | ||
| "unit": "short", | ||
| "min": 0 | ||
| }, | ||
| "overrides": [] | ||
| }, | ||
| "options": { | ||
| "orientation": "horizontal", | ||
| "reduceOptions": { | ||
| "calcs": ["lastNotNull"], | ||
| "fields": "", | ||
| "values": false | ||
| }, | ||
| "showUnfilled": true, | ||
| "displayMode": "gradient" | ||
| }, | ||
| "targets": [ | ||
| { | ||
| "expr": "min by (contract_id) (sorokeep_ttl_remaining_ledgers)", | ||
| "legendFormat": "{{contract_id}}", | ||
| "interval": "", | ||
| "exemplar": true, | ||
| "refId": "A" | ||
| } | ||
| ], | ||
| "description": "Per-contract minimum remaining TTL." | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Several per-contract/global panels ignore the $contract filter entirely.
"TTL Distribution by Contract" (Line 211: min by (contract_id) (sorokeep_ttl_remaining_ledgers)), "Entries Below Threshold" (Line 125), "Minimum TTL Remaining" (Line 167), and the System Overview stats (Lines 1007, 1052, 1097) never apply {contract_id=~"$contract"}. For panels that are explicitly per-contract breakdowns (like the bar gauge), this means the contract selector silently does nothing, which is inconsistent with the two timeseries panels that do respect it.
Also applies to: 973-1105
🤖 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 `@devops/grafana/sorokeep-dashboard.json` around lines 134 - 219, Update the
Prometheus expressions for the identified panels—“TTL Distribution by Contract,”
“Entries Below Threshold,” “Minimum TTL Remaining,” and the System Overview
statistics—to apply the contract selector using the existing contract_id regex
filter. Preserve the per-contract grouping and aggregations while adding
`{contract_id=~"$contract"}` consistently so the dashboard selector affects
every listed panel.
| { | ||
| "id": 5, | ||
| "title": "TTL Remaining Over Time", | ||
| "type": "timeseries", | ||
| "gridPos": { "h": 8, "w": 12, "x": 0, "y": 7 }, | ||
| "datasource": { "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, | ||
| "fieldConfig": { | ||
| "defaults": { | ||
| "color": { "mode": "palette-classic" }, | ||
| "custom": { | ||
| "axisCenteredZero": false, | ||
| "axisColorMode": "text", | ||
| "axisLabel": "", | ||
| "axisPlacement": "auto", | ||
| "barAlignment": 0, | ||
| "drawStyle": "line", | ||
| "fillOpacity": 20, | ||
| "gradientMode": "opacity", | ||
| "hideFrom": { | ||
| "legend": false, | ||
| "tooltip": false, | ||
| "viz": false | ||
| }, | ||
| "lineInterpolation": "smooth", | ||
| "lineWidth": 1, | ||
| "pointSize": 3, | ||
| "scaleDistribution": { "type": "linear" }, | ||
| "showPoints": "never", | ||
| "spanNulls": true, | ||
| "stacking": { "group": "A", "mode": "none" }, | ||
| "thresholdsStyle": { "mode": "line" } | ||
| }, | ||
| "mappings": [], | ||
| "thresholds": { | ||
| "mode": "absolute", | ||
| "steps": [ | ||
| { "color": "green", "value": null }, | ||
| { "color": "red", "value": 100 } | ||
| ] | ||
| }, | ||
| "unit": "short" | ||
| }, | ||
| "overrides": [] | ||
| }, | ||
| "options": { | ||
| "legend": { | ||
| "calcs": ["lastNotNull", "min", "max"], | ||
| "displayMode": "table", | ||
| "placement": "bottom", | ||
| "showLegend": true | ||
| }, | ||
| "tooltip": { | ||
| "mode": "multi", | ||
| "sort": "desc" | ||
| } | ||
| }, | ||
| "targets": [ | ||
| { | ||
| "expr": "sorokeep_ttl_remaining_ledgers{contract_id=~\"$contract\"}", | ||
| "legendFormat": "{{contract_id}} — {{entry_key_xdr}}", | ||
| "interval": "", | ||
| "exemplar": true, | ||
| "refId": "A" | ||
| } | ||
| ], | ||
| "description": "Time series of remaining TTL for each tracked contract entry." | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
$contract filter is applied here but the sibling $network template variable is never referenced anywhere in the dashboard.
These two panels correctly filter on contract_id=~"$contract", but no panel target uses $network even though the "network" template variable is defined in templating.list (Line 1123). The network filter is effectively decorative — selecting a network in the dashboard has no effect on any panel.
🔧 Add the network label filter to panels that expose a `network` label
- "expr": "sorokeep_ttl_remaining_ledgers{contract_id=~\"$contract\"}",
+ "expr": "sorokeep_ttl_remaining_ledgers{contract_id=~\"$contract\", network=~\"$network\"}",Also applies to: 344-353
🤖 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 `@devops/grafana/sorokeep-dashboard.json` around lines 220 - 286, Update the
Prometheus targets for the TTL Remaining Over Time panel and the sibling panel
around the other referenced occurrence to include the `$network` template
variable in their label selectors, alongside the existing `$contract` filter.
Apply the filter only to metrics exposing the `network` label so changing the
network dashboard variable affects both panels.
| "targets": [ | ||
| { | ||
| "expr": "rate(sorokeep_extension_cost_xlm_total[5m])", | ||
| "legendFormat": "XLM / 5min", | ||
| "interval": "", | ||
| "exemplar": true, | ||
| "refId": "A" | ||
| }, | ||
| { | ||
| "expr": "avg_over_time(sorokeep_extension_cost_per_entry[5m])", | ||
| "legendFormat": "Avg cost per entry", | ||
| "interval": "", | ||
| "exemplar": true, | ||
| "refId": "B" | ||
| } | ||
| ], | ||
| "description": "Rate of XLM spend on auto-extensions and average cost per extended entry." | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate dashboard =="
fd -a 'sorokeep-dashboard\.json$' . || true
echo "== relevant sections =="
file="$(fd 'sorokeep-dashboard\.json$' . | head -n1)"
if [ -n "${file:-}" ]; then
sed -n '430,570p' "$file"
fi
echo "== search related target expressions =="
if [ -n "${file:-}" ]; then
rg -n '"expr":\s*"(rate|increase)\([^"]+\[5m\])' "$file" -C 3 || true
fi
echo "== simple Prometheus function semantics from installed local docs if present =="
if [ -x "$(command -v promtool 2>/dev/null)" ]; then
promtool --version 2>&1 | head -n1
else
echo "promtool not available"
fiRepository: AbdulmalikAlayande/sorokeep
Length of output: 4470
🌐 Web query:
Prometheus query language rate function per-second average rate of increase documentation
💡 Result:
The Prometheus rate function calculates the per-second average rate of increase of a time series over a specified time range vector [1][2]. Key characteristics and functionality of rate include: - Calculation: It determines the rate by measuring the increase in the counter value over the provided lookbehind window (e.g., [5m]) and dividing that increase by the duration of the window [3]. - Extrapolation: The function automatically extrapolates to the ends of the specified time range [1][2]. This feature helps account for missed scrapes or scrape cycles that are not perfectly aligned with the query time range [1][2]. - Counter Resets: It automatically detects and compensates for counter resets (e.g., due to target restarts) [1][2]. - Usage: It is designed exclusively for use with counter metrics (both for floats and native histograms) [1][2]. It is considered the best choice for alerting and for graphing slow-moving counters because it provides a smoothed rate [1][4]. - Best Practices: When combining rate with aggregation operators (like sum or any _over_time function), always apply the rate function first, then perform the aggregation [1][2]. Aggregating before applying rate prevents Prometheus from correctly detecting counter resets [1][2]. Comparison with similar functions: - irate: Calculates the instantaneous per-second rate of increase using only the last two data points in the lookbehind window [4][5]. It is more responsive to rapid changes but is generally too noisy for alerting [4][6]. - increase: Calculates the absolute total increase over the specified time range, effectively functioning as rate multiplied by the duration of the range [7][6]. Top results: [1] https://prometheus.io/docs/prometheus/latest/querying/functions/ [2] https://github.com/prometheus/prometheus/blob/main/docs/querying/functions.md [4] https://groups.google.com/g/prometheus-developers/c/hl_NODpFruw [5] https://promlabs.com/blog/2021/01/29/how-exactly-does-promql-calculate-rates/ [6] https://last9.io/blog/prometheus-rate-function/
Citations:
- 1: https://prometheus.io/docs/prometheus/latest/querying/functions/
- 2: https://github.com/prometheus/prometheus/blob/main/docs/querying/functions.md
- 3: https://stackoverflow.com/questions/65400298/promql-what-is-rate-function-meant-for
- 4: https://groups.google.com/g/prometheus-developers/c/hl_NODpFruw
- 5: https://promlabs.com/blog/2021/01/29/how-exactly-does-promql-calculate-rates/
- 6: https://last9.io/blog/prometheus-rate-function/
- 7: https://pagertree.com/learn/prometheus/promql/counter-rates-and-increases
Use increase() for 5-minute totals in these legends.
rate(...[5m]) returns a per-second average, so XLM / 5min and Extensions / 5min imply values ~300x larger than shown. If these should be 5-minute totals, switch the targets at lines 470-487 and 543-552 to increase().
🤖 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 `@devops/grafana/sorokeep-dashboard.json` around lines 470 - 487, Update the
Grafana targets for the 5-minute total legends, including the target near
sorokeep_extension_cost_xlm_total and the corresponding target near lines
543-552, to use Prometheus increase() over [5m] instead of rate(). Preserve the
existing legend formats, refIds, and dashboard structure.
| "targets": [ | ||
| { | ||
| "expr": "rate(sorokeep_alert_delivery_success_total[$__rate_interval]) / (rate(sorokeep_alert_delivery_total[$__rate_interval])) * 100", | ||
| "legendFormat": "Success rate", | ||
| "interval": "", | ||
| "exemplar": true, | ||
| "refId": "A" | ||
| } | ||
| ], | ||
| "description": "Percentage of alert deliveries that succeeded. Threshold lines at 95% (warning) and 99.5% (good)." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching dashboard/pattern:"
git ls-files | rg 'devops/grafana/sorokeep-dashboard.json|sorokeep|alert_delivery|metrics' || true
echo
echo "Dashboard relevant lines:"
if [ -f devops/grafana/sorokeep-dashboard.json ]; then
nl -ba devops/grafana/sorokeep-dashboard.json | sed -n '580,780p'
fi
echo
echo "Search for metric occurrences in repo:"
rg -n "sorokeep_alert_delivery_(success_total|total)|alert_delivery_success_total|alert_delivery_total|status=(\"|')success|status=\"failure\"" . --glob '!node_modules' --glob '!dist' --glob '!build' || true
echo
echo "Git status/diff summary:"
git status --short
git diff --stat || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Dashboard relevant lines:"
sed -n '600,770p' devops/grafana/sorokeep-dashboard.json | cat -n
echo
echo "Metric occurrences:"
grep -RIn "sorokeep_alert_delivery_(success_total|total)|alert_delivery_success_total|alert_delivery_total|status=.success.|status=\047success\047|status=\042success\042|status=.failure.|status=\047failure\047|status=\042failure\042" . --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build || true
echo
echo "Potential code definitions and Prometheus exporters/usages:"
grep -RIn "sorokeep_alert_delivery|DELIVERY|alert_delivery|Counter\\(|Gauge\\(|METRICS|/metrics|metrics" . --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 6882
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Metric occurrences:"
grep -RIn "sorokeep_alert_delivery_(success_total|total)|alert_delivery_success_total|alert_delivery_total|status=.success.|status=.failure." . --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build || true
echo
echo "Potential code definitions/usages of metrics:"
grep -RIn "sorokeep_alert_delivery|DELIVERY|alert_delivery|Counter\.|Gauge\.|METRICS|/metrics|metrics" . --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build || true
echo
echo "Search dashboard for all metric references:"
grep -oP '"expr"\s*:\s*"[^"]*"' devops/grafana/sorokeep-dashboard.json | sort -uRepository: AbdulmalikAlayande/sorokeep
Length of output: 1508
🌐 Web query:
Sorokeep alert_delivery_success_total sorokeep_alert_delivery_total
💡 Result:
The terms alert_delivery_success_total and sorokeep_alert_delivery_total are not standard, documented metrics within the Sorokeep project [1][2][3]. Sorokeep is an open-source, off-chain operations tool designed for monitoring Soroban smart contract TTLs, managing automated storage extensions, and dispatching alerts (via Webhooks, Slack, Discord, Telegram, and PagerDuty) [1][2][3]. While Sorokeep maintains an internal, database-backed queue for managing alert deliveries and supports retry logic for failed attempts [1], it does not expose or use the specific Prometheus-style metrics you mentioned [1][4]. The names alert_delivery_success_total and sorokeep_alert_delivery_total appear to follow a naming convention common in Prometheus or similar instrumentation libraries (e.g., [5]), but they do not exist in the codebase of the Sorokeep project, which primarily utilizes a local SQLite database to track alert status, history, and retry counts [1][6][2]. If you are working with an implementation that includes these metrics, they are likely custom additions or part of a third-party exporter/wrapper built on top of Sorokeep rather than native components of the tool [1][2][3].
Citations:
- 1: https://github.com/AbdulmalikAlayande/sorokeep
- 2: https://github.com/veloura-dev/sorokeep
- 3: https://github.com/adepoju2006/sorokeep
- 4: f57d1ab
- 5: https://docs.cloudera.com/management-console/1.5.4/monitoring-metrics/topics/cdppvc_ds_prometheus_notifications_errors_total_erver.html
- 6: feat(cli): add per-contract polling interval overrides #261
Use the existing delivery-status metric in the success-rate panel.
The rest of this dashboard models deliveries with sorokeep_alert_delivery_total{status="success"} and sorokeep_alert_delivery_total{status="failure"}; the separate sorokeep_alert_delivery_success_total counter is not defined here and will render this panel with no data.
🔧 Align with the `status` label pattern used by the adjacent panels
- "expr": "rate(sorokeep_alert_delivery_success_total[$__rate_interval]) / (rate(sorokeep_alert_delivery_total[$__rate_interval])) * 100",
+ "expr": "sum(rate(sorokeep_alert_delivery_total{status=\"success\"}[$__rate_interval])) / sum(rate(sorokeep_alert_delivery_total[$__rate_interval])) * 100",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "targets": [ | |
| { | |
| "expr": "rate(sorokeep_alert_delivery_success_total[$__rate_interval]) / (rate(sorokeep_alert_delivery_total[$__rate_interval])) * 100", | |
| "legendFormat": "Success rate", | |
| "interval": "", | |
| "exemplar": true, | |
| "refId": "A" | |
| } | |
| ], | |
| "description": "Percentage of alert deliveries that succeeded. Threshold lines at 95% (warning) and 99.5% (good)." | |
| "targets": [ | |
| { | |
| "expr": "sum(rate(sorokeep_alert_delivery_total{status=\"success\"}[$__rate_interval])) / sum(rate(sorokeep_alert_delivery_total[$__rate_interval])) * 100", | |
| "legendFormat": "Success rate", | |
| "interval": "", | |
| "exemplar": true, | |
| "refId": "A" | |
| } | |
| ], | |
| "description": "Percentage of alert deliveries that succeeded. Threshold lines at 95% (warning) and 99.5% (good)." |
🤖 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 `@devops/grafana/sorokeep-dashboard.json` around lines 624 - 633, The
success-rate target uses the undefined sorokeep_alert_delivery_success_total
metric. Update the target expression in the success-rate panel to calculate
successful deliveries from sorokeep_alert_delivery_total using its existing
status="success" label, while retaining the total delivery denominator and
percentage calculation.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🤖 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 `@devops/grafana/sorokeep-dashboard.json`:
- Around line 134-219: Update the Prometheus expressions for the identified
panels—“TTL Distribution by Contract,” “Entries Below Threshold,” “Minimum TTL
Remaining,” and the System Overview statistics—to apply the contract selector
using the existing contract_id regex filter. Preserve the per-contract grouping
and aggregations while adding `{contract_id=~"$contract"}` consistently so the
dashboard selector affects every listed panel.
- Around line 220-286: Update the Prometheus targets for the TTL Remaining Over
Time panel and the sibling panel around the other referenced occurrence to
include the `$network` template variable in their label selectors, alongside the
existing `$contract` filter. Apply the filter only to metrics exposing the
`network` label so changing the network dashboard variable affects both panels.
- Around line 470-487: Update the Grafana targets for the 5-minute total
legends, including the target near sorokeep_extension_cost_xlm_total and the
corresponding target near lines 543-552, to use Prometheus increase() over [5m]
instead of rate(). Preserve the existing legend formats, refIds, and dashboard
structure.
- Around line 624-633: The success-rate target uses the undefined
sorokeep_alert_delivery_success_total metric. Update the target expression in
the success-rate panel to calculate successful deliveries from
sorokeep_alert_delivery_total using its existing status="success" label, while
retaining the total delivery denominator and percentage calculation.
In `@tests/alerts/pagerduty.test.ts`:
- Around line 72-82: Strengthen the “uses resolve event_action for
alert_resolved” test by asserting mockFetch was called exactly once, and verify
the request URL and routing key match the expected PagerDuty destination and
“resolve-key” value, matching the trigger test’s coverage.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 489c2511-bb4d-401c-8d8a-dbbd6a605735
📒 Files selected for processing (3)
README.mddevops/grafana/sorokeep-dashboard.jsontests/alerts/pagerduty.test.ts
📜 Review details
🔇 Additional comments (8)
tests/alerts/pagerduty.test.ts (2)
1-3: LGTM!Also applies to: 8-29, 31-38, 49-70, 84-89
5-6: 🩺 Stability & AvailabilityNo lifecycle change needed.
vitest.config.tsdoes not enableunstubGlobals, and the file’safterEachrestoresfetchat module-file teardown, soglobalThis.fetchis not left mocked after the test suite.> Likely an incorrect or invalid review comment.devops/grafana/sorokeep-dashboard.json (5)
1-74: LGTM!
89-219: LGTM!
288-354: LGTM!Also applies to: 367-887, 888-1105
890-959: LGTM!
1107-1150: LGTM!Also applies to: 1151-1163
README.md (1)
663-665: LGTM!
🛑 Comments failed to post (1)
tests/alerts/pagerduty.test.ts (1)
72-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Assert the resolve path sends exactly one request.
Unlike the trigger test, this case does not verify call count, URL, or routing key. A duplicate or misrouted resolve request could therefore pass. Add
toHaveBeenCalledTimes(1)and assert the destination/routing key for parity.🤖 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 `@tests/alerts/pagerduty.test.ts` around lines 72 - 82, Strengthen the “uses resolve event_action for alert_resolved” test by asserting mockFetch was called exactly once, and verify the request URL and routing key match the expected PagerDuty destination and “resolve-key” value, matching the trigger test’s coverage.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | ded54f4 | tests/commands/guard-cli-export-import.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
43ad363 to
8692701
Compare
…agerduty.test.ts (#549, #336, #356) Adds devops/grafana/sorokeep-dashboard.json: fleet overview, TTL-over-time, extension activity/cost, and daemon health panels, built fresh against only the 8 metrics actually registered in src/observability/metrics/*.ts (the fork's version referenced ~15 fabricated metric names — sorokeep_ttl_*, sorokeep_monitor_*, sorokeep_alert_delivery_* — none of which exist). Cross-references it from README.md and docs/observability.md's previously "not yet bundled" placeholder. Also moves src/alerts/pagerduty.test.ts to tests/alerts/pagerduty.test.ts, where vitest.config.ts's tests/**/*.test.ts glob actually picks it up — no overlap with state_change_alerts.test.ts's mocked dispatcher-routing case.
|
Thanks — closing manually rather than merging. Kept the pagerduty.test.ts move as-is (clean, verified against current pagerduty.ts, no overlap with state_change_alerts.test.ts). The dashboard JSON needed a rebuild though: it referenced ~15 metric names (sorokeep_ttl_remaining_ledgers, sorokeep_monitor_cycle_duration_seconds, sorokeep_alert_delivery_total, etc.) that don't exist in the real registry — none of them matched src/observability/metrics/*.ts. Rebuilt it from scratch using only the 8 metrics actually registered, and cross-referenced it from README.md and docs/observability.md. Merged as b98683a. Good initiative bundling the two issues — thanks! |
18 participants
Overview
This PR addresses two open issues: relocating the orphaned PagerDuty test file so it runs as part of the test suite, and publishing an example Grafana dashboard JSON that teams can import in five minutes after setting up Prometheus to scrape the upcoming
/metricsendpoint.Related Issue
Closes #356, Closes #336
Changes
Issue #356 — Reconcile orphaned
src/alerts/pagerduty.test.tssrc/alerts/pagerduty.test.ts→tests/alerts/pagerduty.test.tssrc/alerts/pagerduty.tstests/alerts/state_change_alerts.test.tsfor PagerDuty overlap — its cases test the dispatcher integration, not the standalonesendPagerDutyAlertfunction, so no dedup neededsrc/alerts/pagerduty.test.ts— orphaned file no longer covered byvitest.config.tsglob patterntests/**/*.test.tsIssue #336 — Publish Grafana Dashboard JSON
devops/grafana/sorokeep-dashboard.jsonsorokeep_-prefixed metric names aligned with the upcoming/metricsendpointREADME.md— added link to the dashboard in the Roadmap sectionVerification Results
src/alerts/pagerduty.test.tsno longer existstests/alerts/and passestests/alerts/pagerduty.test.ts— 3/3 tests pass/metricssorokeep_-prefixed metrics from the planned endpoint