Retire the three orphaned IInfluxMetricsRepository methods (QueryCpuMetricsAsync, QueryMetricsAsync, StreamMetricsAsync) #601

New Issue

2026-07-14T22:12:46Z

spikerj commented

2026-07-14 22:12:46 +00:00

Follow-up from #577 (PR spikersoft-backend#295). Kept out of that PR to hold its scope to the model/project consolidation, since this one touches SpikerSoft.Data and its tests.

What

IInfluxMetricsRepository exposes five methods. After #577 deletes InfluxMetricsService, only two have a production caller:

Method	Status
`QueryMetricsBatchAsync`	live — the worker's only query path
`DiscoverReportingHostsAsync`	live — added by #571 for host-drift detection
`QueryCpuMetricsAsync`	orphaned — called only from `InfluxMetricsRepositoryTests`
`QueryMetricsAsync`	orphaned — zero callers, including tests
`StreamMetricsAsync`	orphaned — `InfluxDashboardHostedService` has its own private `StreamMetricsAsync` and never calls the repository's

Worth being precise about blame: these were already dead before #577. Their one caller was InfluxMetricsService, which was registered in no DI container, so nothing reached them at runtime even then. #577 just removed the fig leaf.

Why bother

The parameterised pair (QueryCpuMetricsAsync / QueryMetricsAsync) are the only methods that take timeRangeStart / windowPeriod and build an aggregateWindow(...) Flux query. Their presence is what made WindowPeriod / TimeRangeStart look like working config for however long they sat there — see #577, where an operator setting InfluxDashboard:WindowPeriod got a knob that bound cleanly and steered nothing. Leaving a plausible-looking-but-uncalled query API on the interface invites the same mistake again: the next person wires up QueryMetricsAsync assuming it is a supported path.

Suggested work

Delete the three methods from IInfluxMetricsRepository + InfluxMetricsRepository, and the BuildFluxQuery / BuildFluxQueryWithMeasurement helpers if they become unreachable.
Delete InfluxMetricsRepositoryTests' QueryCpuMetricsAsync cases (they test a method that no longer exists — not a write-off, the method is genuinely going away).
Confirm nothing in spikersoft-angular or the Api depends on the streaming shape first.

Open question for Joey: is the repository's StreamMetricsAsync (continuous-query streaming) intended future work, or was the worker's own poll-and-aggregate loop always the design? If streaming is on the roadmap, keep that one and mark it explicitly; if not, it goes with the other two. I'd rather ask than guess-delete a deliberate seam.

Follow-up from #577 (PR spikersoft-backend#295). Kept out of that PR to hold its scope to the model/project consolidation, since this one touches `SpikerSoft.Data` and its tests. ## What `IInfluxMetricsRepository` exposes five methods. After #577 deletes `InfluxMetricsService`, only two have a production caller: | Method | Status | |---|---| | `QueryMetricsBatchAsync` | **live** — the worker's only query path | | `DiscoverReportingHostsAsync` | **live** — added by #571 for host-drift detection | | `QueryCpuMetricsAsync` | orphaned — called only from `InfluxMetricsRepositoryTests` | | `QueryMetricsAsync` | orphaned — **zero** callers, including tests | | `StreamMetricsAsync` | orphaned — `InfluxDashboardHostedService` has its *own* private `StreamMetricsAsync` and never calls the repository's | Worth being precise about blame: these were **already dead before #577**. Their one caller was `InfluxMetricsService`, which was registered in no DI container, so nothing reached them at runtime even then. #577 just removed the fig leaf. ## Why bother The parameterised pair (`QueryCpuMetricsAsync` / `QueryMetricsAsync`) are the only methods that take `timeRangeStart` / `windowPeriod` and build an `aggregateWindow(...)` Flux query. Their presence is what made `WindowPeriod` / `TimeRangeStart` *look* like working config for however long they sat there — see #577, where an operator setting `InfluxDashboard:WindowPeriod` got a knob that bound cleanly and steered nothing. Leaving a plausible-looking-but-uncalled query API on the interface invites the same mistake again: the next person wires up `QueryMetricsAsync` assuming it is a supported path. ## Suggested work - Delete the three methods from `IInfluxMetricsRepository` + `InfluxMetricsRepository`, and the `BuildFluxQuery` / `BuildFluxQueryWithMeasurement` helpers if they become unreachable. - Delete `InfluxMetricsRepositoryTests`' `QueryCpuMetricsAsync` cases (they test a method that no longer exists — not a write-off, the method is genuinely going away). - Confirm nothing in `spikersoft-angular` or the Api depends on the streaming shape first. **Open question for Joey:** is the repository's `StreamMetricsAsync` (continuous-query streaming) intended future work, or was the worker's own poll-and-aggregate loop always the design? If streaming is on the roadmap, keep that one and mark it explicitly; if not, it goes with the other two. I'd rather ask than guess-delete a deliberate seam.

