feat: make metrics and logs top-level verbs - #234
Merged
Merged
Conversation
outofcoffee
force-pushed
the
top-level-metrics-and-logs
branch
2 times, most recently
from
September 19, 2026 19:38
657d5d8 to
b535ade
Compare
outofcoffee
marked this pull request as ready for review
September 19, 2026 19:38
added 4 commits
September 19, 2026 22:02
metrics --cost and logs --source/--since/--instance report facts only a cloud environment has, and the shared stats shape has no room for them. Three optional capabilities join ProgressStarter and Keeper on fleet.Node: Coster prices a running session, SourceLogger queries a log store, and Versioner reports the release a node named with its last metrics reading. A kind with no answer does not implement one, so a caller reaches them by assertion and carries no branch on kind. remoteNode implements all three; daemonNode none. Metrics retains the instance type, uptime and version the stats reply carries and statsFromRemote drops, so Cost pays only for the price lookup and Version costs nothing — the reply that carried them is already paid for. metrics.Stats gains no field: a cloud-only fact on the shape every node answers with would leave a column every daemon reports empty.
Both take their target the way status and dashboard do — an environment, a fleet file, or the working directory's fleet.yaml — and fleet metrics and fleet logs are gone. The cloud-only flags reach their capability through the fan-out rather than through a caller that knows what kind a node is. PricedMetricsCall asks a node that implements Coster and leaves the rest as MetricsCall left them; QueriedLogsCall asks a node that implements SourceLogger and reads the rest by offset, so --source against a fleet of daemons returns their output rather than nothing. NodeResult carries what came back, so the renderers read one value per node instead of holding the nodes. MetricsCall now also carries the version, which fleet metrics never showed and remote metrics did — free, because the node answers it from the reading just taken.
…, close remaining test gaps runFleetMetricsWatch never returned when a single-node target (an --env watch) failed to read, unlike the old remote metrics --watch which exited on a fetch error; two tests calling it synchronously with no way to interrupt hung the whole test binary until the timeout killed it. It now ends the watch when the one node it is asked about could not be read, while a multi-node fleet still keeps drawing through a bad node. Also: - export internal/fleet's CloudWatch log fetch as FetchLogsFn so both its own tests and cmd/spinloop's can substitute it, closing the coverage gap on PricedMetricsCall, QueriedLogsCall and remoteNode.LogsMatching - delete cmd/spinloop/remote_logs.go: dead code left behind when logs moved to the top-level verb, its tests already superseded by fleet_logs.go's; rebuild its still-needed coverage (--source/--since/ --instance threading, the log-group/runner mapping) against the new path - restore --since's old 1h default on `spinloop logs`, dropped when the flag moved from `remote logs` - update stale test assertions (old key-value text, single-object JSON) left over from the metrics/logs/status merge - add a test for the six-command movedSubcommands signpost map, previously only exercised for "fleet harness"
Merges the change's five delta specs (fleet-client, remote-endpoint, remote-logs, remote-stats, remote-version-reporting) into the main specs and moves the change under openspec/changes/archive/. All 17 tasks were complete and the specs validate clean. While merging, corrected a few leftover authoring mistakes in the delta files themselves: a duplicated requirement title, a malformed example with --env given twice on one command line, two scenarios describing a default-environment fallback that a prior change already removed, and a scenario claiming `spinloop remote metrics` still displays figures where the same requirement's own next scenario says it fails with a signpost.
outofcoffee
force-pushed
the
top-level-metrics-and-logs
branch
from
September 19, 2026 21:05
0593b83 to
74c7c88
Compare
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.
The last of the read verbs, and the last five duplicate command spellings. After this, no verb is spelled twice. Completes #155 and #156.
In progress — capabilities landed, verbs next.
Summary
metricsandlogsbecome top-level commands, taking their target the waystatusanddashboarddo:--env <name>,--fleet <path>, or the working directory'sfleet.yaml.fleet metrics,fleet logs,remote status,remote metricsandremote logsare removed; each fails naming its replacement.Coster(--cost),SourceLogger(--source/--since/--instance),Versioner.ENVinstructions only, never to select a target — the rule theremotesubcommands already follow.Implementation details
Why these two were held back.
statusanddashboardmoved in #227 because they needed nothing new. These carry facts with no daemon counterpart, so the node contract has to say what a kind can and cannot answer.remote statusis in this change, not #227. It needs the Spinloop-ENVpath and a version a fan-out does not produce — andremote metricsneeds both too, so solving it once for all three beats twice.The rule when a flag meets a target that cannot answer it (design D2): the flag applies to the nodes that can, and leaves the rest as they read without it.
metrics --costover a fleet of daemons shows no cost and succeeds. This is what these views already do for every fact only one kind reports — the readiness mark, the version — and a flag is not a different case. The accepted cost is that a blank column does not explain itself, so the requirement obliges each verb's page to document which flags apply to which kinds.Three findings that shaped it:
statsFromRemotedrops four fields the reply already carries —Version,InstanceType,InstanceID,Environment. The first two are exactly what cost and version need.Metricsnow retains them on the node, soVersion()costs nothing andCost()pays only for the price lookup. That is whymetricscan carry the version whilestatuscould not: onstatusit lives in a different endpoint.metrics.Statsgains no field. Widening the shared reply with a cloud-onlyInstanceTypeand pricing it in the renderer would put a network call in a formatter and a blank field on every daemon. The node answers what did this cost, not what type are you.remote metricspath;Costermoves it behind the node rather than adding one, still behind--cost.