Metrics/TPS pipeline: code-review fixes + TPS unit tests#100
Merged
Conversation
- Derive TPS from the daily txCount already stored by txCountService
instead of re-fetching /metrics/txCount, eliminating a duplicate API
call per chain per cron cycle.
- Fold tpsService onto the shared metric infrastructure: cumulativeTxCount
now uses createMetricService, and tpsService no longer carries its own
copy of the RateLimiter / fetch-retry loop.
- Floor the TPS denominator (>= 1h) so the in-progress day bucket can't
spike on a tiny window or divide by zero at midnight; past full days are
unaffected.
- Cron now inspects each metric update's {success} result and reports the
chain as failed instead of always logging success.
- Run the independent per-chain metric updates concurrently; derive TPS
after txCount is fetched.
- /health/dependencies now reports a 5xx upstream as 'degraded' (503)
rather than 'reachable'.
- initWeeklyData.js calls updateWeeklyData() (the prior method name no
longer existed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers tpsService.updateTpsData against mocked models: full-day division, the in-progress day denominator floor (no spike), skipping future timestamps and non-numeric values, the empty no-op result, and the failure-result-not-throw path. Uses the real service (does not pull in the global tpsService mock from setup.js). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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
Addresses the findings from the high-effort code review of the metrics/TPS pipeline, plus unit tests for the new TPS derivation.
Fixes
txCountalready stored bytxCountServiceinstead of re-fetching/metrics/txCountper chain per cron cycle.cumulativeTxCountnow usescreateMetricService;tpsServiceno longer carries its own copy of theRateLimiter/ fetch-retry loop (~400 lines removed).{success}result and reports the chain as failed instead of always logging success./health/dependenciesnow reports a 5xx upstream asdegraded(503) rather thanreachable.initWeeklyData.jscallsupdateWeeklyData()(the prior method name no longer existed).Tests
tests/tpsDerivation.test.jsexercises the realtpsService.updateTpsDataagainst mocked models: full-day division, the in-progress denominator floor (spike guard), skipping future/non-numeric rows, the empty no-op result, and the failure-result-not-throw path.Testing
🤖 Generated with Claude Code