spikerj referenced this issue

2026-07-14 22:28:15 +00:00

[Backend][Metrics] SpikerSoft.Business.Metrics is inert — IInfluxMetricsService has no consumer, metric models are duplicated, namespace doesn't match the project, and InfluxDashboard re-implements the aggregation #577

spikerj commented

2026-07-29 17:05:02 +00:00

Audited against origin/master — NOT DONE, and I can now answer the question the ticket left open.

All three are still declared at SpikerSoft.Data/Repositories/IInfluxMetricsRepository.cs:38 / :59 / :112, implemented at InfluxMetricsRepository.cs:79 / :140 / :381. Caller analysis:

QueryCpuMetricsAsync — test-only. Exactly two callers, both in SpikerSoft.Data.Tests/Repositories/InfluxMetricsRepositoryTests.cs:48 and :71.
QueryMetricsAsync — zero callers of any kind, tests included. Confirmed by whole-repo grep.
StreamMetricsAsync — the repository's version has zero callers. SpikerSoft.EventHandlers.InfluxDashboard/Services/InfluxDashboardHostedService.cs:137 looks like a caller but invokes its own private method declared at :203 of the same file. Easy to misread, so worth recording.

Two dead helpers come with them, reachable only from the dead methods: BuildFluxQuery (InfluxMetricsRepository.cs:204, sole call site :89) and BuildFluxQueryWithMeasurement (:235, sole call site :151). So the retirement is larger than three methods.

The codebase already agrees with you — SpikerSoft.EventHandlers.InfluxDashboard/README.md:150-151 states all three have no production caller. The finding is documented; only the deletion is outstanding.

On the open question in the ticket — whether repo-level StreamMetricsAsync is intended future work — it's still unanswered after 15 days and zero comments. Worth noting that InfluxDashboardHostedService already has its own working streaming implementation at :203, so the repository version isn't a scaffold something is waiting on; it's a duplicate of a capability that already exists elsewhere. That makes deletion the low-risk call unless you specifically want streaming pushed down into the repository layer.

Remaining: delete the three methods, their implementations, the two helpers, and the two tests that exist only to exercise QueryCpuMetricsAsync — then update README.md:150-151.

One caution given this codebase's habits: those two tests are the same shape as the test(coverage) commits that pin bugs elsewhere (#809–#812) and the tests pinning legacy managers in #576. Coverage on a dead method is not evidence it's wanted.

Audited against `origin/master` — **NOT DONE, and I can now answer the question the ticket left open.** All three are still declared at `SpikerSoft.Data/Repositories/IInfluxMetricsRepository.cs:38` / `:59` / `:112`, implemented at `InfluxMetricsRepository.cs:79` / `:140` / `:381`. Caller analysis: - **`QueryCpuMetricsAsync`** — test-only. Exactly two callers, both in `SpikerSoft.Data.Tests/Repositories/InfluxMetricsRepositoryTests.cs:48` and `:71`. - **`QueryMetricsAsync`** — **zero callers of any kind**, tests included. Confirmed by whole-repo grep. - **`StreamMetricsAsync`** — the repository's version has **zero callers**. `SpikerSoft.EventHandlers.InfluxDashboard/Services/InfluxDashboardHostedService.cs:137` looks like a caller but invokes its *own* private method declared at `:203` of the same file. Easy to misread, so worth recording. **Two dead helpers come with them**, reachable only from the dead methods: `BuildFluxQuery` (`InfluxMetricsRepository.cs:204`, sole call site `:89`) and `BuildFluxQueryWithMeasurement` (`:235`, sole call site `:151`). So the retirement is larger than three methods. **The codebase already agrees with you** — `SpikerSoft.EventHandlers.InfluxDashboard/README.md:150-151` states all three have no production caller. The finding is documented; only the deletion is outstanding. **On the open question in the ticket** — whether repo-level `StreamMetricsAsync` is intended future work — it's still unanswered after 15 days and zero comments. Worth noting that `InfluxDashboardHostedService` already has its own working streaming implementation at `:203`, so the repository version isn't a scaffold something is waiting on; it's a duplicate of a capability that already exists elsewhere. That makes deletion the low-risk call unless you specifically want streaming pushed down into the repository layer. **Remaining:** delete the three methods, their implementations, the two helpers, and the two tests that exist only to exercise `QueryCpuMetricsAsync` — then update `README.md:150-151`. One caution given this codebase's habits: those two tests are the same shape as the `test(coverage)` commits that pin bugs elsewhere (#809–#812) and the tests pinning legacy managers in #576. Coverage on a dead method is not evidence it's wanted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#601