feat(observability): expose contracts-tracked and entries-tracked gauges - #591
Closed
kuse-design wants to merge 1 commit into
Closed
kuse-design wants to merge 1 commit into
kuse-design wants to merge 1 commit into
Conversation
Add fleet-scale Prometheus gauges that report how many contracts and entries sorokeep is watching, labeled by network. New files: - src/observability/metrics/fleet.ts - src/observability/registry.ts - tests/observability/fleet.test.ts (10 tests) Added prom-client dependency.
📝 WalkthroughWalkthroughAdds Prometheus fleet gauges for tracked contracts and entries, a shared observability registry with database collection, the ChangesFleet metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Database
participant collectAll
participant FleetGauges
participant Registry
Database->>collectAll: provide SQLite data
collectAll->>FleetGauges: collect(db)
FleetGauges->>FleetGauges: aggregate totals by network
FleetGauges->>Registry: update contracts and entries gauges
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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: 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 `@src/observability/metrics/fleet.ts`:
- Around line 46-54: Replace the per-contract getEntriesForContract calls in
collectAll with a repository-level aggregate query grouped by contracts.network,
using a LEFT JOIN so networks with no entries remain represented. Add the
repository method and populate entriesByNetwork from its grouped results,
eliminating the synchronous query inside the contracts loop.
In `@tests/observability/fleet.test.ts`:
- Around line 61-65: Bind every expected metric count to its network label by
replacing independent label and number assertions with full sample assertions.
Update tests/observability/fleet.test.ts lines 61-65 for
sorokeep_contracts_tracked, lines 129-133 for sorokeep_entries_tracked, and
lines 195-200 for each mixed-network contract sample, asserting the exact
network-specific sample and count at each site.
🪄 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: 62507d3e-650e-4445-9e76-c69a015d6c52
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/observability/metrics/fleet.tssrc/observability/registry.tstests/observability/fleet.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI Pipeline / 0_build-and-test (22.x).txt: feat(observability): expose contracts-tracked and entries-tracked gauges
Conclusion: failure
##[group]Run npm audit --audit-level=high
�[36;1mnpm audit --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
`@hono/node-server` <2.0.5
Severity: moderate
Node.js Adapter for Hono: Path traversal in `serve-static` on Windows via encoded backslash (`%5C`) - https://github.com/advisories/GHSA-frvp-7c67-39w9
fix available via `npm audit fix`
node_modules/@hono/node-server
`@modelcontextprotocol/sdk` 1.25.0 - 1.29.0
Depends on vulnerable versions of `@hono/node-server`
node_modules/@modelcontextprotocol/sdk
axios 1.0.0 - 1.17.0
Severity: high
Axios: Excessive recursion in formDataToJSON can cause denial of service - https://github.com/advisories/GHSA-42h9-826w-cgv3
Axios: Prototype pollution auth subfields can inject Basic auth - https://github.com/advisories/GHSA-xj6q-8x83-jv6g
Axios: Deep formToJSON Key Recursion Can Cause Denial of Service - https://github.com/advisories/GHSA-pmv8-rq9r-6j72
Axios: Fetch adapter `ReadableStream` uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-jqh4-m9w3-8hp9
Axios: Prototype pollution gadgets can alter axios request construction - https://github.com/advisories/GHSA-mmx7-hfxf-jppx
Axios: NO_PROXY bypass for 0.0.0.0 local addresses in axios - https://github.com/advisories/GHSA-f4gw-2p7v-4548
Axios Node HTTP adapter can use an inherited proxy after interceptor config cloning - https://github.com/advisories/GHSA-gcfj-64vw-6mp9
Axios form serializer maxDepth bypass via {} metatoken - https://github.com/advisories/GHSA-hcpx-6fm6-wx23
Axios: Nested axios option objects can consume polluted prototype values - https://github.com/advisories/GHSA-7q8q-rj6j-mhjq
Axios: HTTP/2 streamed uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-mwf2-3pr3-8698
fix available via `npm audit fix`
node_modules/axios
`@stellar/stellar-sdk` 15.0.1 - 16.0.1
Depends on vulnerable versions of axios
node_modules/@stellar/stellar-sdk
brace-expansion <=5.0.7
...
GitHub Actions: CI Pipeline / build-and-test (22.x): feat(observability): expose contracts-tracked and entries-tracked gauges
Conclusion: failure
##[group]Run npm audit --audit-level=high
�[36;1mnpm audit --audit-level=high�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
# npm audit report
`@hono/node-server` <2.0.5
Severity: moderate
Node.js Adapter for Hono: Path traversal in `serve-static` on Windows via encoded backslash (`%5C`) - https://github.com/advisories/GHSA-frvp-7c67-39w9
fix available via `npm audit fix`
node_modules/@hono/node-server
`@modelcontextprotocol/sdk` 1.25.0 - 1.29.0
Depends on vulnerable versions of `@hono/node-server`
node_modules/@modelcontextprotocol/sdk
axios 1.0.0 - 1.17.0
Severity: high
Axios: Excessive recursion in formDataToJSON can cause denial of service - https://github.com/advisories/GHSA-42h9-826w-cgv3
Axios: Prototype pollution auth subfields can inject Basic auth - https://github.com/advisories/GHSA-xj6q-8x83-jv6g
Axios: Deep formToJSON Key Recursion Can Cause Denial of Service - https://github.com/advisories/GHSA-pmv8-rq9r-6j72
Axios: Fetch adapter `ReadableStream` uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-jqh4-m9w3-8hp9
Axios: Prototype pollution gadgets can alter axios request construction - https://github.com/advisories/GHSA-mmx7-hfxf-jppx
Axios: NO_PROXY bypass for 0.0.0.0 local addresses in axios - https://github.com/advisories/GHSA-f4gw-2p7v-4548
Axios Node HTTP adapter can use an inherited proxy after interceptor config cloning - https://github.com/advisories/GHSA-gcfj-64vw-6mp9
Axios form serializer maxDepth bypass via {} metatoken - https://github.com/advisories/GHSA-hcpx-6fm6-wx23
Axios: Nested axios option objects can consume polluted prototype values - https://github.com/advisories/GHSA-7q8q-rj6j-mhjq
Axios: HTTP/2 streamed uploads bypass `maxBodyLength` - https://github.com/advisories/GHSA-mwf2-3pr3-8698
fix available via `npm audit fix`
node_modules/axios
`@stellar/stellar-sdk` 15.0.1 - 16.0.1
Depends on vulnerable versions of axios
node_modules/@stellar/stellar-sdk
brace-expansion <=5.0.7
...
🔇 Additional comments (4)
package.json (1)
57-69: LGTM!src/observability/metrics/fleet.ts (1)
1-29: LGTM!src/observability/registry.ts (1)
1-16: LGTM!tests/observability/fleet.test.ts (1)
16-48: LGTM!Also applies to: 68-100, 136-179
Comment on lines
+46
to
+54
| // Count entries per network. | ||
| const entriesByNetwork = new Map<string, number>(); | ||
| for (const c of contracts) { | ||
| const entries = getEntriesForContract(db, c.id); | ||
| entriesByNetwork.set( | ||
| c.network, | ||
| (entriesByNetwork.get(c.network) ?? 0) + entries.length, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Replace the per-contract entry queries with an aggregate query.
Lines 48-53 run one synchronous SQLite query per contract. At fleet scale, collectAll() can block metrics serving for thousands of statements. Add a repository aggregate grouped by contracts.network (for example, a LEFT JOIN from contracts to contract_entries) and populate entriesByNetwork from that result.
🤖 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/observability/metrics/fleet.ts` around lines 46 - 54, Replace the
per-contract getEntriesForContract calls in collectAll with a repository-level
aggregate query grouped by contracts.network, using a LEFT JOIN so networks with
no entries remain represented. Add the repository method and populate
entriesByNetwork from its grouped results, eliminating the synchronous query
inside the contracts loop.
Comment on lines
+61
to
+65
| // testnet → 2, mainnet → 1 | ||
| expect(metrics).toContain('network="testnet"'); | ||
| expect(metrics).toContain("2"); | ||
| expect(metrics).toContain('network="mainnet"'); | ||
| expect(metrics).toContain("1"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Bind each expected count to its network label. The current assertions can pass when network totals are swapped because they independently search for labels and numbers.
tests/observability/fleet.test.ts#L61-L65: assert full samples such assorokeep_contracts_tracked{network="testnet"} 2.tests/observability/fleet.test.ts#L129-L133: assert fullsorokeep_entries_trackedsamples per network.tests/observability/fleet.test.ts#L195-L200: assert each mixed-network contract sample with its exact count.
📍 Affects 1 file
tests/observability/fleet.test.ts#L61-L65(this comment)tests/observability/fleet.test.ts#L129-L133tests/observability/fleet.test.ts#L195-L200
🤖 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/observability/fleet.test.ts` around lines 61 - 65, Bind every expected
metric count to its network label by replacing independent label and number
assertions with full sample assertions. Update tests/observability/fleet.test.ts
lines 61-65 for sorokeep_contracts_tracked, lines 129-133 for
sorokeep_entries_tracked, and lines 195-200 for each mixed-network contract
sample, asserting the exact network-specific sample and count at each site.
|
@kuse-design 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! 🚀 |
|
| 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.
AbdulmalikAlayande
added a commit
that referenced
this pull request
Aug 1, 2026
…ges (#591, #335) PR #591's registry.ts used a factory-function pattern (createFleetGauges(registry), a separately-exported registry object, collectAll()) incompatible with the established register/collectAllMetrics singleton pattern other merged metrics already use — taking it as-is would have replaced the shared registry and dropped every other metric. Kept the PR's gauge logic exactly (per-network counting via getAllContracts + getEntriesForContract, reset-then-set to drop stale network labels), adapted to the existing module shape: - src/observability/metrics/fleet.ts: sorokeep_contracts_tracked and sorokeep_entries_tracked as plain exported Gauge objects + collectFleetMetrics(db). - registry.ts: two registration lines + added to collectAllMetrics. - Rewrote the tests against the gauges' own .get() (matching the pattern used by budget/ttl/cost tests) instead of a per-test fresh Registry instance, preserving every original test scenario. Verified: tsc clean, full suite 1287/1287, npm audit clean, build succeeds, and manually smoke-tested end-to-end against an isolated temp database — confirmed correct per-network counts through the real HTTP endpoint (2 testnet / 1 mainnet contracts, 2 testnet / 1 mainnet entries). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
|
Thanks — the counting logic (per-network aggregation via getAllContracts + getEntriesForContract, reset-then-set to drop stale networks) was correct and the test coverage was thorough. Closing manually rather than merging: your registry.ts used a factory-function pattern (createFleetGauges(registry), a separate collectAll()) that doesn't match the singleton register/collectAllMetrics pattern the earlier-merged metrics in this phase already established — taking it as-is would have replaced the shared registry and dropped every other metric currently wired in. Kept your gauge logic exactly, just adapted the module shape to match (commit 7bedde2): plain exported Gauge objects + a collectFleetMetrics(db) function, two registration lines in registry.ts. Rewrote the tests against the gauges' own .get() instead of a per-test fresh Registry, preserving every scenario you wrote. Verified: tsc clean, full suite 1287/1287, npm audit clean, build succeeds, manually smoke-tested end-to-end (2 testnet/1 mainnet contracts, 2 testnet/1 mainnet entries — all correct). Good, clean logic — thanks! |
5 tasks
AbdulmalikAlayande
added a commit
that referenced
this pull request
Aug 2, 2026
… the HTTP server (#540, #339) PR #540's registry.ts diff was, again, a complete standalone reimplementation (computeMetrics/formatPrometheus hand-rolling its own SQL queries and Prometheus serializer, bypassing prom-client and every already-registered metric entirely) — the exact same collision pattern as #543/#566/#591/#542. The issue's own step 1 ("extract the metric-computation logic... reusable by both the HTTP route and this command") was actually already satisfied by the real registry.ts — collectAllMetrics(db) + register.metrics() is exactly that shared logic, already used by the /metrics route. Wrote src/commands/metrics.ts fresh against the real registry: collectAllMetrics(db) then register.metrics() for the default Prometheus-text output, or register.getMetricsAsJSON() (prom-client's own structured format) for --json — no new serialization logic needed. Registered in src/cli/program.ts (the current entry point; src/index.ts is now just a thin wrapper around createProgram()). Added tests asserting the command's default output matches what collectAllMetrics + register.metrics() produce directly for the same DB state (the acceptance criteria's actual claim), that --json produces valid parseable JSON with the expected metric/label shape, and the empty-database case. Verified: tsc clean, full suite 1300/1300, npm audit clean, build succeeds, and manually smoke-tested both `sorokeep metrics` and `sorokeep metrics --json` against the compiled CLI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AbdulmalikAlayande
added a commit
that referenced
this pull request
Aug 2, 2026
…ges (#591, #335) PR #591's registry.ts used a factory-function pattern (createFleetGauges(registry), a separately-exported registry object, collectAll()) incompatible with the established register/collectAllMetrics singleton pattern other merged metrics already use — taking it as-is would have replaced the shared registry and dropped every other metric. Kept the PR's gauge logic exactly (per-network counting via getAllContracts + getEntriesForContract, reset-then-set to drop stale network labels), adapted to the existing module shape: - src/observability/metrics/fleet.ts: sorokeep_contracts_tracked and sorokeep_entries_tracked as plain exported Gauge objects + collectFleetMetrics(db). - registry.ts: two registration lines + added to collectAllMetrics. - Rewrote the tests against the gauges' own .get() (matching the pattern used by budget/ttl/cost tests) instead of a per-test fresh Registry instance, preserving every original test scenario. Verified: tsc clean, full suite 1287/1287, npm audit clean, build succeeds, and manually smoke-tested end-to-end against an isolated temp database — confirmed correct per-network counts through the real HTTP endpoint (2 testnet / 1 mainnet contracts, 2 testnet / 1 mainnet entries).
AbdulmalikAlayande
added a commit
that referenced
this pull request
Aug 2, 2026
… the HTTP server (#540, #339) PR #540's registry.ts diff was, again, a complete standalone reimplementation (computeMetrics/formatPrometheus hand-rolling its own SQL queries and Prometheus serializer, bypassing prom-client and every already-registered metric entirely) — the exact same collision pattern as #543/#566/#591/#542. The issue's own step 1 ("extract the metric-computation logic... reusable by both the HTTP route and this command") was actually already satisfied by the real registry.ts — collectAllMetrics(db) + register.metrics() is exactly that shared logic, already used by the /metrics route. Wrote src/commands/metrics.ts fresh against the real registry: collectAllMetrics(db) then register.metrics() for the default Prometheus-text output, or register.getMetricsAsJSON() (prom-client's own structured format) for --json — no new serialization logic needed. Registered in src/cli/program.ts (the current entry point; src/index.ts is now just a thin wrapper around createProgram()). Added tests asserting the command's default output matches what collectAllMetrics + register.metrics() produce directly for the same DB state (the acceptance criteria's actual claim), that --json produces valid parseable JSON with the expected metric/label shape, and the empty-database case. Verified: tsc clean, full suite 1300/1300, npm audit clean, build succeeds, and manually smoke-tested both `sorokeep metrics` and `sorokeep metrics --json` against the compiled CLI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add fleet-scale Prometheus gauges that report how many contracts and entries sorokeep is watching, labeled by network.
What This Adds
New Files
src/observability/metrics/fleet.ts— Createssorokeep_contracts_trackedandsorokeep_entries_trackedgauges labeled by networksrc/observability/registry.ts— Shared Prometheus registry and collectAll functiontests/observability/fleet.test.ts— 10 tests covering registration, counting, deletions, zero-state, and mixed-network edge casesDependencies
prom-clientfor Prometheus metricsHow It Works
The
createFleetGauges(registry)function creates two gauges that:getAllContracts(db)for contract counts per networkgetEntriesForContract(db, id)for entry counts per networkreset()+set()pattern to handle label cardinality changesExample Output
Testing
All 10 tests pass:
npx vitest run tests/observability/fleet.test.tsAcceptance Criteria
Scope Notes
getAllContractsandgetEntriesForContractfrom db/repositories.ts