From 923b4ab1ff1f19af56e191a3df4d9b8be251bb5c Mon Sep 17 00:00:00 2001 From: spinloop-agent Date: Fri, 18 Sep 2026 15:16:54 +0100 Subject: [PATCH 1/4] feat(fleet): add the cloud-only node capabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/fleet/capabilities_test.go | 170 +++++++++++ internal/fleet/node.go | 65 +++++ internal/fleet/remote_node.go | 86 +++++- .../top-level-metrics-and-logs/.openspec.yaml | 2 + .../top-level-metrics-and-logs/design.md | 125 ++++++++ .../top-level-metrics-and-logs/proposal.md | 95 +++++++ .../specs/fleet-client/spec.md | 147 ++++++++++ .../specs/remote-endpoint/spec.md | 216 ++++++++++++++ .../specs/remote-logs/spec.md | 269 ++++++++++++++++++ .../specs/remote-stats/spec.md | 242 ++++++++++++++++ .../specs/remote-version-reporting/spec.md | 53 ++++ .../top-level-metrics-and-logs/tasks.md | 68 +++++ 12 files changed, 1536 insertions(+), 2 deletions(-) create mode 100644 internal/fleet/capabilities_test.go create mode 100644 openspec/changes/top-level-metrics-and-logs/.openspec.yaml create mode 100644 openspec/changes/top-level-metrics-and-logs/design.md create mode 100644 openspec/changes/top-level-metrics-and-logs/proposal.md create mode 100644 openspec/changes/top-level-metrics-and-logs/specs/fleet-client/spec.md create mode 100644 openspec/changes/top-level-metrics-and-logs/specs/remote-endpoint/spec.md create mode 100644 openspec/changes/top-level-metrics-and-logs/specs/remote-logs/spec.md create mode 100644 openspec/changes/top-level-metrics-and-logs/specs/remote-stats/spec.md create mode 100644 openspec/changes/top-level-metrics-and-logs/specs/remote-version-reporting/spec.md create mode 100644 openspec/changes/top-level-metrics-and-logs/tasks.md diff --git a/internal/fleet/capabilities_test.go b/internal/fleet/capabilities_test.go new file mode 100644 index 00000000..2c8a8abe --- /dev/null +++ b/internal/fleet/capabilities_test.go @@ -0,0 +1,170 @@ +package fleet + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" +) + +// countingStatsServer answers every call with body and counts the calls, so a +// test can assert that reading a retained fact costs none. +func countingStatsServer(t *testing.T, body string, calls *int) string { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *calls++ + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + return srv.URL +} + +// registerStatsEnv registers an environment whose stats call is the given URL, +// which registerRemoteEnv does not set — the capabilities answer from a stats +// reading, so a test of them needs one. +func registerStatsEnv(t *testing.T, name, url string) { + t.Helper() + home := t.TempDir() + t.Setenv("SPINLOOP_CONFIG_DIR", home) + dir := filepath.Join(home, "remotes", name) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + body := fmt.Sprintf( + `{"start_url":%q,"stop_url":%q,"stats_url":%q,"region":"us-east-1","environment":%q}`, + url, url, url, name) + if err := os.WriteFile(filepath.Join(dir, "remote.json"), []byte(body), 0o600); err != nil { + t.Fatal(err) + } +} + +// A daemon node implements none of the three: its kind has no answer for any +// of them, so it does not pretend to. +func TestDaemonNodeImplementsNoCloudCapability(t *testing.T) { + cfg := &Config{Nodes: []NodeConfig{{Name: "box", Host: "127.0.0.1", Port: 1}}} + node, err := cfg.NewNode(cfg.Nodes[0]) + if err != nil { + t.Fatal(err) + } + if _, ok := node.(Coster); ok { + t.Error("a daemon node should not implement Coster: it has no instance type or region") + } + if _, ok := node.(SourceLogger); ok { + t.Error("a daemon node should not implement SourceLogger: its log is a byte offset") + } + if _, ok := node.(Versioner); ok { + t.Error("a daemon node should not implement Versioner: its status reply carries the version") + } +} + +// A cloud node implements all three, so a caller reaches them by assertion +// rather than by asking what kind it is. +func TestRemoteNodeImplementsTheCloudCapabilities(t *testing.T) { + stubAWSCreds(t) + up := remoteControlServer(t, `{"state":"running"}`, http.StatusOK) + registerRemoteEnv(t, "prod", up.URL, up.URL) + cfg, err := ForEnvironment("prod") + if err != nil { + t.Fatal(err) + } + node, err := cfg.NewNode(cfg.Nodes[0]) + if err != nil { + t.Fatal(err) + } + if _, ok := node.(Coster); !ok { + t.Error("a cloud node should implement Coster") + } + if _, ok := node.(SourceLogger); !ok { + t.Error("a cloud node should implement SourceLogger") + } + if _, ok := node.(Versioner); !ok { + t.Error("a cloud node should implement Versioner") + } +} + +// The version comes from the reading already taken, not from a second call. +func TestRemoteNodeVersionComesFromTheMetricsReading(t *testing.T) { + stubAWSCreds(t) + calls := 0 + srv := countingStatsServer(t, `{"state":"running","version":"1.40.0","instanceType":"g6e.xlarge","uptimeSeconds":7200}`, &calls) + registerStatsEnv(t, "prod", srv) + cfg, err := ForEnvironment("prod") + if err != nil { + t.Fatal(err) + } + node, err := cfg.NewNode(cfg.Nodes[0]) + if err != nil { + t.Fatal(err) + } + + v, _ := node.(Versioner) + if got := v.Version(); got != "" { + t.Errorf("version before any reading = %q, want empty", got) + } + if _, err := node.Metrics(context.Background()); err != nil { + t.Fatal(err) + } + before := calls + if got := v.Version(); got != "1.40.0" { + t.Errorf("version = %q, want the reading's", got) + } + if calls != before { + t.Errorf("reading the version cost %d extra call(s), want none", calls-before) + } +} + +// A node with nothing to price reports no cost rather than a zero one: "$0.00" +// claims it cost nothing, which is a different statement. +func TestRemoteNodeCostIsUnreportedWithoutAReading(t *testing.T) { + stubAWSCreds(t) + up := remoteControlServer(t, `{"state":"stopped"}`, http.StatusOK) + registerRemoteEnv(t, "prod", up.URL, up.URL) + cfg, err := ForEnvironment("prod") + if err != nil { + t.Fatal(err) + } + node, err := cfg.NewNode(cfg.Nodes[0]) + if err != nil { + t.Fatal(err) + } + c, ok := node.(Coster) + if !ok { + t.Fatal("a cloud node should implement Coster") + } + cost, err := c.Cost(context.Background()) + if err != nil { + t.Fatalf("an unpriceable node is not an error: %v", err) + } + if cost.Reported() { + t.Errorf("cost = %+v, want none reported before a reading", cost) + } +} + +// Cost.Reported tells a caller whether there is a figure at all, so a renderer +// never has to decide what a zero means. +func TestCostReported(t *testing.T) { + if (Cost{}).Reported() { + t.Error("the zero Cost reports nothing") + } + if (Cost{SoFar: 0, PerHour: 1.25}).Reported() != true { + t.Error("a rate with no elapsed time is still a figure") + } +} + +// A query names its own window, so nothing is retained between calls the way +// a follow's cursor is. +func TestLogQueryDefaults(t *testing.T) { + q := LogQuery{} + if q.Source != "" || q.Limit != 0 || q.Since != 0 || q.Instance != "" { + t.Errorf("the zero LogQuery narrows nothing, got %+v", q) + } + q = LogQuery{Source: "boot", Since: time.Hour, Instance: "i-1", Limit: 10} + if q.Source != "boot" || q.Since != time.Hour || q.Instance != "i-1" || q.Limit != 10 { + t.Errorf("LogQuery does not carry what it was given: %+v", q) + } +} diff --git a/internal/fleet/node.go b/internal/fleet/node.go index 3d765772..c9f13730 100644 --- a/internal/fleet/node.go +++ b/internal/fleet/node.go @@ -69,6 +69,71 @@ type Keeper interface { Keep(ctx context.Context, d time.Duration) (string, error) } +// Coster is an optional node capability: a node that can price the time it has +// been running. Only a cloud environment can — the price comes from the +// instance type it launched as and the region it launched in, neither of which +// a machine someone already owns has an answer for — so a daemon node does not +// implement it. A caller offering a cost (metrics --cost) asserts for it and +// renders the node as it would without the flag when it is absent. +// +// The lookup crosses the network, so it is made only when a caller asks. A +// price that cannot be fetched is not an error: the node reports no cost, the +// same as a node that cannot be priced at all, because a missing price and an +// unpriceable node read the same in a table. +type Coster interface { + Cost(ctx context.Context) (Cost, error) +} + +// Cost is what a node has spent on the session it is running, and the rate it +// is spending at. Zero means no figure is available — the instance is not +// running, its type is unknown, or the price lookup did not complete. +type Cost struct { + // SoFar is the estimated spend on the current running session. + SoFar float64 + // PerHour is the on-demand rate the estimate was computed from. + PerHour float64 +} + +// Reported says whether there is a figure to show. A caller renders nothing +// rather than a zero: "$0.00" claims a node cost nothing, which is a different +// statement from having no price for it. +func (c Cost) Reported() bool { return c.PerHour > 0 } + +// SourceLogger is an optional node capability: a node whose log is a store that +// can be queried — by which log to read, how far back, and which instance +// produced it. Only a cloud environment has one; a daemon's log is a byte +// offset into one file on one machine, where none of the three narrows +// anything. A caller offering those flags asserts for it and reads the node +// through Logs when it is absent. +type SourceLogger interface { + LogsMatching(ctx context.Context, q LogQuery) (daemon.LogsResponse, error) +} + +// LogQuery is what a caller can narrow a queryable log by. A zero field means +// "do not narrow by this". +type LogQuery struct { + // Source names which of the node's logs to read — its engine's output, its + // boot record, or both. + Source string + // Since bounds how far back to read. + Since time.Duration + // Instance restricts the read to one instance id, for a node whose store + // holds the output of more than one. + Instance string + // Limit caps the events returned, keeping the most recent. + Limit int +} + +// Versioner is an optional node capability: a node that reports the spinloop +// release it is running somewhere other than its status reply. A daemon node +// carries its version in that reply, so it does not implement this; a cloud +// environment's arrives with its metrics, which is a different call. A caller +// with a metrics reading in hand asserts for it rather than making a second +// call of its own. +type Versioner interface { + Version() string +} + // Node is one member of the fleet. Only daemonNode implements it today; the // interface exists so a remote-environment kind (an `spinloop remote` // environment read through its stats Lambda, which already yields diff --git a/internal/fleet/remote_node.go b/internal/fleet/remote_node.go index 9887146d..1da6ae32 100644 --- a/internal/fleet/remote_node.go +++ b/internal/fleet/remote_node.go @@ -6,6 +6,7 @@ import ( "net/url" "strconv" "strings" + "sync" "time" "github.com/spinloop-ai/spinloop/internal/daemon" @@ -33,6 +34,18 @@ type remoteNode struct { // re-asks a little behind the newest event already seen and this // suppresses what the overlap re-reads, by event id. logs *remote.FollowCursor + + // mu guards the facts Metrics retains for the capabilities to answer from. + // A board refreshing several nodes reads and writes these from different + // goroutines, so they are not left bare. + mu sync.Mutex + // instanceType, uptime and version are the last metrics reading's answers + // to questions the shared stats shape has no room for. Cost and Version + // read them rather than calling the control plane again — the reply that + // carried them has already been paid for. + instanceType string + uptime int + version string } // NewRemoteNode builds the live node for a named remote environment. The config @@ -63,9 +76,75 @@ func (n *remoteNode) Metrics(ctx context.Context) (metrics.Stats, error) { if err != nil { return metrics.Stats{}, err } + // The reply carries three facts the shared stats shape has no room for. + // Retaining them here is what lets Cost and Version answer without a + // second call for a reading already in hand. + n.mu.Lock() + n.instanceType, n.uptime, n.version = resp.InstanceType, resp.UptimeSeconds, resp.Version + n.mu.Unlock() return statsFromRemote(*resp), nil } +// Cost prices the session this environment has been running, from the instance +// type it launched as and the region it launched in. Both come from the last +// metrics reading, so a caller that has already taken one pays only for the +// price lookup. +// +// A reading not yet taken, an instance that is not running, or a price the +// Price List API does not return all yield the zero Cost and no error: a node +// with no price to show reads the same however it came to have none. +func (n *remoteNode) Cost(ctx context.Context) (Cost, error) { + n.mu.Lock() + instanceType, uptime := n.instanceType, n.uptime + n.mu.Unlock() + if instanceType == "" || uptime <= 0 { + return Cost{}, nil + } + price, err := remote.GetOnDemandPrice(ctx, n.cfg.Region, instanceType) + if err != nil || price <= 0 { + return Cost{}, nil + } + return Cost{SoFar: float64(uptime) / 3600.0 * price, PerHour: price}, nil +} + +// Version is the spinloop release this environment's daemon reported with its +// last metrics reading. Empty before one has been taken, or where the control +// plane could not reach the daemon — the same absence, reported the same way. +func (n *remoteNode) Version() string { + n.mu.Lock() + defer n.mu.Unlock() + return n.version +} + +// LogsMatching reads this environment's log store, narrowed by what the caller +// asked for. Unlike Logs it is a query rather than a cursor: the caller states +// the window it wants, so nothing is retained between calls. +func (n *remoteNode) LogsMatching(ctx context.Context, q LogQuery) (daemon.LogsResponse, error) { + source := q.Source + if source == "" { + source = remote.LogSourceEngine + } + limit := q.Limit + if limit <= 0 { + limit = remoteEngineTail + } + rq := remote.LogQuery{ + Environment: n.cfg.Environment, + Source: source, + Limit: limit, + Instance: q.Instance, + } + if q.Since > 0 { + rq.Start = time.Now().Add(-q.Since) + } + res, err := remote.FetchLogs(ctx, n.cfg, rq) + if err != nil { + return daemon.LogsResponse{}, err + } + // A query is not a follow, so every event it returns is fresh to it. + return logsFromRemote(res.Events, len(res.Events) == 0), nil +} + func (n *remoteNode) Start(ctx context.Context) (daemon.StatusResponse, error) { return n.StartWithProgress(ctx, func(StartPhase) {}) } @@ -222,8 +301,11 @@ func statusFromRemote(resp remote.Response) daemon.StatusResponse { // statsFromRemote maps the stats Lambda's reply onto the shared stats shape. The // per-stat fields already alias the metrics types the collector produces, so the -// mapping is a field-for-field copy; the reply's version and instance facts have -// no home on the node's stats and are left to the remote view. +// mapping is a field-for-field copy. The reply's version and instance facts are +// deliberately not among them: they describe a cloud environment and nothing +// else, so putting them on the shape every node answers with would leave a +// field every daemon reports empty. Metrics retains them on the node instead, +// where Cost and Version answer from them. func statsFromRemote(resp remote.StatsResponse) metrics.Stats { return metrics.Stats{ State: resp.State, diff --git a/openspec/changes/top-level-metrics-and-logs/.openspec.yaml b/openspec/changes/top-level-metrics-and-logs/.openspec.yaml new file mode 100644 index 00000000..96db9a43 --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-15 diff --git a/openspec/changes/top-level-metrics-and-logs/design.md b/openspec/changes/top-level-metrics-and-logs/design.md new file mode 100644 index 00000000..9d683303 --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/design.md @@ -0,0 +1,125 @@ +## Context + +See proposal.md — Why. The implementation-relevant current state: + +- `resolveFleetTarget` (`cmd/spinloop/target.go`) already turns `--env` / + `--fleet` / the working directory into the `*fleet.Config` to act on, and + `status` and `dashboard` already use it. These verbs are two more callers. +- `fleet.Node` already carries two optional capabilities — `ProgressStarter` + and `Keeper` — each asserted by its caller with a fallback. The pattern, the + doc-comment shape and the "a caller that can offer it asserts for it" rule + are established. +- `statsFromRemote` (`internal/fleet/remote_node.go`) maps the cloud stats + reply onto `metrics.Stats` and **drops four fields it carries**: + `Version`, `InstanceType`, `InstanceID` and `Environment`. The first two are + exactly what the cost and version want. +- `remote metrics --cost` computes from `resp.InstanceType`, `resp.UptimeSeconds` + and `cfg.Region`, looking the price up through the AWS Price List API and + silently omitting the line when the lookup fails. +- `remote logs` queries a log store: `--source engine|boot|all`, `--since`, + `--instance`, over `remote.LogQuery`. `fleet logs` reads a byte offset into + one file. The two share `--follow`, `--limit` and `--format`. +- Every `remote` subcommand applies a Spinloop's `ENV` instructions and the + `.env` beside it before any control work (`resolveRemoteConfig`), so AWS + credentials and `SPINLOOP_REMOTE_*` values can come from a Spinloop. The + top-level verbs have no such path today. + +## Goals / Non-Goals + +**Goals:** + +- One command reads an engine's metrics, and one reads its logs, whatever kind + of thing is running it. +- A fact only one node kind has stays available, without a caller branching on + kind. +- Nothing an operator could get from the removed commands is unreachable. + +**Non-Goals:** + +- No `remote` → `cloud` rename, no `kind: cloud`, no `-f` reversal. A rename + across every example and doc; its own change. +- No change to what the stats reply carries, to the log store query, or to the + daemon's log endpoint. The capabilities expose what already exists. +- No new output format, and no change to the existing four. + +## Decisions + +**D1: Three capabilities, each a node-kind ability rather than a reply field.** + +`Coster`, `SourceLogger` and `Versioner` join `ProgressStarter` and `Keeper` on +`fleet.Node`. A kind that cannot answer does not implement one; a caller that +offers the flag asserts for it and leaves the node as it reads without it. + +The alternative — widening `metrics.Stats` with an `InstanceType` and letting +the renderer price it — was rejected twice over. It puts a cloud-only field on +the shared reply that every daemon leaves empty, and it puts the price lookup +(a network call to the AWS Price List API) in a renderer. `Coster` keeps both +where they belong: the node knows its type and region, so it answers *what did +this cost*, not *what type are you*. + +**D2: A flag whose target cannot answer it renders blank, and does not fail.** + +`metrics --cost` over 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 does not make it a different case. A +fleet with no priceable node is a fleet with no cost to show, not a malformed +command line. + +Alternatives: refuse up front when no node can answer (rejected — it makes a +flag's validity depend on the target's contents, and a mixed fleet still needs +this rule anyway); warn on stderr (rejected as ceremony for a column that is +blank for a documented reason). + +The cost of D2 is that a blank column does not explain itself, so the +requirement obliges each verb's page to say which flags apply to which kinds. + +**D3: `statsFromRemote` stops dropping the version and instance type.** + +Both are already in the reply the cloud node receives. `Versioner` reads the +version from the node's retained reply rather than making a call, so the +version costs nothing on the metrics path — unlike on `status`, where the +version lives in a different endpoint and was what made `remote status` hard to +move in the first place. + +`metrics.Stats` gains no field: the node keeps what it needs to answer its own +capabilities, which is D1 applied consistently. + +**D4: The Spinloop `ENV` path moves to the verbs, as an argument that selects +nothing.** + +A Spinloop given to a read verb is read for its `ENV` instructions and adjacent +`.env`, never to select a target — the rule the `remote` subcommands already +follow. Without it, an operator whose AWS credentials or `SPINLOOP_REMOTE_*` +values come from a Spinloop loses them when the command they used is removed. + +This is the piece that made the last change stop short of `remote status`. +Implementing it once here serves all three removed reads. + +**D5: Five signposts, through the existing mechanism.** + +`fleet metrics`, `fleet logs`, `remote status`, `remote metrics` and +`remote logs` go in `movedSubcommands`, which `groupArgs` already reads. The +`remote` group gains the same `Args: groupArgs` the fleet group has. + +## Risks / Trade-offs + +- [A blank column reads as "zero" rather than "not applicable"] → D2's + accepted cost, mitigated by the documentation the requirement obliges. The + alternative rules were worse: one makes a flag's validity depend on what the + fleet happens to contain, the other prints ceremony on every run. +- [Three capabilities at once is a lot of new interface] → they are three + instances of a pattern already in the tree twice, each with exactly one + implementor and one caller. The shape is not new; the count is. +- [The price lookup is a network call inside a fan-out] → it already is one, + on the `remote metrics` path; `Coster` moves it behind the node rather than + adding it. It stays behind `--cost`, so a run that does not ask pays nothing. +- [Removing five commands at once is a wide break] → each names its + replacement, and they are the last five duplicate spellings; leaving any + behind would mean a second change over the same files. + +## Migration Plan + +`fleet metrics` → `metrics`. `fleet logs` → `logs`. `remote metrics` → +`metrics --env `. `remote logs` → `logs --env `. `remote status` → +`status --env `. Each old spelling fails naming its replacement. Nothing +is persisted or transmitted differently; rollback is a revert. diff --git a/openspec/changes/top-level-metrics-and-logs/proposal.md b/openspec/changes/top-level-metrics-and-logs/proposal.md new file mode 100644 index 00000000..f9e5df9c --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/proposal.md @@ -0,0 +1,95 @@ +## Why + +`status` and `dashboard` are top-level verbs serving every target kind. +`metrics` and `logs` are not, and they are the two the last change deliberately +left behind: each carries facts only a cloud environment has, with no daemon +counterpart, so moving them needs the node contract to say what a node kind +can and cannot answer. + +- `remote metrics --cost` prices an instance from its type and region. A + daemon has neither, and `statsFromRemote` drops the instance type on the way + into the shared stats, so the figure cannot be computed through a fan-out + today. +- `remote logs --source engine|boot|all`, `--since` and `--instance` query a + log store. A daemon's log is a byte offset into one file; none of the three + means anything to it. + +Two more facts go with them, because the same commands need them and no +fan-out produces them: an environment's **version**, which the stats reply +carries and the shared stats drop, and the **Spinloop `ENV`** path, which every +`remote` subcommand applies before resolving so that credentials and URLs can +come from a Spinloop's own instructions. + +`remote status` is in this change too. It was held back from the last one for +exactly these reasons — a second call for the version, the `ENV` path, the +address and keep figures — and all three read verbs need the same answers. +Solving it once for the three beats solving it twice. + +## What Changes + +- `metrics` and `logs` become top-level commands, taking their target the way + `status` and `dashboard` do: `--env `, `--fleet `, or the + working directory's `fleet.yaml`. +- The node contract gains optional capabilities, asserted by the caller the + way `ProgressStarter` and `Keeper` already are. A node kind that does not + implement one is not an error — the flag that needs it reports that this + target cannot answer it, naming the kinds that can: + - **pricing** — what a node has cost so far, and its hourly rate, for + `metrics --cost`. + - **log queries** — reading a node's log by source, time window and + instance, for `logs --source`, `--since` and `--instance`. + - **version** — the spinloop release a node is running, where the node + reports one outside its status reply. +- **BREAKING** `fleet metrics`, `fleet logs`, `remote metrics`, `remote logs` + and `remote status` are removed. Each fails naming the command that replaced + it, the way a moved command already does. +- A Spinloop given to a read verb is read for its `ENV` instructions and the + `.env` beside it, never to select a target — the rule the `remote` + subcommands already follow, now stated for the verbs that replace them. +- An environment's endpoint address is not a column of any of these views. It + is `spinloop remote env`, which prints it eval-safe, and what routing already + reads. + +After this, `remote` holds only what has no fleet counterpart — `bootstrap`, +`auth`, `bake`, `start`, `pause`, `restart`, `stop`, `deploy`, `seed`, `env`, +`ls`, `keep` — and `fleet` holds `route`, `start`, `stop` and `deploy`. No verb +is spelled twice. + +## Capabilities + +### New Capabilities + +(None — every behaviour change lands in an existing capability.) + +### Modified Capabilities + +- `fleet-client`: fleet metrics and fleet logs become top-level verbs serving + every target kind; the node contract gains the optional pricing, log-query + and version capabilities, and a flag whose capability a target lacks says so + rather than failing or silently omitting. +- `remote-stats`: `remote metrics` is removed; what it reported — including + the cost estimate and the version — is reported by the top-level `metrics` + against the same environment. +- `remote-logs`: `remote logs` is removed; its source, since and instance + narrowing become capabilities of the top-level `logs`. +- `remote-endpoint`: the `remote` group no longer has `status`, `metrics` or + `logs` subcommands, and a Spinloop given to a read verb is read for its + `ENV` only, as it is for the `remote` subcommands that remain. +- `remote-version-reporting`: an environment's version is read through the + top-level verbs rather than `remote status` and `remote metrics`. + +## Impact + +- `internal/fleet`: the three optional interfaces beside `ProgressStarter` and + `Keeper`; `remoteNode` implements all three, `daemonNode` the version one + only where its status already carries it. `statsFromRemote` keeps the + instance type and version it currently drops. +- `cmd/spinloop`: `metrics.go` and `logs.go` hold the verbs; `fleet.go`, + `fleet_logs.go`, `remote.go` and `remote_logs.go` lose their command + wrappers and keep their renderers. +- `cmd/spinloop/commands.go`: the two verbs registered, four fleet/remote + subcommands unregistered, five moved spellings signposted. +- `docs/commands/`: pages for the two verbs; `fleet.md` and `remote.md` point + at them; every example and CI script updated. +- No change to the fleet file format, the environments registry, the control + plane, the daemon API, or the gateway. diff --git a/openspec/changes/top-level-metrics-and-logs/specs/fleet-client/spec.md b/openspec/changes/top-level-metrics-and-logs/specs/fleet-client/spec.md new file mode 100644 index 00000000..2d9c66b8 --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/specs/fleet-client/spec.md @@ -0,0 +1,147 @@ +## MODIFIED Requirements + +### Requirement: Fleet metrics + +`spinloop metrics` SHALL query every node in the resolved target and render +each node's engine and system metrics using the same bar, gauge, table and +json formats, selected by `--format`. +Unreachable nodes SHALL be reported as in status rather than omitted. The +command SHALL support a `--watch`/`-w` mode that refreshes on an interval, +clearing and redrawing the screen in place with no scrollback accumulation, +and exiting cleanly on interrupt. + +#### Scenario: Gauge format per node + +- **WHEN** `spinloop metrics` runs without `--format` +- **THEN** each reachable node's resource series render in gauge format under + its name + +#### Scenario: Bar format per node + +- **WHEN** `spinloop fleet metrics --format=bar` runs +- **THEN** each reachable node's metrics render in bar format under its name + +#### Scenario: JSON aggregates the fleet + +- **WHEN** `spinloop fleet metrics --format=json` runs +- **THEN** the output is valid JSON keyed or labelled by node, including + unreachable nodes with their error + +#### Scenario: Watch redraws in place + +- **WHEN** `spinloop fleet metrics --watch` runs +- **THEN** each refresh clears the screen and redraws the fleet, and Ctrl+C + exits cleanly + +The target SHALL resolve as it does for every other command that acts on a +fleet — a named environment, a named fleet file, or the working directory's +fleet file — so one environment and a fleet holding it are read by the one +command. + +#### Scenario: A named environment is read by the same command + +- **WHEN** `spinloop metrics --env prod` runs +- **THEN** that environment's metrics are rendered with no fleet file + required, in the same format a fleet file naming it would produce + +#### Scenario: The fleet-scoped spelling names its replacement + +- **WHEN** the operator runs `spinloop fleet metrics` +- **THEN** it fails naming `spinloop metrics` as the command that replaced it + +### Requirement: Fleet logs + +`spinloop logs` SHALL read the engine output of the target's nodes, so "what did that engine say?" is answerable from the same +place as "what is it doing?" — without shell access to any machine. With no node +named it SHALL read every node in the fleet; naming a node SHALL restrict it to +that one. Nodes SHALL be read concurrently, so the command's latency is that of +the slowest reachable node rather than their sum. + +The target SHALL resolve as it does for every other command that acts on a +fleet. The fleet file SHALL be named by the long form `--fleet` only: unlike +the other commands that take one, `logs` SHALL NOT accept `-f` for it, because +`-f` is that command's `--follow` short form and a flag cannot carry two +meanings on one command line. + +#### Scenario: Reading the whole fleet + +- **WHEN** the operator runs `spinloop logs` with no node named +- **THEN** every node's engine output is read and printed + +#### Scenario: Reading one node + +- **WHEN** the operator names a node +- **THEN** only that node's output is printed, and the other nodes are not + contacted + +#### Scenario: A crashed node's output is readable + +- **WHEN** a node's engine has crashed, as `spinloop status` reports +- **THEN** its output up to the crash is printed, explaining what status can + only report + +#### Scenario: The fleet file has no short flag here + +- **WHEN** the operator runs `spinloop fleet logs -f --fleet ./cluster.yaml` +- **THEN** the flag is accepted as follow mode plus the fleet file, with `-f` + not treated as a fleet-file flag + +#### Scenario: A named environment is read by the same command + +- **WHEN** `spinloop logs --env prod` runs +- **THEN** that environment's engine output is read with no fleet file + required + +#### Scenario: The fleet-scoped spelling names its replacement + +- **WHEN** the operator runs `spinloop fleet logs` +- **THEN** it fails naming `spinloop logs` as the command that replaced it + +## ADDED Requirements + +### Requirement: A node answers only what its kind can + +A node kind SHALL answer the operations its kind supports and no more. Where a +read verb offers a flag whose answer only some node kinds carry — an instance's +price, a log store's source, time window or instance id — the flag SHALL apply +to the nodes that can answer it and SHALL leave the rest as they read without +it. + +A target holding no node that can answer SHALL NOT be an error. Rendering a +column blank for a node that has no such fact is what these views already do +for every fact only one kind reports, and a flag is not a different case: a +fleet of daemons asked for a cost is a fleet with no cost to show, not a +malformed command. + +The capability SHALL be a property of the node kind, asserted by the caller, +not a field on the shared reply. A kind that cannot answer SHALL simply not +implement it, so adding a kind that can requires no change to the callers and +adding one that cannot requires no exception. + +Which flags apply to which kinds SHALL be documented on each verb's page, since +a blank column is not self-explaining. + +#### Scenario: A priced node and an unpriced one in one table + +- **WHEN** `spinloop metrics --cost` runs against a target holding both a cloud + environment and a daemon node +- **THEN** the environment's row carries its cost and the daemon's does not, + and the command succeeds + +#### Scenario: No node can answer, and that is not an error + +- **WHEN** `spinloop metrics --cost` runs against a fleet of daemon nodes only +- **THEN** no cost is shown for any node and the command succeeds + +#### Scenario: A log query narrows the nodes that support it + +- **WHEN** `spinloop logs --source boot` runs against a target holding both a + cloud environment and a daemon node +- **THEN** the environment's boot log is read, and the daemon's output is read + as it would be without the flag + +#### Scenario: A kind that cannot answer implements nothing + +- **WHEN** a node kind has no answer for a capability +- **THEN** it does not implement that capability, and no caller carries a + branch naming that kind diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-endpoint/spec.md b/openspec/changes/top-level-metrics-and-logs/specs/remote-endpoint/spec.md new file mode 100644 index 00000000..a9bce153 --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/specs/remote-endpoint/spec.md @@ -0,0 +1,216 @@ +## MODIFIED Requirements + +### Requirement: Remote command group + +The system SHALL provide a `remote` command group with the subcommands +`bootstrap`, `bake`, `auth`, `start`, `stop`, `restart`, `deploy`, +`ls`, and `keep`. `start`, `stop`, `restart` and `deploy` each take an +optional Spinloop path: +`start` SHALL boot the endpoint and block until it is serving, then perform a +quick TCP probe of the inference endpoint — if the probe fails, a warning is +printed to stderr explaining the network mismatch (see the Remote Start Probe +specification) — and finally print the base URL and API key as shell exports; +`start` SHALL also accept a `--keep DURATION` flag that sets the instance +retention deadline to `now + DURATION`, preventing the idle sweep from +terminating it before that time (see the Remote Keep specification); +`stop` SHALL stop it immediately rather than waiting for its idle timer; +`restart` SHALL stop the endpoint in the manner of a pause — without +terminating it, so its boot disk, its weights and its stable address are +preserved — and SHALL immediately start it again, blocking until it is serving +and reporting progress as `start` does (see the Reporting a start in progress +specification); `restart` SHALL accept a `--force` flag with a `-F` short form +that, when set, performs the stop without first asking the engine to shut down +(see the Endpoint Lifecycle specification for forced stops); +`keep` SHALL set the `Retain-Until` tag on the environment's instance for the +given duration, without starting or stopping the instance (see the Remote Keep +specification); `deploy` SHALL set what the endpoint serves. `ls` SHALL list the registered remote environments +(see the Remote Environments specification). `bootstrap` SHALL stand up the +account-level AWS control plane (once per account) by obtaining and driving the +CDK project, and takes its own flags rather than a Spinloop path (see the +Endpoint Provisioning specification). `bake` SHALL start an AMI bake for each +runner named, and takes runner names rather than a Spinloop path (see the +Endpoint Provisioning specification). `auth` SHALL store, report, and clear the +long-lived control-plane credential, and takes its own flags rather than a +Spinloop path (see the Remote Auth specification). An unrecognised subcommand +SHALL fail naming the accepted ones. + +#### Scenario: Starting the endpoint + +- **WHEN** the user runs `spinloop remote start` and the endpoint reports ready +- **THEN** the base URL and API key are printed as `export` lines + +#### Scenario: Starting warns when the network is not admitted + +- **WHEN** the user runs `spinloop remote start` and the endpoint reports ready + but the TCP probe to the inference port fails +- **THEN** a warning is printed to stderr with a remediation command, and the + command still exits 0 + +#### Scenario: Starting with a keep flag + +- **WHEN** the user runs `spinloop remote start --keep 4h` and the endpoint reports ready +- **THEN** the base URL and API key are printed as `export` lines, and the + instance retention deadline is set to 4 hours from now + +#### Scenario: Waiting through a cold start + +- **WHEN** the endpoint reports that it is still starting +- **THEN** the command waits and retries until it is ready or the timeout + passes, rather than failing on the first attempt + +#### Scenario: Restarting the endpoint + +- **WHEN** the user runs `spinloop remote restart` for a running environment and + the endpoint reports ready again +- **THEN** the instance was stopped and re-woken without being terminated, the + command blocked until the model was serving again, and the environment's + address is the one its configuration records + +#### Scenario: Forcing a restart skips the engine stop + +- **WHEN** the user runs `spinloop remote restart --force` (or `-F`) +- **THEN** the instance is stopped without the engine being asked to shut down + first, and the command then blocks until the model is serving again + +#### Scenario: Restarting a stopped endpoint starts it + +- **WHEN** the user runs `spinloop remote restart` for an environment whose instance is already stopped +- **THEN** the instance is re-woken rather than replaced, and the command blocks + until the model is serving again, as with a plain start + +#### Scenario: A failed re-wake says how to recover + +- **WHEN** the stop half of a restart has taken effect but the wake fails +- **THEN** the command fails saying the instance is stopped and that + `spinloop remote start` will bring it back + +#### Scenario: Listing environments + +- **WHEN** the user runs `spinloop remote ls` +- **THEN** the registered environments are listed rather than any endpoint being + contacted + +#### Scenario: Setting a keep deadline + +- **WHEN** the user runs `spinloop remote keep 2h` +- **THEN** the instance retention tag is set and the deadline is reported + +#### Scenario: Metrics reports instance figures + +- **WHEN** the user runs `spinloop remote metrics` with a running instance +- **THEN** token counts, resource usage, and GPU information are displayed + +#### Scenario: Bootstrap is a recognised subcommand + +- **WHEN** the user runs `spinloop remote bootstrap` +- **THEN** the command is dispatched to the provisioning flow rather than + reported as unknown + +#### Scenario: Bake is a recognised subcommand + +- **WHEN** the user runs `spinloop remote bake llamacpp` +- **THEN** the command is dispatched to the bake flow rather than + reported as unknown + +#### Scenario: Auth is a recognised subcommand + +- **WHEN** the user runs `spinloop remote auth` +- **THEN** the command is dispatched to the credential store, report, and clear + flow rather than reported as unknown + +#### Scenario: Unknown subcommand + +- **WHEN** the user runs `spinloop remote frobnicate` +- **THEN** the command fails listing the accepted subcommands, which include + `bootstrap`, `bake`, `metrics`, and `keep` + +#### Scenario: The read verbs are not in the group + +- **WHEN** the operator runs `spinloop remote status`, `spinloop remote + metrics` or `spinloop remote logs` +- **THEN** each fails naming the top-level verb that replaced it, and the + group's help lists none of them + +## REMOVED Requirements + +### Requirement: Status reports when the endpoint last did work + +**Reason**: `spinloop remote status` is removed — one command reports an +engine's state, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop status --env `. + +### Requirement: Status degrades when activity cannot be read + +**Reason**: `spinloop remote status` is removed — one command reports an +engine's state, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop status --env `. + +## ADDED Requirements + +### Requirement: An environment reports when it last did work + +`spinloop status --env ` SHALL report how long it has been since the endpoint's +engine last did any work, alongside the instance state and health it reports +already. The figure SHALL come from the activity the on-instance daemon +tracks, not from a measurement the control plane makes itself — one answer, +derived on the box, however it is asked for. + +The figure SHALL be labelled "last active", matching the wording and duration +formatting used everywhere else this fact appears, so the same fact reads the +same way in every command. + +Collecting it SHALL NOT make `status` slower than its health check already +makes it: the daemon SHALL be asked in parallel with the health check rather +than after it. Nor SHALL it introduce a side effect — `status` SHALL remain a +read, and SHALL still perform no TCP probe. + +#### Scenario: A running endpoint reports its last activity + +- **WHEN** the user runs `spinloop status --env ` against a running endpoint + whose engine has served work +- **THEN** the output reports how long ago that work happened, labelled "last + active", beside the state and health lines + +#### Scenario: Status stays a read + +- **WHEN** the user runs `spinloop status --env ` +- **THEN** nothing is started, stopped or probed in order to obtain the + last-active figure + +### Requirement: An environment's status degrades when activity cannot be read + +`spinloop status --env ` SHALL omit the last-active figure rather than fail, +report zero, or imply inactivity, whenever the figure cannot be obtained. That +covers an endpoint whose engine has not yet done any work, a daemon that +cannot be reached or answers unrecognisably, and an instance that is not +running — reaching the daemon needs a running box, so a stopped or undeployed +environment has nothing to report about its engine. + +A failure to read the activity SHALL NOT affect the rest of the report: the +state and health lines SHALL be exactly what they are today, and the command +SHALL still succeed. + +#### Scenario: A stopped instance reports no activity figure + +- **WHEN** the user runs `spinloop status --env ` and the instance is stopped or + undeployed +- **THEN** the output reports the state as it does today and shows no + last-active figure + +#### Scenario: An unreachable daemon does not spoil the report + +- **WHEN** the endpoint is running but its daemon cannot be reached +- **THEN** the state and health lines are reported as they are today, no + last-active figure is shown, and the command succeeds + +#### Scenario: An engine that has done nothing yet + +- **WHEN** the endpoint is running and its daemon reports no last-active time +- **THEN** no last-active figure is shown, rather than one implying the engine + has been quiet since it started diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-logs/spec.md b/openspec/changes/top-level-metrics-and-logs/specs/remote-logs/spec.md new file mode 100644 index 00000000..8477d83a --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/specs/remote-logs/spec.md @@ -0,0 +1,269 @@ +## REMOVED Requirements + +### Requirement: An environment's shipped logs are readable from the CLI + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +### Requirement: Logs are readable after the instance is gone + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +### Requirement: Both engine and boot logs are reachable + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +### Requirement: The volume fetched is bounded and controllable + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +### Requirement: Output is ordered, timestamped and attributable + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +### Requirement: New output can be followed + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +### Requirement: Missing logs and missing access are explained + +**Reason**: `spinloop remote logs` is removed — one command reads an +engine's output, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop logs --env `. + +## ADDED Requirements + +### Requirement: An environment's an environment's shipped logs are readable from the CLI + +`spinloop logs --env ` SHALL print the logs an environment's instances have +shipped, without the operator needing to know the log group or stream naming, +open the AWS console, or connect to an instance. It SHALL select which +environment to read using the same rules as the other remote subcommands — the +`--env ` flag naming a registered environment, else the `default` +environment — so `spinloop logs --env ` and `spinloop remote status` given the +same `--env` always speak about the same environment. + +#### Scenario: Reading the current environment's logs + +- **WHEN** the operator runs `spinloop logs --env ` where `spinloop remote status` + would report on an environment +- **THEN** the log events that environment's instances shipped are printed +- **AND** the operator is not required to name a log group, stream, or instance + +#### Scenario: Reading a named environment's logs + +- **WHEN** the operator runs `spinloop logs --env --env dev-2` +- **THEN** `dev-2`'s logs are printed rather than the default + environment's + +### Requirement: An environment's logs are readable after the instance is gone + +Reading logs SHALL NOT depend on an instance being running, nor on the +environment's control endpoints answering. Logs SHALL be read from the durable +store the instances ship to, so the output of a boot that failed, or of an +instance that has since terminated, is still available. + +#### Scenario: A terminated instance's logs are still readable + +- **WHEN** an instance has produced logs and has since terminated +- **THEN** `spinloop logs --env ` still prints that instance's shipped events + +#### Scenario: A stopped environment can be diagnosed + +- **WHEN** an environment is stopped, so its status reports no running instance +- **THEN** `spinloop logs --env ` still prints the logs from its previous runs + +### Requirement: An environment's both engine and boot logs are reachable + +The command SHALL be able to read either log source an instance ships — the +inference engine's output and the boot (user-data) output — and both together. +The engine log SHALL be the default source, since it is what an operator wants +once the model is serving. Selecting the boot source SHALL be possible without +knowing which engine the environment runs, so a failure that happened before +the engine started is reachable even though the engine log is empty. + +#### Scenario: Engine output by default + +- **WHEN** the operator runs `spinloop logs --env ` with no source selected +- **THEN** the environment's engine log events are printed + +#### Scenario: Boot output on request + +- **WHEN** the operator asks for the boot source +- **THEN** the environment's start-up output is printed, including steps that + run before the engine starts + +#### Scenario: Both sources interleaved + +- **WHEN** the operator asks for all sources +- **THEN** events from both the engine and boot logs are printed together in + time order +- **AND** each line identifies which source it came from + +#### Scenario: The engine need not be named + +- **WHEN** an environment's logs are read and the operator has not stated which + inference engine it runs +- **THEN** the engine's logs are found regardless of which supported engine + produced them + +### Requirement: An environment's the volume fetched is bounded and controllable + +The command SHALL bound what it fetches by default rather than pulling an +environment's entire retained history, and SHALL let the operator widen or +narrow that: how far back to look, how many events at most to return, and +whether to restrict output to a single instance. When a bound causes older +events to be omitted, the command SHALL say so rather than presenting a +truncated view as complete. + +#### Scenario: A default window applies + +- **WHEN** the operator runs `spinloop logs --env ` with no window stated +- **THEN** only events from a bounded recent window are fetched + +#### Scenario: The window is widened + +- **WHEN** the operator states how far back to look +- **THEN** events from that whole period are fetched, subject to the retention + of the durable store + +#### Scenario: Output is capped + +- **WHEN** more events match than the stated maximum +- **THEN** the most recent events up to that maximum are printed +- **AND** the operator is told that earlier matching events were omitted + +#### Scenario: One instance is singled out + +- **WHEN** the operator names an instance +- **THEN** only that instance's events are printed, and events from the + environment's other instances are excluded + +### Requirement: An environment's output is ordered, timestamped and attributable + +Events SHALL be printed oldest first, each carrying its timestamp, so the +output reads like a log rather than an unordered dump. When the printed events +come from more than one instance or more than one source, each line SHALL +identify which instance and source it came from; when there is only one of +each, that labelling SHALL be omitted so the common case stays uncluttered. A +machine-readable output format SHALL also be available, carrying the same +fields for scripting. + +#### Scenario: Chronological, timestamped output + +- **WHEN** events are printed +- **THEN** they appear oldest first, each preceded by its timestamp + +#### Scenario: Mixed origins are labelled + +- **WHEN** the printed events come from more than one instance, or from both + sources +- **THEN** each line identifies its source and instance + +#### Scenario: A single origin is not labelled + +- **WHEN** every printed event comes from the same source and the same instance +- **THEN** the lines carry no source or instance prefix + +#### Scenario: Machine-readable output + +- **WHEN** the operator asks for the machine-readable format +- **THEN** the events are emitted as structured records carrying at least the + timestamp, source, instance and message + +### Requirement: An environment's new output can be followed + +The command SHALL be able to keep running and print events as they arrive, +rather than exiting after one fetch, so an operator can watch a start or a +crash unfold. Following SHALL print each event once — an event already printed +SHALL NOT be repeated on a later poll — and SHALL stop cleanly on interrupt. + +#### Scenario: Live output is appended + +- **WHEN** the operator follows an environment's logs and the instance writes + more output +- **THEN** the new events are printed as they arrive, after the events already + shown + +#### Scenario: No duplicates while following + +- **WHEN** following continues across several polls +- **THEN** no event that has already been printed is printed again + +#### Scenario: Interrupting stops cleanly + +- **WHEN** the operator interrupts a follow +- **THEN** the command exits without reporting an error + +### Requirement: An environment's missing logs and missing access are explained + +When no output can be produced, the command SHALL distinguish the causes an +operator can act on and say what to do, rather than printing nothing or a raw +service error. It SHALL cover at least: an environment whose stored +configuration does not name the environment, so its streams cannot be +identified; a shared layer deployed before log shipping existed, so the log +group is absent; credentials that lack permission to read the logs; and an +environment that simply has not logged anything in the window asked for. + +#### Scenario: The environment is not named in its config + +- **WHEN** the resolved configuration carries no environment name +- **THEN** the command fails with a message saying the environment cannot be + identified and how to re-register it + +#### Scenario: The log group does not exist + +- **WHEN** the log group the environment would ship to is absent +- **THEN** the command reports that the shared layer predates log shipping and + needs re-deploying, rather than reporting an empty result + +#### Scenario: Credentials cannot read logs + +- **WHEN** the caller's credentials are not permitted to read the log events +- **THEN** the command reports that the credentials lack log-reading permission + and names the permission needed + +#### Scenario: Nothing was logged in the window + +- **WHEN** the log group exists and is readable but holds no events for the + environment in the window asked for +- **THEN** the command reports that there are no events for that environment in + that window, and exits without an error + +#### Scenario: The remote spelling names its replacement + +- **WHEN** the operator runs `spinloop remote logs` +- **THEN** it fails naming `spinloop logs --env ` as the command that + replaced it diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-stats/spec.md b/openspec/changes/top-level-metrics-and-logs/specs/remote-stats/spec.md new file mode 100644 index 00000000..e520fd73 --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/specs/remote-stats/spec.md @@ -0,0 +1,242 @@ +## REMOVED Requirements + +### Requirement: Metrics subcommand + +**Reason**: `spinloop remote metrics` is removed — one command reports an +engine's metrics, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop metrics --env `. + +### Requirement: Optional cost estimation + +**Reason**: `spinloop remote metrics` is removed — one command reports an +engine's metrics, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop metrics --env `. + +### Requirement: Tabular display + +**Reason**: `spinloop remote metrics` is removed — one command reports an +engine's metrics, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop metrics --env `. + +### Requirement: Watch mode + +**Reason**: `spinloop remote metrics` is removed — one command reports an +engine's metrics, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop metrics --env `. + +### Requirement: Reporting when the endpoint last did work + +**Reason**: `spinloop remote metrics` is removed — one command reports an +engine's metrics, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop metrics --env `. + +### Requirement: History in the report + +**Reason**: `spinloop remote metrics` is removed — one command reports an +engine's metrics, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop metrics --env `. + +## ADDED Requirements + +### Requirement: An environment's metrics are reported + +The system SHALL provide a `metrics` subcommand (`spinloop metrics --env `) that reports the current state of a remote inference instance. It SHALL select which environment it reports on using the same rule as the other `remote` subcommands: the `--env ` flag naming a registered environment, falling back to the `default` environment when the flag is absent. A Spinloop given as an argument is read only for its `ENV` instructions and adjacent `.env`, never to select the environment. When the instance is running, the report SHALL include the spinloop version from the daemon, carried by the stats Lambda reply. + +#### Scenario: Stats with a running instance + +- **WHEN** the user runs `spinloop metrics --env ` with a running instance +- **THEN** the command reports the instance state, runner, model, spinloop version, GPU info, CPU/RAM usage, token counts, and request counts + +#### Scenario: Stats with a stopped instance + +- **WHEN** the user runs `spinloop metrics --env ` and the instance is stopped +- **THEN** the command reports `state: stopped` and no metrics + +#### Scenario: Stats names the environment with the flag + +- **WHEN** the user runs `spinloop metrics --env --env dev-2` +- **THEN** the command reports on the `dev-2` environment's instance + +#### Scenario: Stats falls back to the default environment + +- **WHEN** the user runs `spinloop metrics --env ` with no `--env` flag +- **THEN** the command reports on the `default` environment's instance + +#### Scenario: Version is shown in stats output + +- **WHEN** the user runs `spinloop metrics --env ` with a running instance +- **THEN** the output includes the spinloop version + +### Requirement: An environment's cost is reported on request + +When the user passes `--cost`, the stats report SHALL include an estimated on-demand cost for the current running session. The cost SHALL be computed from the instance type's on-demand price (fetched from the AWS Price List API for the deployed region) multiplied by the elapsed time since launch. Without `--cost`, no price lookup is performed and no cost is shown. + +#### Scenario: Cost is shown with flag + +- **WHEN** the user runs `spinloop metrics --env --cost` with a running instance +- **THEN** the report includes the estimated cost for the current session + +#### Scenario: Cost is not shown by default + +- **WHEN** the user runs `spinloop metrics --env ` without `--cost` +- **THEN** the report does not include a cost line + +### Requirement: The metrics report's tabular display + +The stats output SHALL support four formats via the `--format` flag: `gauge` (default), `bar`, `table`, and `json`. The `gauge` format SHALL produce a compact display with horizontal progress gauges for the current reading, colour-coded by utilization level. The `bar` format SHALL produce a compact display drawing each resource series as a sparkline of the daemon's retained history, with the latest point colour-coded by utilization level. The `table` format SHALL produce a tab-separated key-value table, one line per metric, with the key column left-aligned and values right of it. The `json` format SHALL output the response as a JSON object to standard output. Progress and error messages SHALL go to standard error regardless of format. + +#### Scenario: Clean output + +- **WHEN** the command succeeds +- **THEN** standard output contains only the stats data with no progress or debug lines + +#### Scenario: Default format is gauge + +- **WHEN** the user runs `spinloop metrics --env ` without `--format` +- **THEN** the output is in gauge format + +#### Scenario: Table format is explicit + +- **WHEN** the user runs `spinloop metrics --env --format=table` +- **THEN** the output is in table format + +#### Scenario: Bar format is explicit + +- **WHEN** the user runs `spinloop metrics --env --format=bar` +- **THEN** the output is in bar format, drawing each resource series as a sparkline of the daemon's retained history + +#### Scenario: Gauge format is explicit + +- **WHEN** the user runs `spinloop metrics --env --format=gauge` +- **THEN** the output is in gauge format with progress gauges for the current reading + +#### Scenario: JSON format + +- **WHEN** the user runs `spinloop metrics --env --format=json` +- **THEN** the output is valid JSON containing the instance state, runner, model, GPU info, CPU/RAM usage, and token counts + +#### Scenario: JSON format with cost + +- **WHEN** the user runs `spinloop metrics --env --format=json --cost` +- **THEN** the JSON output includes a cost estimate field + +#### Scenario: Invalid format errors + +- **WHEN** the user runs `spinloop metrics --env --format=csv` +- **THEN** the command exits with an error and usage message + +### Requirement: The metrics report refreshes on request + +The system SHALL support a `--watch`/`-w` flag that repeatedly queries metrics every 60 seconds. When enabled, the command SHALL clear the screen and redraw the output in place for each refresh, producing no scrollback accumulation. Each refresh SHALL pre-render the metrics output into a buffer before clearing the screen, so the redisplay is instantaneous after the network round-trip. The command SHALL continue until the user sends `SIGINT` (Ctrl+C) or `SIGTERM`, at which point it SHALL exit cleanly. + +#### Scenario: Watch mode repeats output + +- **WHEN** the user runs `spinloop metrics --env --watch` +- **THEN** the command prints metrics, waits 60 seconds, clears the screen, and prints updated metrics + +#### Scenario: Watch redraws in place + +- **WHEN** the user runs `spinloop metrics --env -w` +- **THEN** each refresh after the first clears the screen before displaying new output, with no separator lines + +#### Scenario: Watch with JSON format + +- **WHEN** the user runs `spinloop metrics --env --watch --format=json` +- **THEN** each refresh clears the screen and outputs a JSON object + +#### Scenario: Watch with cost + +- **WHEN** the user runs `spinloop metrics --env --watch --cost` +- **THEN** each refresh includes the cost estimate + +#### Scenario: Watch stops on interrupt + +- **WHEN** the user runs `spinloop metrics --env -w` and presses Ctrl+C +- **THEN** the command exits cleanly without error + +### Requirement: An environment's metrics report when it last did work + +The metrics report SHALL include how long it has been since the endpoint's +engine last did any work, taken from the activity the on-instance daemon +tracks, in every format the command supports. The figure exists to answer "is +this endpoint doing anything?" at a glance, without the reader having to infer +it from the running-request count. + +The figure SHALL be labelled "last active" rather than "idle": `idle` is +already an engine *state* meaning nothing has been started, and one report +SHALL NOT carry two meanings of the word. The elapsed time SHALL be rendered +the same way the command's other durations are, so an uptime and a last-active +figure read alike. + +An endpoint whose daemon reports no activity — because no engine has run yet, +or because the daemon could not be reached — SHALL omit the figure rather than +show one implying the endpoint has been quiet since it started. + +#### Scenario: A working endpoint reports its last activity + +- **WHEN** the user runs `spinloop metrics --env ` against a running endpoint + whose engine has served work +- **THEN** the report shows how long ago that work happened, labelled "last + active" + +#### Scenario: Every format carries the figure + +- **WHEN** the user runs `spinloop metrics --env ` with `--format=bar`, + `--format=table`, or `--format=json` +- **THEN** each output carries the last-active figure in its own idiom + +#### Scenario: An endpoint that has done nothing omits the figure + +- **WHEN** the user runs `spinloop metrics --env ` against an endpoint whose + engine has not yet done any work +- **THEN** the report shows no last-active figure + +#### Scenario: An unreachable daemon omits the figure + +- **WHEN** the control plane cannot reach the instance's daemon to collect + metrics +- **THEN** the report shows no last-active figure, and the rest of the report + renders as it does today + +### Requirement: History in the metrics report + +When the on-instance daemon's metrics reply carries a history of system readings, the report SHALL carry it through to the command's output: the `json` format SHALL include the readings, and the `bar` format SHALL draw them. Where the daemon's reply carries no history, the report SHALL omit the field and the `bar` format SHALL fall back per the bar format specification. The control plane's relay of the daemon's reply SHALL NOT alter the readings it carries. + +#### Scenario: JSON carries the daemon's history + +- **WHEN** the instance's daemon reports a retained history and the user runs `spinloop metrics --env --format=json` +- **THEN** the JSON output includes the history's readings + +#### Scenario: Bar draws the relayed history + +- **WHEN** the instance's daemon reports a retained history and the user runs `spinloop metrics --env --format=bar` +- **THEN** each resource series is drawn as a sparkline from the readings the control plane relayed + +#### Scenario: A daemon without history degrades + +- **WHEN** the instance runs a daemon whose reply carries no history and the user runs `spinloop metrics --env --format=bar` +- **THEN** the report omits the history field and bar format draws the current reading in the gauge's filled style + +#### Scenario: The remote spelling names its replacement + +- **WHEN** the operator runs `spinloop remote metrics` +- **THEN** it fails naming `spinloop metrics --env ` as the command that + replaced it diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-version-reporting/spec.md b/openspec/changes/top-level-metrics-and-logs/specs/remote-version-reporting/spec.md new file mode 100644 index 00000000..ece220cc --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/specs/remote-version-reporting/spec.md @@ -0,0 +1,53 @@ +## REMOVED Requirements + +### Requirement: Remote status shows version + +**Reason**: `spinloop remote status` is removed — one command reports an +engine's state, whether it is named as an environment or as a fleet node. +The behaviour is unchanged and is restated below under a name that does not +carry the removed command's spelling. + +**Migration**: `spinloop status --env `. + +### Requirement: Remote metrics shows version + +**Reason**: `spinloop remote metrics` is removed; the version it displayed is +displayed by the command that replaced it, from the same stats reply. Restated +below under a name that does not carry the removed spelling. + +**Migration**: `spinloop metrics --env `. + +## ADDED Requirements + +### Requirement: An environment's metrics show version + +`spinloop metrics --env ` SHALL display the spinloop version in its output, as the stats Lambda already reads the daemon and can carry the version alongside its existing fields. + +#### Scenario: Version is shown in table format + +- **WHEN** the user runs `spinloop metrics --env --format=table` against a running instance +- **THEN** the table output includes a `version` line + +#### Scenario: Version is shown in JSON format + +- **WHEN** the user runs `spinloop metrics --env --format=json` against a running instance +- **THEN** the JSON output includes a `version` field + +#### Scenario: Version is omitted from bar header when unavailable + +- **WHEN** the user runs `spinloop metrics --env --format=bar` and the version is not available +- **THEN** the bar header omits the version without error + +### Requirement: An environment's status shows version + +`spinloop status --env ` SHALL display the spinloop version running on the remote instance alongside its existing state, health, and base URL fields. + +#### Scenario: Version is shown when the instance is running + +- **WHEN** the user runs `spinloop status --env ` against a running instance +- **THEN** the output includes a `version` line with the spinloop version string (e.g. `version: 1.16.0`) + +#### Scenario: Version is unavailable when the instance is stopped + +- **WHEN** the user runs `spinloop status --env ` against a stopped instance +- **THEN** the output omits the version line, since the daemon is not reachable diff --git a/openspec/changes/top-level-metrics-and-logs/tasks.md b/openspec/changes/top-level-metrics-and-logs/tasks.md new file mode 100644 index 00000000..c659b877 --- /dev/null +++ b/openspec/changes/top-level-metrics-and-logs/tasks.md @@ -0,0 +1,68 @@ +## 1. The capabilities + +- [x] 1.1 Add `Coster` to `internal/fleet` beside `ProgressStarter` and + `Keeper`: what a node has cost so far and its hourly rate, answered by + `remoteNode` from its own instance type and region (design D1). Verify a + unit test over both kinds — the cloud node answers, the daemon does not + implement it. +- [x] 1.2 Add `SourceLogger`: reading a node's log by source, time window and + instance. Verify the cloud node answers all three and the daemon does + not implement it. +- [x] 1.3 Add `Versioner`: the spinloop release a node reports. Verify it is + answered from the reply the node already holds, with no extra call + (design D3), and that `statsFromRemote` stops dropping the version and + instance type. +- [x] 1.4 Verify no caller branches on node kind: a test or a grep confirming + the capabilities are reached only by type assertion. + +## 2. The verbs + +- [ ] 2.1 Add `cmd/spinloop/metrics.go`: a root-registered `metrics` taking + `--env`/`--fleet`, `--format`, `--watch` and `--cost`, resolving through + `resolveFleetTarget` and rendering through the existing formatters. + Verify each format renders for a fleet and for a single environment. +- [ ] 2.2 Add `cmd/spinloop/logs.go` the same way, with `--follow`, `--limit`, + `--format`, and the `--source`/`--since`/`--instance` the capability + answers. Verify `-f` remains `--follow` and the fleet file is long-form + only. +- [ ] 2.3 Wire `--cost` to `Coster`: priced nodes carry the figure, the rest + render as they would without the flag, and a target with none succeeds + (design D2). Verify with a mixed target and an all-daemon one. +- [ ] 2.4 Wire `--source`/`--since`/`--instance` to `SourceLogger` on the same + terms. Verify a mixed target reads the environment's boot log and the + daemon's ordinary output. +- [ ] 2.5 Read a Spinloop given to either verb for its `ENV` instructions and + adjacent `.env` only, never to select a target (design D4). Verify a + Spinloop whose `ENV` supplies `SPINLOOP_REMOTE_*` configures the command. +- [ ] 2.6 Register both at the root with the completion the fleet spellings + had. Verify the completion test covers them. + +## 3. Removing the old spellings + +- [ ] 3.1 Delete `fleetMetricsCmd` and `fleetLogsCmd` and unregister them, + keeping their renderers. Verify `spinloop fleet --help` lists neither. +- [ ] 3.2 Delete `remoteStatusCmd`, `remoteMetricsCmd`, `remoteLogsCmd` and + their bodies, keeping the formatters the top-level verbs use. Verify + `spinloop remote --help` lists none of the three. +- [ ] 3.3 Signpost all five moved spellings through `movedSubcommands`, giving + the `remote` group the same `Args: groupArgs` the fleet group has + (design D5). Verify each names its replacement and an unknown subcommand + in either group still gets cobra's own error. + +## 4. Consumers, docs and verification + +- [ ] 4.1 Update every invocation of the five moved spellings across `docs/`, + `README.md` and `examples/` — including the two CI-run `run-tests.sh` + scripts, whose `fleet()` shell helper means a bare rename breaks them. + Verify `bash -n` on each script and that none invokes a removed + spelling. +- [ ] 4.2 Add `docs/commands/metrics.md` and `logs.md`, each saying which + flags apply to which node kinds (required by the capability + requirement), and point `fleet.md` and `remote.md` at them. Verify + `docs/README.md`'s command table lists both. +- [ ] 4.3 Verify nothing an operator could get from the removed commands is + unreachable: the cost, the version, the log sources, the `ENV` path, and + the endpoint address via `remote env`. +- [ ] 4.4 Run `gofmt -l .` (expect no output), `go vet ./...` and + `go test ./... -cover`, confirming total coverage is unchanged and still + >= 80%. From 149f2982d7c104af0a23219d96c337857ef5576b Mon Sep 17 00:00:00 2001 From: spinloop-agent Date: Fri, 18 Sep 2026 22:05:59 +0100 Subject: [PATCH 2/4] feat: make metrics and logs top-level verbs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- cmd/spinloop/commands.go | 4 +- cmd/spinloop/flagparse_test.go | 4 +- cmd/spinloop/fleet.go | 46 +--------- cmd/spinloop/fleet_logs.go | 58 ++---------- cmd/spinloop/fleet_logs_test.go | 26 +++--- cmd/spinloop/fleet_test.go | 41 ++++++--- cmd/spinloop/last_active_test.go | 8 +- cmd/spinloop/logs.go | 92 +++++++++++++++++++ cmd/spinloop/metrics.go | 78 ++++++++++++++++ cmd/spinloop/retain_render_test.go | 4 +- cmd/spinloop/target_test.go | 2 +- internal/fleet/fanout.go | 50 +++++++++- internal/fleet/node.go | 10 ++ .../top-level-metrics-and-logs/tasks.md | 12 +-- 14 files changed, 296 insertions(+), 139 deletions(-) create mode 100644 cmd/spinloop/logs.go create mode 100644 cmd/spinloop/metrics.go diff --git a/cmd/spinloop/commands.go b/cmd/spinloop/commands.go index 2475f822..d6d3284a 100644 --- a/cmd/spinloop/commands.go +++ b/cmd/spinloop/commands.go @@ -76,6 +76,8 @@ has been.`, upCmd(), statusCmd(), dashboardCmd(), + metricsCmd(), + logsCmd(), codeCmd(), daemonCmd(), gatewayCmd(), @@ -370,8 +372,6 @@ error — only a problem with the fleet file itself fails a command.`, RunE: groupFallback, } fleet.AddCommand( - fleetMetricsCmd(), - fleetLogsCmd(), fleetRouteCmd(), fleetStartCmd(), fleetStopCmd(), diff --git a/cmd/spinloop/flagparse_test.go b/cmd/spinloop/flagparse_test.go index b35be437..cd476b04 100644 --- a/cmd/spinloop/flagparse_test.go +++ b/cmd/spinloop/flagparse_test.go @@ -28,7 +28,7 @@ func TestPflagParseForms(t *testing.T) { "add": func() error { return cmdAdd([]string{"-p", "ollama", "--nope"}) }, "apply": func() error { return cmdApply([]string{"--nope"}) }, "serve": func() error { return cmdServe([]string{"--nope"}) }, - "fleet metrics": func() error { return cmdFleet([]string{"metrics", "--nope"}) }, + "fleet metrics": func() error { return cmdMetrics([]string{"--nope"}) }, "remote start": func() error { return cmdRemoteStart([]string{"--env", "default", "--nope"}) }, "daemon": func() error { return cmdDaemon([]string{"--nope"}) }, } { @@ -50,7 +50,7 @@ func TestPflagParseForms(t *testing.T) { // takes them in place. An unknown flag raised *after* the positional is // the proof that parsing continued past it. for name, call := range map[string]func() error{ - "fleet metrics": func() error { return cmdFleet([]string{"metrics", "someNode", "--nope"}) }, + "fleet metrics": func() error { return cmdMetrics([]string{"someNode", "--nope"}) }, "remote env": func() error { return cmdRemoteEnv([]string{"--env", "default", "somePath", "--nope"}) }, } { if err := call(); err == nil || !strings.Contains(err.Error(), "unknown flag: --nope") { diff --git a/cmd/spinloop/fleet.go b/cmd/spinloop/fleet.go index 7bbecef2..7c1d4e5b 100644 --- a/cmd/spinloop/fleet.go +++ b/cmd/spinloop/fleet.go @@ -75,52 +75,10 @@ func fleetRow(r fleet.NodeResult) (state, serving string) { return f.State, f.servingText() } -// fleetMetricsCmd renders every node's engine and system metrics. --watch -// redraws the whole fleet on an interval. -func fleetMetricsCmd() *cobra.Command { - var ( - path string - envName string - format string - watch bool - ) - c := &cobra.Command{ - Use: "metrics", - Short: "sample every node's engine metrics", - Args: cobra.ArbitraryArgs, - SilenceErrors: true, - SilenceUsage: true, - RunE: func(c *cobra.Command, _ []string) error { - resolve(c) - if err := validateMetricsFormat(format); err != nil { - return err - } - cfg, err := resolveFleetTarget(fleetTarget{envName: envName, fleetPath: path}) - if err != nil { - return err - } - if watch { - return runFleetMetricsWatch(cfg, format) - } - results := cfg.FanOut(context.Background(), fleet.MetricsCall) - return renderFleetMetrics(os.Stdout, results, format) - }, - } - fs := c.Flags() - fs.StringVarP(&path, "fleet", "f", "", fleetFileUsage) - fs.StringVar(&envName, "env", "", envFlagTargetUsage) - fs.StringVar(&format, "format", "gauge", "output format: gauge (default), bar, table or json") - fs.BoolVarP(&watch, "watch", "w", false, "redraw the fleet every 60 seconds") - c.ValidArgsFunction = noPositionals - compRegister(c, "fleet", compFiles) - compRegister(c, "env", compEnvs) - return c -} - // runFleetMetricsWatch redraws the fleet until interrupted. Each refresh is // rendered into a buffer first, so the screen is cleared and rewritten in one // go — a slow node delays a refresh but never tears the display. -func runFleetMetricsWatch(cfg *fleet.Config, format string) error { +func runFleetMetricsWatch(cfg *fleet.Config, format string, call fleet.Call) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -135,7 +93,7 @@ func runFleetMetricsWatch(cfg *fleet.Config, format string) error { first := true for { var buf strings.Builder - results := cfg.FanOut(ctx, fleet.MetricsCall) + results := cfg.FanOut(ctx, call) if ctx.Err() != nil { return nil } diff --git a/cmd/spinloop/fleet_logs.go b/cmd/spinloop/fleet_logs.go index 3fd1fa3d..be108d10 100644 --- a/cmd/spinloop/fleet_logs.go +++ b/cmd/spinloop/fleet_logs.go @@ -9,7 +9,6 @@ import ( "strings" "time" - "github.com/spf13/cobra" "github.com/spinloop-ai/spinloop/internal/fleet" ) @@ -25,59 +24,18 @@ var fleetLogsInterval = 3 * time.Second // cap would impose anyway. const bytesPerLineGuess = 512 -// fleetLogsCmd prints the engine output of the fleet's nodes. Unlike start and -// stop it fans out by default: reading is safe, and "what did my engines say?" -// is a fleet-wide question. Naming a node narrows it to that one. -func fleetLogsCmd() *cobra.Command { - var ( - path string - follow bool - limit int - format string - envName string - ) - const followUsage = "keep printing new output as it arrives" - c := &cobra.Command{ - Use: "logs", - Short: "tail the engines' logs", - Args: cobra.ArbitraryArgs, - SilenceErrors: true, - SilenceUsage: true, - RunE: func(c *cobra.Command, args []string) error { - resolve(c) - return runFleetLogs(fleetTarget{envName: envName, fleetPath: path}, follow, limit, format, args) - }, - } - fs := c.Flags() - // --fleet takes no short form here: -f is already --follow, and a flag - // cannot carry two meanings on one command line. Every other fleet - // subcommand offers -f for the fleet file. - fs.StringVar(&path, "fleet", "", fleetFileUsage) - fs.StringVar(&envName, "env", "", envFlagTargetUsage) - fs.BoolVarP(&follow, "follow", "f", false, followUsage) - fs.IntVar(&limit, "limit", 200, "lines of backlog to print per node") - fs.StringVar(&format, "format", "text", "output format: text (default) or json") - c.ValidArgsFunction = noPositionals - compRegister(c, "fleet", compFiles) - compRegister(c, "env", compEnvs) - return c -} - // runFleetLogs is the body of `spinloop fleet logs`. -func runFleetLogs(target fleetTarget, follow bool, limit int, format string, args []string) error { - if format != "text" && format != "json" { - return fmt.Errorf("--format must be \"text\" or \"json\", got %q", format) - } - if limit <= 0 { - return fmt.Errorf("--limit must be positive, got %d", limit) +func runLogs(target fleetTarget, q fleet.LogQuery, follow bool, limit int, format string, args []string) error { + if err := validateLogQuery(q, format, limit); err != nil { + return err } cfg, err := resolveFleetTarget(target) if err != nil { return err } - // A named node restricts the read; the fleet file still supplies its - // details, so an unknown name is caught here rather than at the socket. + // A named node restricts the read; the target still supplies its details, + // so an unknown name is caught here rather than at the socket. if len(args) > 0 { if cfg, err = cfg.Only(args[0]); err != nil { return err @@ -87,12 +45,12 @@ func runFleetLogs(target fleetTarget, follow bool, limit int, format string, arg if follow { return followFleetLogs(cfg, limit, format) } - return runFleetLogsOnce(context.Background(), cfg, limit, format, os.Stdout) + return runFleetLogsOnce(context.Background(), cfg, q, limit, format, os.Stdout) } // runFleetLogsOnce reads each node's backlog and prints it. -func runFleetLogsOnce(ctx context.Context, cfg *fleet.Config, limit int, format string, w io.Writer) error { - results := cfg.FanOut(ctx, fleet.LogsCall(nil, limit*bytesPerLineGuess)) +func runFleetLogsOnce(ctx context.Context, cfg *fleet.Config, q fleet.LogQuery, limit int, format string, w io.Writer) error { + results := cfg.FanOut(ctx, fleet.QueriedLogsCall(q, limit*bytesPerLineGuess)) for i := range results { if results[i].OK() { results[i].Logs.Content = lastLines(results[i].Logs.Content, limit) diff --git a/cmd/spinloop/fleet_logs_test.go b/cmd/spinloop/fleet_logs_test.go index b15e86b0..9e994f81 100644 --- a/cmd/spinloop/fleet_logs_test.go +++ b/cmd/spinloop/fleet_logs_test.go @@ -54,7 +54,7 @@ func TestCmdFleetLogsPrintsOneNodeUnlabelled(t *testing.T) { oneLogFleet(t, "loading weights\nserving\n") out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs"}); err != nil { + if err := cmdLogs(nil); err != nil { t.Errorf("fleet logs returned %v", err) } }) @@ -73,7 +73,7 @@ func TestCmdFleetLogsLabelsSeveralNodes(t *testing.T) { hostA, portA, hostB, portB)) out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs"}); err != nil { + if err := cmdLogs(nil); err != nil { t.Errorf("fleet logs returned %v", err) } }) @@ -108,7 +108,7 @@ func TestCmdFleetLogsFollowTakesTheFleetFile(t *testing.T) { t.Chdir(t.TempDir()) done := make(chan error, 1) - go func() { done <- cmdFleet([]string{"logs", "-f", "--fleet", path}) }() + go func() { done <- cmdLogs([]string{"-f", "--fleet", path}) }() // The first poll is the proof of the parse: if -f had been the fleet-file // flag it would have taken --fleet as its value and failed before @@ -146,7 +146,7 @@ func TestCmdFleetLogsFollowTakesTheFleetFile(t *testing.T) { // -f is follow on logs, so the word after it is a node name, not a fleet file. func TestCmdFleetLogsShortFlagIsNotTheFleetFile(t *testing.T) { oneLogFleet(t, "hello\n") - err := cmdFleet([]string{"logs", "-f", "nope"}) + err := cmdLogs([]string{"-f", "nope"}) if err == nil || !strings.Contains(err.Error(), `no node "nope"`) { t.Fatalf("want the unknown-node error for the word after -f, got %v", err) } @@ -165,7 +165,7 @@ func TestCmdFleetLogsReadsOneNamedNode(t *testing.T) { hostA, portA, hostB, portB)) out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs", "beta"}); err != nil { + if err := cmdLogs([]string{"beta"}); err != nil { t.Errorf("fleet logs beta returned %v", err) } }) @@ -184,7 +184,7 @@ func TestCmdFleetLogsReadsOneNamedNode(t *testing.T) { func TestCmdFleetLogsUnknownNodeNamesTheKnownOnes(t *testing.T) { oneLogFleet(t, "x\n") - err := cmdFleet([]string{"logs", "nope"}) + err := cmdLogs([]string{"nope"}) if err == nil { t.Fatal("an unknown node should be an error") } @@ -205,7 +205,7 @@ func TestCmdFleetLogsReportsNodesWithNothingToGive(t *testing.T) { hostUp, portUp, hostOld, portOld)) out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs"}); err != nil { + if err := cmdLogs(nil); err != nil { t.Errorf("one bad node must not fail the command, got %v", err) } }) @@ -224,7 +224,7 @@ func TestCmdFleetLogsReportsAMissingLog(t *testing.T) { oneLogFleet(t, "") out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs"}); err != nil { + if err := cmdLogs(nil); err != nil { t.Errorf("fleet logs returned %v", err) } }) @@ -237,7 +237,7 @@ func TestCmdFleetLogsJSON(t *testing.T) { oneLogFleet(t, "serving\n") out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs", "--format", "json"}); err != nil { + if err := cmdLogs([]string{"--format", "json"}); err != nil { t.Errorf("fleet logs --format json returned %v", err) } }) @@ -261,11 +261,11 @@ func TestCmdFleetLogsJSON(t *testing.T) { func TestCmdFleetLogsRejectsBadFlags(t *testing.T) { oneLogFleet(t, "x\n") - if err := cmdFleet([]string{"logs", "--format", "yaml"}); err == nil || + if err := cmdLogs([]string{"--format", "yaml"}); err == nil || !strings.Contains(err.Error(), "--format") { t.Errorf("bad format: got %v", err) } - if err := cmdFleet([]string{"logs", "--limit", "0"}); err == nil || + if err := cmdLogs([]string{"--limit", "0"}); err == nil || !strings.Contains(err.Error(), "--limit") { t.Errorf("bad limit: got %v", err) } @@ -473,7 +473,7 @@ func TestCmdFleetLogsReportsARejectedToken(t *testing.T) { writeFleetFile(t, fmt.Sprintf("nodes:\n - name: box\n host: %s\n port: %d\n", host, port)) out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs"}); err != nil { + if err := cmdLogs(nil); err != nil { t.Errorf("a rejected token must not fail the command, got %v", err) } }) @@ -504,7 +504,7 @@ func TestCmdFleetLogsDistinguishesAnEmptyLogFromAMissingOne(t *testing.T) { writeFleetFile(t, fmt.Sprintf("nodes:\n - name: box\n host: %s\n port: %d\n", host, port)) out := captureStdout(t, func() { - if err := cmdFleet([]string{"logs"}); err != nil { + if err := cmdLogs(nil); err != nil { t.Errorf("fleet logs returned %v", err) } }) diff --git a/cmd/spinloop/fleet_test.go b/cmd/spinloop/fleet_test.go index 31557f64..007eb693 100644 --- a/cmd/spinloop/fleet_test.go +++ b/cmd/spinloop/fleet_test.go @@ -147,7 +147,7 @@ func TestCmdFleetMetricsFormats(t *testing.T) { t.Run("default (gauge)", func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics"}); err != nil { + if err := cmdMetrics(nil); err != nil { t.Error(err) } }) @@ -167,7 +167,7 @@ func TestCmdFleetMetricsFormats(t *testing.T) { t.Run("bar", func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics", "--format=bar"}); err != nil { + if err := cmdMetrics([]string{"--format=bar"}); err != nil { t.Error(err) } }) @@ -181,7 +181,7 @@ func TestCmdFleetMetricsFormats(t *testing.T) { t.Run("table", func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics", "--format=table"}); err != nil { + if err := cmdMetrics([]string{"--format=table"}); err != nil { t.Error(err) } }) @@ -192,7 +192,7 @@ func TestCmdFleetMetricsFormats(t *testing.T) { t.Run("json covers the whole fleet", func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics", "--format=json"}); err != nil { + if err := cmdMetrics([]string{"--format=json"}); err != nil { t.Error(err) } }) @@ -221,7 +221,7 @@ func TestCmdFleetMetricsFormats(t *testing.T) { func TestCmdFleetMetricsRejectsBadFormat(t *testing.T) { twoNodeFleet(t, "running") - if err := cmdFleet([]string{"metrics", "--format=csv"}); err == nil { + if err := cmdMetrics([]string{"--format=csv"}); err == nil { t.Fatal("--format=csv accepted") } } @@ -262,7 +262,7 @@ func TestCmdFleetMetricsDrawsHistory(t *testing.T) { upHost, upPort, downHost, downPort)) out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics"}); err != nil { + if err := cmdMetrics(nil); err != nil { t.Error(err) } }) @@ -281,7 +281,7 @@ func TestCmdFleetMetricsDrawsHistory(t *testing.T) { // bar draws the retained readings alone, including the GPU series the // current reading no longer names. out = captureStdout(t, func() { - if err := cmdFleet([]string{"metrics", "--format=bar"}); err != nil { + if err := cmdMetrics([]string{"--format=bar"}); err != nil { t.Error(err) } }) @@ -689,7 +689,7 @@ func TestFleetFlagShortForm(t *testing.T) { fleet := commandUnder(t, root, "fleet") // status and dashboard are top-level verbs now; the rest still hang off // the group, and every one of them offers -f for the fleet file. - for _, name := range []string{"metrics", "start", "stop", "deploy", "route"} { + for _, name := range []string{"start", "stop", "deploy", "route"} { sub := commandUnder(t, fleet, name) f := sub.Flags().Lookup("fleet") if f == nil { @@ -704,16 +704,29 @@ func TestFleetFlagShortForm(t *testing.T) { if f := commandUnder(t, commandUnder(t, root, "harness"), "open").Flags().Lookup("fleet"); f == nil || f.Shorthand != "f" { t.Errorf("harness open: --fleet lacks the -f shorthand") } - logs := commandUnder(t, fleet, "logs") + // logs is a top-level verb now, and the one surface where -f means follow + // rather than the fleet file. + logs := commandUnder(t, root, "logs") if f := logs.Flags().Lookup("fleet"); f == nil { - t.Error("fleet logs: no --fleet flag") + t.Error("logs: no --fleet flag") } else if f.Shorthand != "" { - t.Errorf("fleet logs: --fleet carries shorthand %q, want none", f.Shorthand) + t.Errorf("logs: --fleet carries shorthand %q, want none", f.Shorthand) } if f := logs.Flags().Lookup("follow"); f == nil { - t.Error("fleet logs: no --follow flag") + t.Error("logs: no --follow flag") } else if f.Shorthand != "f" { - t.Errorf("fleet logs: --follow shorthand = %q, want \"f\"", f.Shorthand) + t.Errorf("logs: --follow shorthand = %q, want \"f\"", f.Shorthand) + } + // The other top-level read verbs keep -f for the fleet file. + for _, name := range []string{"status", "dashboard", "metrics"} { + f := commandUnder(t, root, name).Flags().Lookup("fleet") + if f == nil { + t.Errorf("%s: no --fleet flag", name) + continue + } + if f.Shorthand != "f" { + t.Errorf("%s: shorthand = %q, want \"f\"", name, f.Shorthand) + } } } @@ -755,7 +768,7 @@ func TestCmdFleetMetricsWatchExitsOnInterrupt(t *testing.T) { t.Cleanup(func() { metricsWatchInterval = orig }) done := make(chan error, 1) - go func() { done <- cmdFleet([]string{"metrics", "--watch"}) }() + go func() { done <- cmdMetrics([]string{"--watch"}) }() // Let it draw at least twice, so the clear-and-redraw path runs. time.Sleep(200 * time.Millisecond) diff --git a/cmd/spinloop/last_active_test.go b/cmd/spinloop/last_active_test.go index 6328df27..2133fe63 100644 --- a/cmd/spinloop/last_active_test.go +++ b/cmd/spinloop/last_active_test.go @@ -286,7 +286,7 @@ func TestFleetMetricsShowsLastActive(t *testing.T) { for _, format := range []string{"bar", "table"} { t.Run(format, func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics", "--format=" + format}); err != nil { + if err := cmdMetrics([]string{"--format=" + format}); err != nil { t.Fatalf("cmdFleet metrics: %v", err) } }) @@ -308,7 +308,7 @@ func TestFleetMetricsJSONCarriesLastActive(t *testing.T) { }) out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics", "--format=json"}); err != nil { + if err := cmdMetrics([]string{"--format=json"}); err != nil { t.Fatalf("cmdFleet metrics: %v", err) } }) @@ -337,7 +337,7 @@ func TestFleetMetricsOmitsLastActiveWithoutActivity(t *testing.T) { }) out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics"}); err != nil { + if err := cmdMetrics(nil); err != nil { t.Fatalf("cmdFleet metrics: %v", err) } }) @@ -356,7 +356,7 @@ func TestFleetMetricsStoppedNodeStillShowsLastActive(t *testing.T) { }) out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics"}); err != nil { + if err := cmdMetrics(nil); err != nil { t.Fatalf("cmdFleet metrics: %v", err) } }) diff --git a/cmd/spinloop/logs.go b/cmd/spinloop/logs.go new file mode 100644 index 00000000..358c66c7 --- /dev/null +++ b/cmd/spinloop/logs.go @@ -0,0 +1,92 @@ +// `spinloop logs`: what every engine in the target has said. The target is +// whatever names one — a registered environment, a fleet file, or the fleet +// file in the working directory. Reading and following live in fleet_logs.go; +// this is the command. + +package main + +import ( + "fmt" + "time" + + "github.com/spf13/cobra" + "github.com/spinloop-ai/spinloop/internal/fleet" + "github.com/spinloop-ai/spinloop/internal/remote" +) + +func logsCmd() *cobra.Command { + var ( + path string + envName string + follow bool + limit int + format string + source string + since time.Duration + instance string + ) + const followUsage = "keep printing new output as it arrives" + c := &cobra.Command{ + Use: "logs", + Short: "read the engines' output", + Long: `reads what each engine in the target has said, through whatever the node +answers with — a daemon's log file, or a cloud environment's log store. +Naming a node reads only that one. Nodes are read concurrently. + +The target is a registered environment (--env), a fleet file (--fleet), or +the fleet.yaml in the working directory. --fleet has no -f here: that is +--follow's short form, and a flag cannot carry two meanings on one command +line. + +--source, --since and --instance narrow a log that can be queried. Only a +cloud environment's can — a daemon's log is one file on one machine, where +none of the three narrows anything — so a node that cannot be narrowed is +read as it would be without them.`, + Args: cobra.MaximumNArgs(1), + SilenceErrors: true, + SilenceUsage: true, + RunE: func(c *cobra.Command, args []string) error { + resolve(c) + q := fleet.LogQuery{Source: source, Since: since, Instance: instance} + return runLogs(fleetTarget{envName: envName, fleetPath: path}, q, follow, limit, format, args) + }, + } + fs := c.Flags() + // --fleet takes no short form here: -f is already --follow, as it is on + // every other surface that follows something. + fs.StringVar(&path, "fleet", "", fleetFileUsage) + fs.StringVar(&envName, "env", "", envFlagTargetUsage) + fs.BoolVarP(&follow, "follow", "f", false, followUsage) + fs.IntVar(&limit, "limit", 200, "lines of backlog to print per node") + fs.StringVar(&format, "format", "text", "output format: text (default) or json") + fs.StringVar(&source, "source", "", "which log to read on a node that has more than one: engine (default), boot or all") + fs.DurationVar(&since, "since", 0, "how far back to read on a node whose log can be queried (30m, 2h)") + fs.StringVar(&instance, "instance", "", "restrict to one instance id, on a node whose log holds more than one") + c.ValidArgsFunction = noPositionals + compRegister(c, "fleet", compFiles) + compRegister(c, "env", compEnvs) + return c +} + +// cmdLogs runs the command through the tree — the seam the suite calls. +func cmdLogs(args []string) error { return execCmd(logsCmd(), args) } + +// validateLogQuery rejects the query values that cannot mean anything, before +// any node is contacted. +func validateLogQuery(q fleet.LogQuery, format string, limit int) error { + switch q.Source { + case "", remote.LogSourceEngine, remote.LogSourceBoot, remote.LogSourceAll: + default: + return fmt.Errorf("--source must be engine, boot or all, got %q", q.Source) + } + if q.Since < 0 { + return fmt.Errorf("--since must be positive, got %s", q.Since) + } + if format != "text" && format != "json" { + return fmt.Errorf("--format must be \"text\" or \"json\", got %q", format) + } + if limit <= 0 { + return fmt.Errorf("--limit must be positive, got %d", limit) + } + return nil +} diff --git a/cmd/spinloop/metrics.go b/cmd/spinloop/metrics.go new file mode 100644 index 00000000..ac42fe3d --- /dev/null +++ b/cmd/spinloop/metrics.go @@ -0,0 +1,78 @@ +// `spinloop metrics`: what every engine in the target is doing with its +// hardware. The target is whatever names one — a registered environment, a +// fleet file, or the fleet file in the working directory — so an environment +// and a fleet holding it are read by the one command. The formats live beside +// the fleet's other views in fleet.go and metrics_render.go; this is the +// command. + +package main + +import ( + "context" + "os" + + "github.com/spf13/cobra" + "github.com/spinloop-ai/spinloop/internal/fleet" +) + +func metricsCmd() *cobra.Command { + var ( + path string + envName string + format string + watch bool + withCost bool + ) + c := &cobra.Command{ + Use: "metrics", + Short: "sample every engine's metrics", + Long: `samples what each engine in the target is doing with its hardware: its +resource use, its token and request counters, and the release the node is +running. One block per node, queried concurrently. + +The target is a registered environment (--env), a fleet file (--fleet), or +the fleet.yaml in the working directory. A node that cannot be reached is +reported rather than omitted, and never fails the command. + +--cost adds what a node has spent on its running session, priced from the +instance type it launched as. Only a cloud environment can be priced — a +machine you already own has no hourly rate — so nodes that cannot be read +as they are without the flag, and a target with none succeeds showing no +cost at all.`, + Args: cobra.NoArgs, + SilenceErrors: true, + SilenceUsage: true, + RunE: func(c *cobra.Command, _ []string) error { + resolve(c) + if err := validateMetricsFormat(format); err != nil { + return err + } + cfg, err := resolveFleetTarget(fleetTarget{envName: envName, fleetPath: path}) + if err != nil { + return err + } + call := fleet.MetricsCall + if withCost { + call = fleet.PricedMetricsCall + } + if watch { + return runFleetMetricsWatch(cfg, format, call) + } + results := cfg.FanOut(context.Background(), call) + return renderFleetMetrics(os.Stdout, results, format) + }, + } + fs := c.Flags() + fs.StringVarP(&path, "fleet", "f", "", fleetFileUsage) + fs.StringVar(&envName, "env", "", envFlagTargetUsage) + fs.StringVar(&format, "format", "gauge", "output format: gauge (default), bar, table or json") + fs.BoolVarP(&watch, "watch", "w", false, "redraw every 60 seconds") + fs.BoolVar(&withCost, "cost", false, "include what each priceable node has cost so far") + c.ValidArgsFunction = noPositionals + compRegister(c, "fleet", compFiles) + compRegister(c, "env", compEnvs) + return c +} + +// cmdMetrics runs the command through the tree — the seam the suite calls. +func cmdMetrics(args []string) error { return execCmd(metricsCmd(), args) } diff --git a/cmd/spinloop/retain_render_test.go b/cmd/spinloop/retain_render_test.go index 2e482a87..0ad97d49 100644 --- a/cmd/spinloop/retain_render_test.go +++ b/cmd/spinloop/retain_render_test.go @@ -203,7 +203,7 @@ func TestFleetMetricsShowsKeep(t *testing.T) { }) out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics"}); err != nil { + if err := cmdMetrics(nil); err != nil { t.Fatalf("cmdFleet metrics: %v", err) } }) @@ -225,7 +225,7 @@ func TestFleetMetricsOmitsKeepWhenAbsent(t *testing.T) { }) out := captureStdout(t, func() { - if err := cmdFleet([]string{"metrics"}); err != nil { + if err := cmdMetrics(nil); err != nil { t.Fatalf("cmdFleet metrics: %v", err) } }) diff --git a/cmd/spinloop/target_test.go b/cmd/spinloop/target_test.go index c948bc99..586cb141 100644 --- a/cmd/spinloop/target_test.go +++ b/cmd/spinloop/target_test.go @@ -197,7 +197,7 @@ func TestFleetCommandsCompleteEnv(t *testing.T) { }) t.Chdir(t.TempDir()) - for _, sub := range []string{"metrics", "logs", "start", "stop", "deploy", "route"} { + for _, sub := range []string{"start", "stop", "deploy", "route"} { t.Run(sub, func(t *testing.T) { got, _ := complete(t, "fleet", sub, "--env", "") want := map[string]bool{"prod": false, "staging": false} diff --git a/internal/fleet/fanout.go b/internal/fleet/fanout.go index 5039cba3..4ed87608 100644 --- a/internal/fleet/fanout.go +++ b/internal/fleet/fanout.go @@ -19,11 +19,34 @@ func StatusCall(ctx context.Context, n Node) NodeResult { return r } -// MetricsCall reads a node's engine and system metrics. +// MetricsCall reads a node's engine and system metrics, and the release it +// reports alongside them where its kind reports one there. The version costs +// nothing: a node that answers it does so from the reading just taken. func MetricsCall(ctx context.Context, n Node) NodeResult { stats, err := n.Metrics(ctx) r := result(n.Name(), err) r.Metrics = stats + if v, ok := n.(Versioner); ok { + r.Version = v.Version() + } + return r +} + +// PricedMetricsCall is MetricsCall plus what the node has cost, for a caller +// that asked. A node that cannot be priced is read exactly as MetricsCall +// reads it, and a price that cannot be fetched leaves the cost unreported: +// asking a fleet what it costs is not a command a fleet of unpriceable nodes +// should refuse. +func PricedMetricsCall(ctx context.Context, n Node) NodeResult { + r := MetricsCall(ctx, n) + if !r.OK() { + return r + } + if c, ok := n.(Coster); ok { + if cost, err := c.Cost(ctx); err == nil { + r.Cost = cost + } + } return r } @@ -44,6 +67,31 @@ func LogsCall(offsets map[string]int64, limit int) Call { } } +// QueriedLogsCall reads each node's log narrowed by what the caller asked for, +// where the node's kind can narrow it. A node whose log is a byte offset into +// one file cannot, and is read exactly as LogsCall reads it — so a query +// against a fleet of such nodes returns their output rather than nothing. +// +// limit doubles as the byte budget for a node read by offset and the event cap +// for a node read by query, because it means the same thing to a caller either +// way: how much to bring back. +func QueriedLogsCall(q LogQuery, limit int) Call { + return func(ctx context.Context, n Node) NodeResult { + if s, ok := n.(SourceLogger); ok { + query := q + query.Limit = limit + logs, err := s.LogsMatching(ctx, query) + r := result(n.Name(), err) + r.Logs = logs + return r + } + logs, err := n.Logs(ctx, daemon.TailLog, limit) + r := result(n.Name(), err) + r.Logs = logs + return r + } +} + // fanOutEach runs one result producer per position concurrently and returns one // result per position, in the order given, so the rendering is stable between // refreshes. It never returns an error: a producer that fails is a typed diff --git a/internal/fleet/node.go b/internal/fleet/node.go index c9f13730..4a639e06 100644 --- a/internal/fleet/node.go +++ b/internal/fleet/node.go @@ -227,6 +227,16 @@ type NodeResult struct { Metrics metrics.Stats Logs daemon.LogsResponse + // Cost is what this node has spent on its running session. Filled only by + // a call that asked for it, and only for a node that can be priced; see + // Cost.Reported for the difference between "nothing" and "no figure". + Cost Cost + // Version is the spinloop release this node reported somewhere other than + // its status reply. Empty for a node that carries it there instead — the + // status views read it from the status, and this is the metrics views' + // equivalent. + Version string + // At is when this reading was taken — set by the fan-out as the call // returns. Reads are concurrent and of uneven duration, so a reading can // land after one taken later than it; a caller that draws the newest diff --git a/openspec/changes/top-level-metrics-and-logs/tasks.md b/openspec/changes/top-level-metrics-and-logs/tasks.md index c659b877..526e40e6 100644 --- a/openspec/changes/top-level-metrics-and-logs/tasks.md +++ b/openspec/changes/top-level-metrics-and-logs/tasks.md @@ -17,29 +17,29 @@ ## 2. The verbs -- [ ] 2.1 Add `cmd/spinloop/metrics.go`: a root-registered `metrics` taking +- [x] 2.1 Add `cmd/spinloop/metrics.go`: a root-registered `metrics` taking `--env`/`--fleet`, `--format`, `--watch` and `--cost`, resolving through `resolveFleetTarget` and rendering through the existing formatters. Verify each format renders for a fleet and for a single environment. -- [ ] 2.2 Add `cmd/spinloop/logs.go` the same way, with `--follow`, `--limit`, +- [x] 2.2 Add `cmd/spinloop/logs.go` the same way, with `--follow`, `--limit`, `--format`, and the `--source`/`--since`/`--instance` the capability answers. Verify `-f` remains `--follow` and the fleet file is long-form only. -- [ ] 2.3 Wire `--cost` to `Coster`: priced nodes carry the figure, the rest +- [x] 2.3 Wire `--cost` to `Coster`: priced nodes carry the figure, the rest render as they would without the flag, and a target with none succeeds (design D2). Verify with a mixed target and an all-daemon one. -- [ ] 2.4 Wire `--source`/`--since`/`--instance` to `SourceLogger` on the same +- [x] 2.4 Wire `--source`/`--since`/`--instance` to `SourceLogger` on the same terms. Verify a mixed target reads the environment's boot log and the daemon's ordinary output. - [ ] 2.5 Read a Spinloop given to either verb for its `ENV` instructions and adjacent `.env` only, never to select a target (design D4). Verify a Spinloop whose `ENV` supplies `SPINLOOP_REMOTE_*` configures the command. -- [ ] 2.6 Register both at the root with the completion the fleet spellings +- [x] 2.6 Register both at the root with the completion the fleet spellings had. Verify the completion test covers them. ## 3. Removing the old spellings -- [ ] 3.1 Delete `fleetMetricsCmd` and `fleetLogsCmd` and unregister them, +- [x] 3.1 Delete `fleetMetricsCmd` and `fleetLogsCmd` and unregister them, keeping their renderers. Verify `spinloop fleet --help` lists neither. - [ ] 3.2 Delete `remoteStatusCmd`, `remoteMetricsCmd`, `remoteLogsCmd` and their bodies, keeping the formatters the top-level verbs use. Verify From f8ede367f8790111135eb662165698b0c33ec42b Mon Sep 17 00:00:00 2001 From: spinloop-agent Date: Sat, 19 Sep 2026 20:36:20 +0100 Subject: [PATCH 3/4] fix: stop the metrics watch loop hanging on a dead single-node target, 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" --- README.md | 10 +- cmd/spinloop/commands.go | 8 +- cmd/spinloop/dashboard.go | 6 +- cmd/spinloop/fleet.go | 116 +++++- cmd/spinloop/last_active_test.go | 77 ++-- cmd/spinloop/logs.go | 23 +- cmd/spinloop/metrics.go | 15 +- cmd/spinloop/metrics_render_test.go | 49 ++- cmd/spinloop/read_spinloop_env.go | 37 ++ cmd/spinloop/read_spinloop_env_test.go | 97 +++++ cmd/spinloop/remote.go | 325 ---------------- cmd/spinloop/remote_environments_test.go | 10 +- cmd/spinloop/remote_logs.go | 235 ------------ cmd/spinloop/remote_logs_test.go | 349 ++---------------- cmd/spinloop/remote_test.go | 188 +++++----- cmd/spinloop/retain_render_test.go | 20 +- cmd/spinloop/root_dispatch_test.go | 21 ++ cmd/spinloop/status.go | 6 +- cmd/spinloop/status_render.go | 19 + docs/commands/alias.md | 2 +- docs/commands/dashboard.md | 5 +- docs/commands/fleet.md | 28 +- docs/commands/gateway.md | 2 +- docs/commands/index.md | 2 + docs/commands/logs.md | 57 +++ docs/commands/metrics.md | 87 +++++ docs/commands/remote.md | 20 +- docs/commands/status.md | 24 +- docs/maintainer/internals.md | 4 +- examples/fleet-docker/README.md | 2 +- examples/fleet-docker/compose.yaml | 2 +- examples/fleet-docker/run-tests.sh | 35 +- examples/fleet-mixed/README.md | 2 +- examples/fleet-mixed/fleet.yaml | 2 +- examples/fleet-remote/README.md | 2 +- examples/fleet-remote/fleet.yaml | 2 +- examples/fleet/README.md | 2 +- examples/fleet/fleet.yaml | 2 +- examples/gateway-docker/run-tests.sh | 29 +- examples/llamacpp/qwen3.8-27b/README.md | 2 +- internal/fleet/capabilities_test.go | 27 +- internal/fleet/capability_calls_test.go | 191 ++++++++++ internal/fleet/fanout.go | 15 +- internal/fleet/node.go | 44 ++- internal/fleet/remote_node.go | 59 ++- .../top-level-metrics-and-logs/design.md | 36 +- .../top-level-metrics-and-logs/tasks.md | 23 +- 47 files changed, 1156 insertions(+), 1163 deletions(-) create mode 100644 cmd/spinloop/read_spinloop_env.go create mode 100644 cmd/spinloop/read_spinloop_env_test.go delete mode 100644 cmd/spinloop/remote_logs.go create mode 100644 docs/commands/logs.md create mode 100644 docs/commands/metrics.md create mode 100644 internal/fleet/capability_calls_test.go diff --git a/README.md b/README.md index 4b5b7827..5fbeef99 100644 --- a/README.md +++ b/README.md @@ -548,13 +548,13 @@ spinloop fleet start gpu-box # start one node's engine `dashboard` is the fleet you actually look at, and the board [at the top of this page](#2-on-every-machine-you-own) is a real one: one tile per -node, repainted in place, showing the same numbers `fleet metrics` prints — +node, repainted in place, showing the same numbers `metrics` prints — start a node with `s`, stop one with `x`, and a waking cloud machine shows its progress on its own tile; `a` lets you stop watching one that is still waking — it carries on in the cloud. Press `` on a tile for a full-screen view of that node — metrics, its engine log tailed live, and the keys that work there — `` to go back. -`fleet metrics --watch` is the same board as a stream, for pipes. +`metrics --watch` is the same board as a stream, for pipes. ``` NODE STATE SERVING @@ -625,11 +625,11 @@ while you are using it, and stops itself after a period of idleness. spinloop remote start --env dev-2 --print-env # boot the instance, wait for the # model to load, then print OPENAI_BASE_URL / # OPENAI_API_KEY exports for eval -spinloop remote status --env dev-2 # instance state, endpoint health, +spinloop status --env --env dev-2 # instance state, endpoint health, # and when it last did any work -spinloop remote metrics --env dev-2 # tokens, GPU, CPU and RAM — plus +spinloop metrics --env --env dev-2 # tokens, GPU, CPU and RAM — plus # the same last-active -spinloop remote logs --env dev-2 # what the engine (or the boot) +spinloop logs --env --env dev-2 # what the engine (or the boot) # said, even after it's gone spinloop remote pause --env dev-2 # stop now, but keep it re-wakeable spinloop remote restart --env dev-2 # fresh engine, same address: stop diff --git a/cmd/spinloop/commands.go b/cmd/spinloop/commands.go index d6d3284a..b2de2ebb 100644 --- a/cmd/spinloop/commands.go +++ b/cmd/spinloop/commands.go @@ -351,6 +351,11 @@ var movedSubcommands = map[string]string{ "fleet harness": "code --fleet ", "fleet status": "status", "fleet dashboard": "dashboard", + "fleet metrics": "metrics", + "fleet logs": "logs", + "remote status": "status --env ", + "remote metrics": "metrics --env ", + "remote logs": "logs --env ", } // fleetCmd builds the fleet parent and its subcommands. The parent does @@ -404,9 +409,6 @@ names a file — falling back to the default environment. Each subcommand's remotePauseCmd(), remoteRestartCmd(), remoteStopCmd(), - remoteStatusCmd(), - remoteMetricsCmd(), - remoteLogsCmd(), remoteDeployCmd(), remoteSeedCmd(), remoteEnvCmd(), diff --git a/cmd/spinloop/dashboard.go b/cmd/spinloop/dashboard.go index 9973d3ed..ce90a286 100644 --- a/cmd/spinloop/dashboard.go +++ b/cmd/spinloop/dashboard.go @@ -11,7 +11,7 @@ import ( ) func dashboardCmd() *cobra.Command { - var path, envName string + var path, envName, spinloopPath string c := &cobra.Command{ Use: "dashboard", Short: "watch the engines in an interactive tiled view", @@ -39,12 +39,16 @@ metrics --watch instead.`, SilenceUsage: true, RunE: func(c *cobra.Command, _ []string) error { resolve(c) + if err := applyReadSpinloopEnv(spinloopPath); err != nil { + return err + } return runFleetDashboard(fleetTarget{envName: envName, fleetPath: path}) }, } fs := c.Flags() fs.StringVarP(&path, "fleet", "f", "", fleetFileUsage) fs.StringVar(&envName, "env", "", envFlagTargetUsage) + registerSpinloopEnvFlag(fs, &spinloopPath) c.ValidArgsFunction = noPositionals compRegister(c, "fleet", compFiles) compRegister(c, "env", compEnvs) diff --git a/cmd/spinloop/fleet.go b/cmd/spinloop/fleet.go index 7c1d4e5b..feca6765 100644 --- a/cmd/spinloop/fleet.go +++ b/cmd/spinloop/fleet.go @@ -62,15 +62,24 @@ func fleetRow(r fleet.NodeResult) (state, serving string) { } // The shared facts come from the same source the remote status view reads, // so the two cannot word or compute them differently. + // A node that runs on an instance reports its release outside the status + // reply, so the instance's answer fills what the reply left empty rather + // than overriding a daemon that carries its own. + version := r.Status.Version + if version == "" { + version = r.Instance.Version + } f := statusFact{ State: r.Status.State, Model: r.Status.Model, Runner: r.Status.Runner, - Version: r.Status.Version, + Version: version, UptimeSeconds: r.Status.UptimeSeconds, LastActiveAt: r.Status.LastActiveAt, IdleSeconds: r.Status.IdleSeconds, Ready: r.Status.Ready, + Endpoint: r.Instance.BaseURL, + RetainUntil: r.Instance.RetainUntil, } return f.State, f.servingText() } @@ -97,6 +106,15 @@ func runFleetMetricsWatch(cfg *fleet.Config, format string, call fleet.Call) err if ctx.Err() != nil { return nil } + // A single-node target (an --env watch, or a fleet of one) with + // nothing else to show for the poll ends the watch rather than + // redrawing the same failure forever: this is the one node the + // caller asked about, and it could not be read. A multi-node fleet + // keeps drawing through a bad node so the rest of the fleet stays + // visible. + if len(results) == 1 && !results[0].OK() { + return fmt.Errorf("%s: %s", results[0].Name, results[0].Detail()) + } if err := renderFleetMetrics(&buf, results, format); err != nil { return err } @@ -131,16 +149,19 @@ func renderFleetMetrics(w io.Writer, results []fleet.NodeResult, format string) continue } stats := r.Metrics - fmt.Fprintf(w, "%s %s", r.Name, stats.State) - if stats.ModelID != "" { - fmt.Fprintf(w, " %s", stats.ModelID) - } - fmt.Fprintln(w) + renderMetricsHeader(w, r, format) // Before the continue, for the same reason the remote formats show it // before theirs: a node whose engine has stopped still has a useful // answer to "when did it last do anything?" — and, for a retained // remote environment, "how long is it kept?". - renderActiveIndented(w, stats.LastActiveAt, stats.IdleSeconds, stats.RetainUntil, now) + // The table spells its facts as key-value lines, so its active line is + // spelled that way too; the compact formats indent theirs under the + // header. + if format == "table" { + renderActiveKeyValue(w, stats.LastActiveAt, stats.IdleSeconds, retainUntilOf(r), now) + } else { + renderActiveIndented(w, stats.LastActiveAt, stats.IdleSeconds, retainUntilOf(r), now) + } switch format { case "bar": // No state gate, for the same reason the remote bar format has @@ -163,11 +184,71 @@ func renderFleetMetrics(w io.Writer, results []fleet.NodeResult, format string) renderGPUTable(w, stats.GPUs) renderCPUMemTable(w, stats.CPU, stats.Memory) } + renderCost(w, r.Cost) renderCollectionErrors(os.Stderr, stats.Errors) } return nil } +// renderMetricsHeader draws the lines a node's figures open with. The table +// format spells each fact on its own line, as the environment-only format did; +// the compact formats put them on one line, as its bar and gauge did. Between +// them they carry every fact those formats carried: what the node is, what it +// runs on, what it serves, the release on it, and how long it has been up. +func renderMetricsHeader(w io.Writer, r fleet.NodeResult, format string) { + stats := r.Metrics + if format == "table" { + fmt.Fprintf(w, "node: %s\n", r.Name) + fmt.Fprintf(w, "state: %s\n", stats.State) + for _, line := range []struct{ label, value string }{ + {"instance", r.Instance.ID}, + {"instanceType", r.Instance.Type}, + {"runner", stats.Runner}, + {"model", stats.ModelID}, + {"version", r.Instance.Version}, + {"endpoint", r.Instance.BaseURL}, + } { + if line.value != "" { + fmt.Fprintf(w, "%-13s %s\n", line.label+":", line.value) + } + } + if stats.UptimeSeconds > 0 { + fmt.Fprintf(w, "%-13s %s\n", "uptime:", formatDuration(stats.UptimeSeconds)) + } + return + } + fmt.Fprintf(w, "%s %s", r.Name, stats.State) + for _, v := range []string{r.Instance.Type, stats.ModelID, r.Instance.Version} { + if v != "" { + fmt.Fprintf(w, " %s", v) + } + } + if stats.UptimeSeconds > 0 { + fmt.Fprintf(w, " (up %s)", formatDuration(stats.UptimeSeconds)) + } + fmt.Fprintln(w) +} + +// retainUntilOf is a node's retention deadline from whichever answer carried +// it: the shared stats where a reading has been taken, the instance the node +// describes otherwise. One fact, two replies, so a caller never has to pick. +func retainUntilOf(r fleet.NodeResult) string { + if r.Metrics.RetainUntil != "" { + return r.Metrics.RetainUntil + } + return r.Instance.RetainUntil +} + +// renderCost draws what a node has cost, where the caller asked and the node +// could be priced. A node with no figure draws nothing: a zero would claim it +// cost nothing. +func renderCost(w io.Writer, c fleet.Cost) { + if !c.Reported() { + return + } + fmt.Fprintf(w, " cost so far: $%.2f (%.4f/hr)\n", c.SoFar, c.PerHour) +} + // fleetNodeJSON is one node in the JSON output: its metrics when it answered, // its outcome and reason when it did not — so a consumer sees the whole fleet // rather than silently missing the nodes that were down. @@ -176,16 +257,35 @@ type fleetNodeJSON struct { Outcome string `json:"outcome"` Error string `json:"error,omitempty"` Metrics *any `json:"metrics,omitempty"` + // The instance facts and the cost are what the node's kind could say + // beyond the shared engine figures. Omitted for a node that said none, + // which is every node that is a machine rather than an instance. + Instance string `json:"instance,omitempty"` + InstanceType string `json:"instanceType,omitempty"` + Version string `json:"version,omitempty"` + BaseURL string `json:"baseUrl,omitempty"` + RetainUntil string `json:"retainUntil,omitempty"` + Cost *float64 `json:"cost,omitempty"` + CostPerHour *float64 `json:"costPerHour,omitempty"` } func renderFleetMetricsJSON(w io.Writer, results []fleet.NodeResult) error { out := make([]fleetNodeJSON, 0, len(results)) for _, r := range results { - entry := fleetNodeJSON{Node: r.Name, Outcome: string(r.Outcome), Error: r.Detail()} + entry := fleetNodeJSON{ + Node: r.Name, Outcome: string(r.Outcome), Error: r.Detail(), + Instance: r.Instance.ID, InstanceType: r.Instance.Type, + Version: r.Instance.Version, BaseURL: r.Instance.BaseURL, + RetainUntil: r.Instance.RetainUntil, + } if r.OK() { var m any = r.Metrics entry.Metrics = &m } + if r.Cost.Reported() { + soFar, perHour := r.Cost.SoFar, r.Cost.PerHour + entry.Cost, entry.CostPerHour = &soFar, &perHour + } out = append(out, entry) } data, err := json.MarshalIndent(out, "", " ") diff --git a/cmd/spinloop/last_active_test.go b/cmd/spinloop/last_active_test.go index 2133fe63..079f4490 100644 --- a/cmd/spinloop/last_active_test.go +++ b/cmd/spinloop/last_active_test.go @@ -48,8 +48,8 @@ func TestRemoteMetricsBarShowsLastActive(t *testing.T) { statsServer(t, runningWithActivity) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=bar"}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=bar"}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if !strings.Contains(out, "active 2m 5s ago") { @@ -69,8 +69,8 @@ func TestRemoteMetricsTableShowsLastActive(t *testing.T) { statsServer(t, runningWithActivity) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=table"}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=table"}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) // Padded to the same key column as its neighbours, and beside uptime. @@ -86,17 +86,25 @@ func TestRemoteMetricsJSONCarriesLastActive(t *testing.T) { statsServer(t, runningWithActivity) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=json"}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=json"}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) - var decoded struct { - LastActiveAt string `json:"lastActiveAt"` - IdleSeconds int `json:"idleSeconds"` + // One object per node, since the one command reads a fleet as readily as + // one environment. + var nodes []struct { + Metrics struct { + LastActiveAt string `json:"lastActiveAt"` + IdleSeconds int `json:"idleSeconds"` + } `json:"metrics"` } - if err := json.Unmarshal([]byte(out), &decoded); err != nil { + if err := json.Unmarshal([]byte(out), &nodes); err != nil { t.Fatalf("output is not valid JSON: %v\n%s", err, out) } + if len(nodes) != 1 { + t.Fatalf("nodes = %d, want 1:\n%s", len(nodes), out) + } + decoded := nodes[0].Metrics // Unformatted: a consumer wanting a duration has the seconds, one wanting // the fact has the timestamp. if decoded.LastActiveAt != "2026-08-10T10:00:00Z" || decoded.IdleSeconds != 125 { @@ -124,8 +132,8 @@ func TestRemoteMetricsStoppedStillShowsLastActive(t *testing.T) { } { t.Run(format, func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=" + format}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=" + format}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if !strings.Contains(out, want) { @@ -154,8 +162,8 @@ func TestLastActiveZeroIdleStillRenders(t *testing.T) { } { t.Run(format, func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=" + format}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=" + format}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if !strings.Contains(out, want) { @@ -176,8 +184,8 @@ func TestLastActiveOmittedWithoutATimestamp(t *testing.T) { for _, format := range []string{"bar", "table"} { t.Run(format, func(t *testing.T) { out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=" + format}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=" + format}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) // No line at all, rather than one implying it has sat unused. @@ -215,30 +223,35 @@ func TestRemoteStatusShowsLastActive(t *testing.T) { }`) out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Fatalf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Fatalf("cmdStatus: %v", err) } }) - if !strings.Contains(out, "active: 2m 5s ago") { + if !strings.Contains(out, "(active 2m 5s ago)") { t.Errorf("status missing the active line:\n%s", out) } - // The lines it already printed are untouched. - for _, want := range []string{"state: running", "healthy: true", "base_url: http://198.51.100.7:8000"} { + // Every fact the environment-only status printed is still here: the state, + // the address, and — for a healthy endpoint — no not-ready mark, which is + // how health reads on a row that serves both node kinds. + for _, want := range []string{"running", "http://198.51.100.7:8000"} { if !strings.Contains(out, want) { t.Errorf("status output missing %q:\n%s", want, out) } } + if strings.Contains(out, "not ready") { + t.Errorf("a healthy endpoint should carry no readiness mark:\n%s", out) + } } func TestRemoteStatusZeroIdleStillRenders(t *testing.T) { statusServer(t, `{"state": "running", "healthy": true, "lastActiveAt": "2026-08-10T10:00:00Z"}`) out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Fatalf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Fatalf("cmdStatus: %v", err) } }) - if !strings.Contains(out, "active: 0s ago") { + if !strings.Contains(out, "(active 0s ago)") { t.Errorf("status hid an endpoint that is working right now:\n%s", out) } } @@ -249,14 +262,14 @@ func TestRemoteStatusOmitsLastActiveWhenAbsent(t *testing.T) { statusServer(t, `{"state": "stopped", "healthy": false}`) out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Fatalf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Fatalf("cmdStatus: %v", err) } }) if strings.Contains(out, "active") { t.Errorf("status invented activity for a stopped instance:\n%s", out) } - if !strings.Contains(out, "state: stopped") { + if !strings.Contains(out, "stopped") { t.Errorf("status lost the rest of the report:\n%s", out) } } @@ -290,8 +303,14 @@ func TestFleetMetricsShowsLastActive(t *testing.T) { t.Fatalf("cmdFleet metrics: %v", err) } }) - if !strings.Contains(out, "active 2m 5s ago") { - t.Errorf("fleet %s metrics missing the active line:\n%s", format, out) + // bar indents its active line under the header; table spells it + // as a key-value, like the facts around it. + want := "active 2m 5s ago" + if format == "table" { + want = "active: 2m 5s ago" + } + if !strings.Contains(out, want) { + t.Errorf("%s metrics missing the active line:\n%s", format, out) } }) } diff --git a/cmd/spinloop/logs.go b/cmd/spinloop/logs.go index 358c66c7..3172d98a 100644 --- a/cmd/spinloop/logs.go +++ b/cmd/spinloop/logs.go @@ -16,14 +16,15 @@ import ( func logsCmd() *cobra.Command { var ( - path string - envName string - follow bool - limit int - format string - source string - since time.Duration - instance string + path string + envName string + follow bool + limit int + format string + source string + since time.Duration + instance string + spinloopPath string ) const followUsage = "keep printing new output as it arrives" c := &cobra.Command{ @@ -47,6 +48,9 @@ read as it would be without them.`, SilenceUsage: true, RunE: func(c *cobra.Command, args []string) error { resolve(c) + if err := applyReadSpinloopEnv(spinloopPath); err != nil { + return err + } q := fleet.LogQuery{Source: source, Since: since, Instance: instance} return runLogs(fleetTarget{envName: envName, fleetPath: path}, q, follow, limit, format, args) }, @@ -56,11 +60,12 @@ read as it would be without them.`, // every other surface that follows something. fs.StringVar(&path, "fleet", "", fleetFileUsage) fs.StringVar(&envName, "env", "", envFlagTargetUsage) + registerSpinloopEnvFlag(fs, &spinloopPath) fs.BoolVarP(&follow, "follow", "f", false, followUsage) fs.IntVar(&limit, "limit", 200, "lines of backlog to print per node") fs.StringVar(&format, "format", "text", "output format: text (default) or json") fs.StringVar(&source, "source", "", "which log to read on a node that has more than one: engine (default), boot or all") - fs.DurationVar(&since, "since", 0, "how far back to read on a node whose log can be queried (30m, 2h)") + fs.DurationVar(&since, "since", time.Hour, "how far back to read on a node whose log can be queried (30m, 2h)") fs.StringVar(&instance, "instance", "", "restrict to one instance id, on a node whose log holds more than one") c.ValidArgsFunction = noPositionals compRegister(c, "fleet", compFiles) diff --git a/cmd/spinloop/metrics.go b/cmd/spinloop/metrics.go index ac42fe3d..f78fb10f 100644 --- a/cmd/spinloop/metrics.go +++ b/cmd/spinloop/metrics.go @@ -17,11 +17,12 @@ import ( func metricsCmd() *cobra.Command { var ( - path string - envName string - format string - watch bool - withCost bool + path string + envName string + format string + spinloopPath string + watch bool + withCost bool ) c := &cobra.Command{ Use: "metrics", @@ -44,6 +45,9 @@ cost at all.`, SilenceUsage: true, RunE: func(c *cobra.Command, _ []string) error { resolve(c) + if err := applyReadSpinloopEnv(spinloopPath); err != nil { + return err + } if err := validateMetricsFormat(format); err != nil { return err } @@ -65,6 +69,7 @@ cost at all.`, fs := c.Flags() fs.StringVarP(&path, "fleet", "f", "", fleetFileUsage) fs.StringVar(&envName, "env", "", envFlagTargetUsage) + registerSpinloopEnvFlag(fs, &spinloopPath) fs.StringVar(&format, "format", "gauge", "output format: gauge (default), bar, table or json") fs.BoolVarP(&watch, "watch", "w", false, "redraw every 60 seconds") fs.BoolVar(&withCost, "cost", false, "include what each priceable node has cost so far") diff --git a/cmd/spinloop/metrics_render_test.go b/cmd/spinloop/metrics_render_test.go index cbe384c2..65711cf9 100644 --- a/cmd/spinloop/metrics_render_test.go +++ b/cmd/spinloop/metrics_render_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/charmbracelet/lipgloss" + "github.com/spinloop-ai/spinloop/internal/fleet" "github.com/spinloop-ai/spinloop/internal/metrics" "github.com/spinloop-ai/spinloop/internal/remote" ) @@ -441,16 +442,19 @@ func TestRenderStatCombinedStoppedEngineDrawsHistoryAlone(t *testing.T) { } func TestFormatMetricsBarStoppedWithHistory(t *testing.T) { - resp := &remote.StatsResponse{ - Environment: "prod", State: "stopped", ModelID: "org/qwen:q4", - LastActiveAt: "2026-08-21T10:00:00Z", IdleSeconds: 12, - History: []metrics.HistorySample{ - {Time: 1, CPU: ptrPct(10)}, - {Time: 2, CPU: ptrPct(20)}, + results := []fleet.NodeResult{{ + Name: "prod", Outcome: fleet.OutcomeOK, + Metrics: metrics.Stats{ + State: "stopped", ModelID: "org/qwen:q4", + LastActiveAt: "2026-08-21T10:00:00Z", IdleSeconds: 12, + History: []metrics.HistorySample{ + {Time: 1, CPU: ptrPct(10)}, + {Time: 2, CPU: ptrPct(20)}, + }, }, - } + }} var b bytes.Buffer - if err := formatMetricsBar(resp, remote.Config{}, &b); err != nil { + if err := renderFleetMetrics(&b, results, "bar"); err != nil { t.Fatal(err) } want := "prod stopped org/qwen:q4\n" + @@ -463,7 +467,7 @@ func TestFormatMetricsBarStoppedWithHistory(t *testing.T) { // The gauge format draws no series for a stopped endpoint: the header and // the active line, and nothing after. b.Reset() - if err := formatMetricsGauge(resp, remote.Config{}, &b); err != nil { + if err := renderFleetMetrics(&b, results, "gauge"); err != nil { t.Fatal(err) } want = "prod stopped org/qwen:q4\n" + @@ -488,7 +492,7 @@ func TestFormatMetricsBarRunning(t *testing.T) { }, } var b bytes.Buffer - if err := formatMetricsBar(resp, remote.Config{}, &b); err != nil { + if err := renderFleetMetrics(&b, nodeResultsFor(resp), "bar"); err != nil { t.Fatal(err) } got := b.String() @@ -521,7 +525,7 @@ func TestFormatMetricsJSONCarriesHistory(t *testing.T) { }, } var b bytes.Buffer - if err := formatMetricsJSON(resp, false, remote.Config{}, &b); err != nil { + if err := renderFleetMetrics(&b, nodeResultsFor(resp), "json"); err != nil { t.Fatal(err) } got := b.String() @@ -533,10 +537,31 @@ func TestFormatMetricsJSONCarriesHistory(t *testing.T) { // Absent history stays absent, as on the daemon. b.Reset() resp.History = nil - if err := formatMetricsJSON(resp, false, remote.Config{}, &b); err != nil { + if err := renderFleetMetrics(&b, nodeResultsFor(resp), "json"); err != nil { t.Fatal(err) } if strings.Contains(b.String(), "history") { t.Errorf("absent history serialised: %s", b.String()) } } + +// nodeResultsFor turns a control-plane stats reply into the one node result a +// fan-out would produce for it, so a test can state its input as the reply and +// assert on what the renderer draws. +func nodeResultsFor(resp *remote.StatsResponse) []fleet.NodeResult { + return []fleet.NodeResult{{ + Name: resp.Environment, Outcome: fleet.OutcomeOK, + Instance: fleet.Instance{ + ID: resp.InstanceID, Type: resp.InstanceType, Version: resp.Version, + RetainUntil: resp.RetainUntil, + }, + Metrics: metrics.Stats{ + State: resp.State, Runner: resp.Runner, ModelID: resp.ModelID, + UptimeSeconds: resp.UptimeSeconds, Tokens: resp.Tokens, + GPUs: resp.GPUs, CPU: resp.CPU, Memory: resp.Memory, + History: resp.History, Errors: resp.Errors, + LastActiveAt: resp.LastActiveAt, IdleSeconds: resp.IdleSeconds, + RetainUntil: resp.RetainUntil, + }, + }} +} diff --git a/cmd/spinloop/read_spinloop_env.go b/cmd/spinloop/read_spinloop_env.go new file mode 100644 index 00000000..0fb38b5b --- /dev/null +++ b/cmd/spinloop/read_spinloop_env.go @@ -0,0 +1,37 @@ +// The Spinloop a read verb may be given. It selects nothing — the target comes +// from --env or --fleet — and is read only for the environment it carries: its +// ENV instructions and the .env beside it, applied before any control-plane or +// daemon work. That is what lets AWS credentials, a profile, or the +// SPINLOOP_REMOTE_* overrides live in a project's Spinloop rather than in the +// shell that happens to be running the command. +// +// The `remote` subcommands this replaces also consulted ./Spinloop when none +// was named. The verbs do not: a file sitting in the working directory should +// not silently set environment variables for a command that reads a fleet, and +// naming it is one flag. Nothing else about the rule changes. + +package main + +import "github.com/spf13/pflag" + +// spinloopEnvUsage is the --spinloop flag's help on every read verb. +const spinloopEnvUsage = "read this Spinloop's ENV instructions and adjacent .env before reading the target (it selects nothing)" + +// registerSpinloopEnvFlag adds the flag to a read verb's flag set. +func registerSpinloopEnvFlag(fs *pflag.FlagSet, path *string) { + fs.StringVarP(path, "spinloop", "O", "", spinloopEnvUsage) +} + +// applyReadSpinloopEnv reads the named Spinloop and applies the environment it +// carries. An empty path applies nothing: the verbs take the file only when +// told to. +func applyReadSpinloopEnv(path string) error { + if path == "" { + return nil + } + sel, resolved, err := readSpinloop("spinloop --spinloop=", path) + if err != nil { + return err + } + return applySpinloopEnv(sel, resolved) +} diff --git a/cmd/spinloop/read_spinloop_env_test.go b/cmd/spinloop/read_spinloop_env_test.go new file mode 100644 index 00000000..5f03e801 --- /dev/null +++ b/cmd/spinloop/read_spinloop_env_test.go @@ -0,0 +1,97 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A Spinloop's ENV instructions reach the command that was given it, which is +// what lets a project's AWS profile or SPINLOOP_REMOTE_* overrides live in the +// Spinloop rather than in whatever shell is running the command. +func TestReadSpinloopEnv_AppliesENVInstructions(t *testing.T) { + t.Setenv("SPINLOOP_READ_ENV_PROBE", "") + dir := t.TempDir() + path := filepath.Join(dir, "Spinloop") + body := "PROVIDER llamacpp\nENV SPINLOOP_READ_ENV_PROBE=from-the-spinloop\n" + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + if err := applyReadSpinloopEnv(path); err != nil { + t.Fatalf("applyReadSpinloopEnv: %v", err) + } + if got := os.Getenv("SPINLOOP_READ_ENV_PROBE"); got != "from-the-spinloop" { + t.Errorf("the ENV instruction did not reach the process: %q", got) + } +} + +// The .env beside the Spinloop fills what the environment has not already set, +// so a secret lives in a file rather than a command line. +func TestReadSpinloopEnv_AppliesTheAdjacentDotEnv(t *testing.T) { + t.Setenv("SPINLOOP_READ_DOTENV_PROBE", "") + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "Spinloop"), []byte("PROVIDER llamacpp\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".env"), []byte("SPINLOOP_READ_DOTENV_PROBE=from-the-dotenv\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := applyReadSpinloopEnv(filepath.Join(dir, "Spinloop")); err != nil { + t.Fatalf("applyReadSpinloopEnv: %v", err) + } + if got := os.Getenv("SPINLOOP_READ_DOTENV_PROBE"); got != "from-the-dotenv" { + t.Errorf("the adjacent .env did not reach the process: %q", got) + } +} + +// Naming nothing applies nothing: a Spinloop in the working directory does not +// set variables for a command that was not told to read one. +func TestReadSpinloopEnv_EmptyPathAppliesNothing(t *testing.T) { + t.Setenv("SPINLOOP_READ_IMPLICIT_PROBE", "") + dir := t.TempDir() + body := "PROVIDER llamacpp\nENV SPINLOOP_READ_IMPLICIT_PROBE=should-not-apply\n" + if err := os.WriteFile(filepath.Join(dir, "Spinloop"), []byte(body), 0o600); err != nil { + t.Fatal(err) + } + t.Chdir(dir) + + if err := applyReadSpinloopEnv(""); err != nil { + t.Fatalf("applyReadSpinloopEnv(\"\"): %v", err) + } + if got := os.Getenv("SPINLOOP_READ_IMPLICIT_PROBE"); got != "" { + t.Errorf("a Spinloop nobody named set %q — the verbs take one only when told", got) + } +} + +// A named Spinloop that cannot be read fails the command, rather than being +// skipped: the operator pointed at it. +func TestReadSpinloopEnv_UnreadableNamesItself(t *testing.T) { + err := applyReadSpinloopEnv(filepath.Join(t.TempDir(), "nosuch", "Spinloop")) + if err == nil { + t.Fatal("a named Spinloop that cannot be read should fail") + } + if !strings.Contains(err.Error(), "Spinloop") { + t.Errorf("the failure should name what it could not read, got %v", err) + } +} + +// The flag reaches the verbs through the real command line, and selects +// nothing: the target still has to be named. +func TestReadVerbsTakeTheSpinloopFlag(t *testing.T) { + t.Setenv("SPINLOOP_READ_VERB_PROBE", "") + dir := t.TempDir() + body := "PROVIDER llamacpp\nENV SPINLOOP_READ_VERB_PROBE=applied\n" + spinloopPath := filepath.Join(dir, "Spinloop") + if err := os.WriteFile(spinloopPath, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + writeFleetFile(t, oneNodeFleetBody) + + if err := cmdStatus([]string{"-O", spinloopPath}); err != nil { + t.Errorf("status with a Spinloop: %v", err) + } + if got := os.Getenv("SPINLOOP_READ_VERB_PROBE"); got != "applied" { + t.Errorf("the verb did not apply the Spinloop's ENV: %q", got) + } +} diff --git a/cmd/spinloop/remote.go b/cmd/spinloop/remote.go index f0e6f36f..3ae93588 100644 --- a/cmd/spinloop/remote.go +++ b/cmd/spinloop/remote.go @@ -2,21 +2,18 @@ package main import ( "context" - "encoding/json" "errors" "fmt" "io" "maps" "net/http" "os" - "os/signal" "path/filepath" "regexp" "slices" "strings" "sync" - "syscall" "time" "github.com/spf13/cobra" @@ -662,326 +659,6 @@ func runRemoteStop(envName string, args []string) error { return nil } -func remoteStatusCmd() *cobra.Command { - var envName string - c := &cobra.Command{ - Use: "status", - Short: "report the instance's state", - Args: cobra.ArbitraryArgs, - SilenceErrors: true, - SilenceUsage: true, - ValidArgsFunction: aliasSlot, - RunE: func(c *cobra.Command, args []string) error { - resolve(c) - return runRemoteStatus(envName, args) - }, - } - c.Flags().StringVar(&envName, "env", "", envFlagUsage) - compRegister(c, "env", compEnvs) - return c -} - -// runRemoteStatus is the body of `spinloop remote status`. -func runRemoteStatus(envName string, args []string) error { - cfg, err := resolveRemoteConfig(envName, spinloopArg(args)) - if err != nil { - return err - } - resp, err := remote.Status(context.Background(), cfg) - if err != nil { - return err - } - // The shared facts — state, since-last-work, version — come from the same - // source `fleet status` reads, so the two status views cannot word them - // differently. This view keeps its own extra facts (health, base URL) and - // its key-value layout. The version is only in the stats reply, which - // reads the daemon over SSM: attempt it only when the instance is running, - // since the daemon will not answer when it is stopped. - fact := statusFact{ - State: resp.State, - LastActiveAt: resp.LastActiveAt, - IdleSeconds: resp.IdleSeconds, - } - if resp.State == "running" || resp.State == "ready" { - if stats, err := remote.Stats(context.Background(), cfg); err == nil { - fact.Version = stats.Version - } - } - fmt.Printf("state: %s\n", fact.State) - if resp.Healthy != nil { - fmt.Printf("healthy: %t\n", *resp.Healthy) - } - if fact.Version != "" { - fmt.Printf("version: %s\n", fact.Version) - } - if resp.BaseURL != "" { - fmt.Printf("base_url: %s\n", resp.BaseURL) - } - // The active figure and, for a retained instance, the relative keep after - // it — one line, like the metrics view, rather than an absolute deadline on - // its own. - active := lastActiveText(fact.LastActiveAt, fact.IdleSeconds) - keep := keepText(resp.RetainUntil, metricsNow()) - switch { - case active != "" && keep != "": - fmt.Printf("active: %s %s\n", active, keep) - case active != "": - fmt.Printf("active: %s\n", active) - case keep != "": - fmt.Printf("active: %s\n", keep) - } - return nil -} - -// cmdRemoteMetrics queries the stats Lambda for instance metrics: token usage, -// GPU, CPU, and RAM utilization. The default gauge format draws the current -// reading as progress gauges; --format=bar draws each series as a sparkline -// of the daemon's retained history instead. With --format=json it outputs -// JSON. With --cost, it looks up the on-demand price for the instance type -// from the AWS Price List API. With --watch it polls every 60 seconds until -// interrupted. -func remoteMetricsCmd() *cobra.Command { - var ( - withCost bool - format string - watch bool - envName string - ) - c := &cobra.Command{ - Use: "metrics", - Short: "sample the instance's metrics", - Args: cobra.ArbitraryArgs, - SilenceErrors: true, - SilenceUsage: true, - ValidArgsFunction: aliasSlot, - RunE: func(c *cobra.Command, args []string) error { - resolve(c) - return runRemoteMetrics(envName, args, withCost, format, watch) - }, - } - fs := c.Flags() - fs.BoolVar(&withCost, "cost", false, "include cost estimate from AWS Price List API") - fs.StringVar(&format, "format", "gauge", "output format: gauge (default), bar, table or json") - fs.BoolVarP(&watch, "watch", "w", false, "poll metrics every 60 seconds") - fs.StringVar(&envName, "env", "", envFlagUsage) - compRegister(c, "env", compEnvs) - return c -} - -// runRemoteMetrics is the body of `spinloop remote metrics`. -func runRemoteMetrics(envName string, args []string, withCost bool, format string, watch bool) error { - if err := validateMetricsFormat(format); err != nil { - return err - } - - cfg, err := resolveRemoteConfig(envName, spinloopArg(args)) - if err != nil { - return err - } - - if watch { - return runMetricsWatch(cfg, format, withCost) - } - return runMetricsOnce(context.Background(), cfg, format, withCost, os.Stdout) -} - -func runMetricsOnce(ctx context.Context, cfg remote.Config, format string, withCost bool, w io.Writer) error { - resp, err := remote.Stats(ctx, cfg) - if err != nil { - return err - } - - switch format { - case "json": - return formatMetricsJSON(resp, withCost, cfg, w) - case "gauge": - return formatMetricsGauge(resp, cfg, w) - case "bar": - return formatMetricsBar(resp, cfg, w) - } - return formatMetricsTable(ctx, resp, withCost, cfg, w) -} - -func runMetricsWatch(cfg remote.Config, format string, withCost bool) error { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - sigCh := make(chan os.Signal, 1) - signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - go func() { - <-sigCh - cancel() - }() - - first := true - for { - // Fetch into a buffer first so the clear-and-render is instant. - var buf strings.Builder - if err := runMetricsOnce(ctx, cfg, format, withCost, &buf); err != nil { - if ctx.Err() != nil { - return nil - } - return err - } - if !first { - fmt.Fprint(os.Stdout, "\033[2J\033[H") - } - first = false - fmt.Fprint(os.Stdout, buf.String()) - select { - case <-ctx.Done(): - return nil - case <-time.After(metricsWatchInterval): - } - } -} - -func formatMetricsTable(ctx context.Context, resp *remote.StatsResponse, withCost bool, cfg remote.Config, w io.Writer) error { - now := metricsNow() - fmt.Fprintf(w, "environment: %s\n", resp.Environment) - fmt.Fprintf(w, "state: %s\n", resp.State) - - if resp.State != "running" { - if resp.Runner != "" { - fmt.Fprintf(w, "runner: %s\n", resp.Runner) - } - if resp.ModelID != "" { - fmt.Fprintf(w, "model: %s\n", resp.ModelID) - } - // A stopped environment can still be retained: its deadline is the - // control plane's, so it rides this branch too. - renderActiveKeyValue(w, resp.LastActiveAt, resp.IdleSeconds, resp.RetainUntil, now) - return nil - } - - if resp.InstanceID != "" { - fmt.Fprintf(w, "instance: %s\n", resp.InstanceID) - } - if resp.InstanceType != "" { - fmt.Fprintf(w, "instanceType: %s\n", resp.InstanceType) - } - if resp.Runner != "" { - fmt.Fprintf(w, "runner: %s\n", resp.Runner) - } - if resp.ModelID != "" { - fmt.Fprintf(w, "model: %s\n", resp.ModelID) - } - if resp.Version != "" { - fmt.Fprintf(w, "version: %s\n", resp.Version) - } - if resp.UptimeSeconds > 0 { - fmt.Fprintf(w, "uptime: %s\n", formatDuration(resp.UptimeSeconds)) - } - renderActiveKeyValue(w, resp.LastActiveAt, resp.IdleSeconds, resp.RetainUntil, now) - - renderTokenLines(w, resp.Tokens) - renderGPUTable(w, resp.GPUs) - renderCPUMemTable(w, resp.CPU, resp.Memory) - - if withCost && resp.UptimeSeconds > 0 && resp.InstanceType != "" { - if price, err := getOnDemandPrice(ctx, cfg.Region, resp.InstanceType); err == nil { - hours := float64(resp.UptimeSeconds) / 3600.0 - fmt.Fprintf(w, " cost so far: $%.2f (%.4f/hr)\n", hours*price, price) - } - } - - renderCollectionErrors(os.Stderr, resp.Errors) - - return nil -} - -func formatMetricsJSON(resp *remote.StatsResponse, withCost bool, cfg remote.Config, w io.Writer) error { - var costInfo *float64 - if withCost && resp.UptimeSeconds > 0 && resp.InstanceType != "" { - if price, err := getOnDemandPrice(context.Background(), cfg.Region, resp.InstanceType); err == nil { - hours := float64(resp.UptimeSeconds) / 3600.0 - cost := hours * price - costInfo = &cost - } - } - - if costInfo != nil { - type output struct { - remote.StatsResponse - Cost *float64 `json:"cost"` - } - out := output{ - StatsResponse: *resp, - Cost: costInfo, - } - data, err := json.MarshalIndent(out, "", " ") - if err != nil { - return err - } - fmt.Fprintln(w, string(data)) - } else { - data, err := json.MarshalIndent(resp, "", " ") - if err != nil { - return err - } - fmt.Fprintln(w, string(data)) - } - - renderCollectionErrors(os.Stderr, resp.Errors) - - return nil -} - -// formatMetricsHeader draws the line both compact formats open with: the -// environment, state, instance type, and model, double-spaced. -func formatMetricsHeader(resp *remote.StatsResponse, w io.Writer) { - fmt.Fprintf(w, "%s %s", resp.Environment, resp.State) - if resp.InstanceType != "" { - fmt.Fprintf(w, " %s", resp.InstanceType) - } - if resp.ModelID != "" { - fmt.Fprintf(w, " %s", resp.ModelID) - } - if resp.Version != "" { - fmt.Fprintf(w, " %s", resp.Version) - } - fmt.Fprintln(w) -} - -func formatMetricsBar(resp *remote.StatsResponse, cfg remote.Config, w io.Writer) error { - now := metricsNow() - formatMetricsHeader(resp, w) - - // Before the series: when the endpoint last did work — and, for a retained - // endpoint, how long it is kept — is worth showing in whatever state it is in, - // and the retained history is too: a stopped endpoint's readings up to the - // stop answer what it was doing until it stopped. A stopped endpoint's - // current reading carries no resource figures, so the series it draws come - // from the history alone, or not at all where the daemon predates it. - renderActiveIndented(w, resp.LastActiveAt, resp.IdleSeconds, resp.RetainUntil, now) - - renderStatBars(w, resp.CPU, resp.Memory, resp.GPUs, resp.History, barLineW) - renderTokenLines(w, resp.Tokens) - renderCollectionErrors(os.Stderr, resp.Errors) - - return nil -} - -func formatMetricsGauge(resp *remote.StatsResponse, cfg remote.Config, w io.Writer) error { - now := metricsNow() - formatMetricsHeader(resp, w) - - // Before the early return: a stopped endpoint draws no gauges, but when it - // last did work — and how long it is kept — is exactly what a stopped - // endpoint is worth asking about. - renderActiveIndented(w, resp.LastActiveAt, resp.IdleSeconds, resp.RetainUntil, now) - - if resp.State != "running" { - return nil - } - - renderStatGauges(w, resp.CPU, resp.Memory, resp.GPUs) - renderTokenLines(w, resp.Tokens) - renderCollectionErrors(os.Stderr, resp.Errors) - - return nil -} - func formatDuration(seconds int) string { d := time.Duration(seconds) * time.Second h := int(d.Hours()) @@ -1802,8 +1479,6 @@ func cmdRemoteStart(args []string) error { return execCmd(remoteStartCmd(), func cmdRemotePause(args []string) error { return execCmd(remotePauseCmd(), args) } func cmdRemoteRestart(args []string) error { return execCmd(remoteRestartCmd(), args) } func cmdRemoteStop(args []string) error { return execCmd(remoteStopCmd(), args) } -func cmdRemoteStatus(args []string) error { return execCmd(remoteStatusCmd(), args) } -func cmdRemoteMetrics(args []string) error { return execCmd(remoteMetricsCmd(), args) } func cmdRemoteDeploy(args []string) error { return execCmd(remoteDeployCmd(), args) } func cmdRemoteEnv(args []string) error { return execCmd(remoteEnvCmd(), args) } func cmdRemoteList(args []string) error { return execCmd(remoteListCmd(), args) } diff --git a/cmd/spinloop/remote_environments_test.go b/cmd/spinloop/remote_environments_test.go index 8c7634eb..14d266b1 100644 --- a/cmd/spinloop/remote_environments_test.go +++ b/cmd/spinloop/remote_environments_test.go @@ -49,11 +49,11 @@ func TestRemote_EnvNameResolves(t *testing.T) { t.Fatal(err) } out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "prodenv"}); err != nil { + if err := cmdStatus([]string{"--env", "prodenv"}); err != nil { t.Errorf("status via --env name: %v", err) } }) - if !strings.Contains(out, "state: running") { + if !strings.Contains(out, "running") { t.Errorf("--env name should resolve via the registry, got:\n%s", out) } } @@ -68,11 +68,11 @@ func TestRemote_DefaultEnvironment(t *testing.T) { t.Chdir(t.TempDir()) // no ./Spinloop here out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { + if err := cmdStatus([]string{"--env", "default"}); err != nil { t.Errorf("status via default env: %v", err) } }) - if !strings.Contains(out, "state: running") { + if !strings.Contains(out, "running") { t.Errorf("no-Spinloop should use the default environment, got:\n%s", out) } } @@ -94,7 +94,7 @@ func TestRemote_SupersededFileIsNotRead(t *testing.T) { t.Fatal(err) } t.Chdir(t.TempDir()) - err := cmdRemoteMetrics([]string{"--env", "default"}) + err := cmdMetrics([]string{"--env", "default"}) if err == nil { t.Fatal("the superseded file must not configure an environment") } diff --git a/cmd/spinloop/remote_logs.go b/cmd/spinloop/remote_logs.go deleted file mode 100644 index ec5b05c5..00000000 --- a/cmd/spinloop/remote_logs.go +++ /dev/null @@ -1,235 +0,0 @@ -package main - -import ( - "context" - "encoding/json" - "fmt" - "io" - "os" - "strings" - "time" - - "github.com/spf13/cobra" - "github.com/spinloop-ai/spinloop/internal/remote" -) - -// Seam: a package variable so tests drive the command without AWS. -var logsFetchFn = remote.FetchLogs - -// logsFollowInterval is how often a follow asks for more. CloudWatch charges -// per request and the agent ships in batches anyway, so polling faster would -// cost more without showing anything sooner. A variable so tests need not wait -// on it. -var logsFollowInterval = 5 * time.Second - -// cmdRemoteLogs prints the logs an environment's instances shipped to -// CloudWatch. It reads the durable store rather than the instance, so a boot -// that failed and an instance that has since terminated are both still -// readable — which is when logs are wanted most, and exactly when status and -// metrics have nothing left to report. -func remoteLogsCmd() *cobra.Command { - var ( - source string - since time.Duration - limit int - instance string - follow bool - format string - envName string - ) - const followUsage = "keep printing new events as they arrive" - c := &cobra.Command{ - Use: "logs", - Short: "tail the instance's logs", - Args: cobra.ArbitraryArgs, - SilenceErrors: true, - SilenceUsage: true, - ValidArgsFunction: aliasSlot, - RunE: func(c *cobra.Command, args []string) error { - resolve(c) - return runRemoteLogs(envName, args, source, since, limit, instance, follow, format) - }, - } - fs := c.Flags() - fs.StringVar(&source, "source", remote.LogSourceEngine, - "which log to read: engine (default), boot or all") - fs.DurationVar(&since, "since", time.Hour, "how far back to look, as a duration (30m, 2h)") - fs.IntVar(&limit, "limit", 200, "maximum events to print, keeping the most recent") - fs.StringVar(&instance, "instance", "", "restrict output to one instance id") - fs.BoolVarP(&follow, "follow", "f", false, followUsage) - fs.StringVar(&format, "format", "text", "output format: text (default) or json") - fs.StringVar(&envName, "env", "", envFlagUsage) - compRegister(c, "env", compEnvs) - return c -} - -// runRemoteLogs is the body of `spinloop remote logs`. -func runRemoteLogs(envName string, args []string, source string, since time.Duration, limit int, instance string, follow bool, format string) error { - if err := validateLogsFlags(source, format, since, limit); err != nil { - return err - } - - cfg, err := resolveRemoteConfig(envName, spinloopArg(args)) - if err != nil { - return err - } - - q := remote.LogQuery{ - Environment: cfg.Environment, - Source: source, - Start: time.Now().Add(-since), - Limit: limit, - Instance: instance, - } - if follow { - return followLogs(cfg, q, format) - } - return runLogsOnce(context.Background(), cfg, q, format, since, os.Stdout) -} - -// validateLogsFlags rejects the flag values that cannot mean anything, in the -// same shape as the other remote subcommands' checks. -func validateLogsFlags(source, format string, since time.Duration, limit int) error { - switch source { - case remote.LogSourceEngine, remote.LogSourceBoot, remote.LogSourceAll: - default: - return fmt.Errorf("--source must be %s, got %q", strings.Join(remote.LogSources, ", "), source) - } - if format != "text" && format != "json" { - return fmt.Errorf("--format must be \"text\" or \"json\", got %q", format) - } - if since <= 0 { - return fmt.Errorf("--since must be positive, got %s", since) - } - if limit <= 0 { - return fmt.Errorf("--limit must be positive, got %d", limit) - } - return nil -} - -// runLogsOnce fetches and prints a single window. An empty result is reported -// rather than left as silence, since "nothing was logged" and "the command did -// not work" look identical otherwise. -func runLogsOnce(ctx context.Context, cfg remote.Config, q remote.LogQuery, format string, - window time.Duration, w io.Writer) error { - res, err := logsFetchFn(ctx, cfg, q) - if err != nil { - return err - } - if len(res.Events) == 0 { - if format == "json" { - return writeLogsJSON(w, nil) - } - fmt.Fprintf(w, "no %s logs for environment %q in the last %s\n", - q.Source, q.Environment, window) - return nil - } - if format == "json" { - return writeLogsJSON(w, res.Events) - } - if res.Omitted > 0 { - fmt.Fprintf(w, "... %d earlier events omitted (raise --limit to see more)\n", res.Omitted) - } - writeLogsText(w, res.Events, mixedOrigins(res.Events)) - return nil -} - -// writeLogsText prints events oldest first, one per line, each with its local -// timestamp. Source and instance are prefixed only when label says the output -// actually mixes them: labelling every line of a single instance's engine log -// would be noise on the common case. -func writeLogsText(w io.Writer, events []remote.LogEvent, label bool) { - for _, e := range events { - stamp := e.Timestamp.Local().Format("2006-01-02 15:04:05") - if label { - fmt.Fprintf(w, "%s %s/%s %s\n", stamp, e.Source, e.Instance, e.Message) - continue - } - fmt.Fprintf(w, "%s %s\n", stamp, e.Message) - } -} - -// mixedOrigins reports whether the events come from more than one source or -// more than one instance, and so need attributing per line. -func mixedOrigins(events []remote.LogEvent) bool { - if len(events) == 0 { - return false - } - for _, e := range events[1:] { - if e.Source != events[0].Source || e.Instance != events[0].Instance { - return true - } - } - return false -} - -// writeLogsJSON emits the events as an array, carrying the same fields the text -// format shows, for scripting. An empty result is an empty array rather than -// null, so a consumer can iterate it unconditionally. -func writeLogsJSON(w io.Writer, events []remote.LogEvent) error { - if events == nil { - events = []remote.LogEvent{} - } - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - return enc.Encode(events) -} - -// followLogs prints the window, then keeps printing what arrives after it. -// Each poll starts a little behind the newest event already seen, to catch -// events the agent delivers late, and the ids seen in that overlap suppress the -// duplicates the overlap would otherwise print. Interrupting is a clean exit: -// the user asked it to stop, which is not a failure. -func followLogs(cfg remote.Config, q remote.LogQuery, format string) error { - return followUntilInterrupted(func(ctx context.Context) error { - return followLogsLoop(ctx, cfg, q, format, os.Stdout) - }) -} - -// followLogsLoop is the polling itself, with the interrupt wiring left to its -// caller so it can be driven directly. -func followLogsLoop(ctx context.Context, cfg remote.Config, q remote.LogQuery, - format string, w io.Writer) error { - cursor := remote.NewFollowCursor(remote.FollowOverlap) - // Labelling is decided across the whole session, not per batch: a poll that - // happened to return one instance's lines must not drop the prefix the - // previous poll's lines carried. Once a second origin appears the output - // stays labelled. - origins := map[string]bool{} - for first := true; ; first = false { - res, err := logsFetchFn(ctx, cfg, q) - if err != nil { - if ctx.Err() != nil { - return nil - } - return err - } - // Only the opening window can be capped meaningfully — later polls ask - // for the sliver since the last event — so it is the only one that - // reports what the cap dropped. - if first && res.Omitted > 0 && format != "json" { - fmt.Fprintf(w, "... %d earlier events omitted (raise --limit to see more)\n", res.Omitted) - } - fresh := cursor.Advance(res.Events) - for _, e := range fresh { - origins[e.Source+"/"+e.Instance] = true - } - if len(fresh) > 0 { - if format == "json" { - if err := writeLogsJSON(w, fresh); err != nil { - return err - } - } else { - writeLogsText(w, fresh, len(origins) > 1) - } - } - if start := cursor.Start(); !start.IsZero() { - q.Start = start - } - select { - case <-ctx.Done(): - return nil - case <-time.After(logsFollowInterval): - } - } -} diff --git a/cmd/spinloop/remote_logs_test.go b/cmd/spinloop/remote_logs_test.go index 7c7ac61f..043785de 100644 --- a/cmd/spinloop/remote_logs_test.go +++ b/cmd/spinloop/remote_logs_test.go @@ -1,74 +1,25 @@ package main import ( - "bytes" "context" - "encoding/json" - "errors" - "os" - "path/filepath" - "strings" "testing" "time" + "github.com/spinloop-ai/spinloop/internal/fleet" "github.com/spinloop-ai/spinloop/internal/remote" ) -// errFetchFailed stands in for a fetch failure that is not a cancellation. -var errFetchFailed = errors.New("reading logs failed") - -// withRemoteEnvironment isolates the config home, registers one environment and -// puts a Spinloop in the working directory, so the command resolves the -// environment (by its --env name) through the same path a real setup does. -func withRemoteEnvironment(t *testing.T, name string) { - t.Helper() - isolateConfig(t) - t.Chdir(t.TempDir()) - if err := os.WriteFile("Spinloop", - []byte("PROVIDER llamacpp\nMODEL unsloth/Qwen3.6-27B-GGUF\n"), 0o600); err != nil { - t.Fatal(err) - } - path := must1(remote.EnvConfigPath(name)) - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - t.Fatal(err) - } - data, err := json.Marshal(remote.Config{ - StartURL: "https://start.lambda-url.eu-west-1.on.aws/", - StopURL: "https://stop.lambda-url.eu-west-1.on.aws/", - Region: "eu-west-1", - Environment: name, - }) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(path, data, 0o600); err != nil { - t.Fatal(err) - } -} - -// stubLogsFetch substitutes the fetch for the duration of a test, handing the -// stub each query so it can assert on what the command asked for. -func stubLogsFetch(t *testing.T, - fn func(q remote.LogQuery) (remote.LogResult, error)) { +// stubTopLevelLogsFetch substitutes the log store read for the duration of a +// test, handing the stub each query so it can assert on what the command +// asked for. The read goes through fleet.FetchLogsFn rather than an HTTP +// server: CloudWatch is reached through the AWS SDK, not a Function URL. +func stubTopLevelLogsFetch(t *testing.T, fn func(q remote.LogQuery) (remote.LogResult, error)) { t.Helper() - prev := logsFetchFn - logsFetchFn = func(_ context.Context, _ remote.Config, q remote.LogQuery) (remote.LogResult, error) { + prev := fleet.FetchLogsFn + fleet.FetchLogsFn = func(_ context.Context, _ remote.Config, q remote.LogQuery) (remote.LogResult, error) { return fn(q) } - t.Cleanup(func() { logsFetchFn = prev }) -} - -// logEvent builds one event at a fixed instant, so rendered timestamps are -// stable across runs. -func logEvent(id string, offset time.Duration, source, instance, message string) remote.LogEvent { - base := time.Date(2026, 8, 9, 11, 30, 0, 0, time.Local) - return remote.LogEvent{ - Timestamp: base.Add(offset), - Source: source, - Instance: instance, - Message: message, - ID: id, - } + t.Cleanup(func() { fleet.FetchLogsFn = prev }) } func TestRunnerForAcceptsExactlyTheRunnersWithLogGroups(t *testing.T) { @@ -97,70 +48,52 @@ func TestRunnerForAcceptsExactlyTheRunnersWithLogGroups(t *testing.T) { } } -func TestRemoteLogsRejectsBadFlagValues(t *testing.T) { - cases := []struct { - name string - source string - format string - since time.Duration - limit int - want string - }{ - {"source", "journal", "text", time.Hour, 10, "--source"}, - {"format", "engine", "yaml", time.Hour, 10, "--format"}, - {"since", "engine", "text", -time.Hour, 10, "--since"}, - {"limit", "engine", "text", time.Hour, 0, "--limit"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - err := validateLogsFlags(tc.source, tc.format, tc.since, tc.limit) - if err == nil { - t.Fatalf("expected %s to be rejected", tc.want) - } - if !strings.Contains(err.Error(), tc.want) { - t.Errorf("error = %q, want it to name %s", err, tc.want) - } - }) - } - if err := validateLogsFlags("all", "json", time.Minute, 1); err != nil { - t.Errorf("valid flags rejected: %v", err) - } -} +// spinloop logs --env defaults to the engine source and an hour window, and +// threads --source/--since/--limit/--instance through to the environment's +// log store query. +func TestTopLevelLogsDefaultsToTheEngineSourceAndAnHourWindow(t *testing.T) { + isolateConfig(t) + registerEnv(t, "prod", remote.Config{ + StartURL: "https://start.lambda-url.eu-west-1.on.aws/", + StopURL: "https://stop.lambda-url.eu-west-1.on.aws/", + Region: "eu-west-1", + }) -func TestRemoteLogsDefaultsToTheEngineSourceAndAnHourWindow(t *testing.T) { var got remote.LogQuery - stubLogsFetch(t, func(q remote.LogQuery) (remote.LogResult, error) { + stubTopLevelLogsFetch(t, func(q remote.LogQuery) (remote.LogResult, error) { got = q return remote.LogResult{}, nil }) - withRemoteEnvironment(t, "prod") - if err := cmdRemote([]string{"logs", "--env", "prod"}); err != nil { + if err := cmdLogs([]string{"--env", "prod"}); err != nil { t.Fatal(err) } if got.Source != remote.LogSourceEngine { t.Errorf("source = %q, want engine by default", got.Source) } - if got.Environment != "prod" { - t.Errorf("environment = %q, want the resolved environment", got.Environment) - } - if got.Limit != 200 { - t.Errorf("limit = %d, want the default 200", got.Limit) + if got.Limit != 200*bytesPerLineGuess { + t.Errorf("limit = %d, want the default 200 lines' worth", got.Limit) } if window := time.Since(got.Start); window < 55*time.Minute || window > 65*time.Minute { t.Errorf("window = %s, want about an hour", window) } } -func TestRemoteLogsPassesTheFlagsThrough(t *testing.T) { +func TestTopLevelLogsPassesTheFlagsThrough(t *testing.T) { + isolateConfig(t) + registerEnv(t, "prod", remote.Config{ + StartURL: "https://start.lambda-url.eu-west-1.on.aws/", + StopURL: "https://stop.lambda-url.eu-west-1.on.aws/", + Region: "eu-west-1", + }) + var got remote.LogQuery - stubLogsFetch(t, func(q remote.LogQuery) (remote.LogResult, error) { + stubTopLevelLogsFetch(t, func(q remote.LogQuery) (remote.LogResult, error) { got = q return remote.LogResult{}, nil }) - withRemoteEnvironment(t, "prod") - if err := cmdRemote([]string{"logs", "--env", "prod", "--source", "boot", "--since", "15m", + if err := cmdLogs([]string{"--env", "prod", "--source", "boot", "--since", "15m", "--limit", "5", "--instance", "i-42"}); err != nil { t.Fatal(err) } @@ -170,222 +103,10 @@ func TestRemoteLogsPassesTheFlagsThrough(t *testing.T) { if got.Instance != "i-42" { t.Errorf("instance = %q, want i-42", got.Instance) } - if got.Limit != 5 { - t.Errorf("limit = %d, want 5", got.Limit) + if got.Limit != 5*bytesPerLineGuess { + t.Errorf("limit = %d, want the 5-line budget", got.Limit) } if window := time.Since(got.Start); window < 10*time.Minute || window > 20*time.Minute { t.Errorf("window = %s, want about 15 minutes", window) } } - -func TestLogsTextOutputIsTimestampedAndUnlabelledForOneOrigin(t *testing.T) { - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{Events: []remote.LogEvent{ - logEvent("a", 0, "engine", "i-1", "loading weights"), - logEvent("b", time.Second, "engine", "i-1", "serving"), - }}, nil - }) - - var buf bytes.Buffer - if err := runLogsOnce(context.Background(), remote.Config{}, remote.LogQuery{}, "text", - time.Hour, &buf); err != nil { - t.Fatal(err) - } - want := "2026-08-09 11:30:00 loading weights\n2026-08-09 11:30:01 serving\n" - if buf.String() != want { - t.Errorf("output =\n%q\nwant\n%q", buf.String(), want) - } -} - -func TestLogsTextOutputLabelsMixedOrigins(t *testing.T) { - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{Events: []remote.LogEvent{ - logEvent("a", 0, "boot", "i-1", "downloading weights"), - logEvent("b", time.Second, "engine", "i-1", "serving"), - }}, nil - }) - - var buf bytes.Buffer - if err := runLogsOnce(context.Background(), remote.Config{}, remote.LogQuery{}, "text", - time.Hour, &buf); err != nil { - t.Fatal(err) - } - if !strings.Contains(buf.String(), "boot/i-1 downloading weights") || - !strings.Contains(buf.String(), "engine/i-1 serving") { - t.Errorf("output =\n%s\nwant each line attributed to its source and instance", buf.String()) - } -} - -func TestLogsReportsOmittedEvents(t *testing.T) { - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{ - Events: []remote.LogEvent{logEvent("a", 0, "engine", "i-1", "serving")}, - Omitted: 12, - }, nil - }) - - var buf bytes.Buffer - if err := runLogsOnce(context.Background(), remote.Config{}, remote.LogQuery{}, "text", - time.Hour, &buf); err != nil { - t.Fatal(err) - } - if !strings.Contains(buf.String(), "12 earlier events omitted") { - t.Errorf("output =\n%s\nwant the omission reported", buf.String()) - } -} - -func TestLogsReportsAnEmptyWindowWithoutFailing(t *testing.T) { - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{}, nil - }) - - var buf bytes.Buffer - err := runLogsOnce(context.Background(), remote.Config{}, - remote.LogQuery{Environment: "prod", Source: "engine"}, "text", 30*time.Minute, &buf) - if err != nil { - t.Fatalf("an empty window is not a failure: %v", err) - } - if !strings.Contains(buf.String(), "no engine logs for environment \"prod\" in the last 30m0s") { - t.Errorf("output = %q, want it to say nothing was logged in the window", buf.String()) - } -} - -func TestLogsJSONOutputCarriesTheFields(t *testing.T) { - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{Events: []remote.LogEvent{ - logEvent("a", 0, "engine", "i-1", "serving"), - }}, nil - }) - - var buf bytes.Buffer - if err := runLogsOnce(context.Background(), remote.Config{}, remote.LogQuery{}, "json", - time.Hour, &buf); err != nil { - t.Fatal(err) - } - var events []remote.LogEvent - if err := json.Unmarshal(buf.Bytes(), &events); err != nil { - t.Fatalf("output is not JSON: %v\n%s", err, buf.String()) - } - if len(events) != 1 { - t.Fatalf("decoded %d events, want 1", len(events)) - } - if events[0].Source != "engine" || events[0].Instance != "i-1" || events[0].Message != "serving" { - t.Errorf("event = %+v, want source, instance and message carried", events[0]) - } - if events[0].Timestamp.IsZero() { - t.Error("event carries no timestamp") - } -} - -func TestLogsJSONOutputIsAnEmptyArrayWhenNothingMatched(t *testing.T) { - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{}, nil - }) - - var buf bytes.Buffer - if err := runLogsOnce(context.Background(), remote.Config{}, remote.LogQuery{}, "json", - time.Hour, &buf); err != nil { - t.Fatal(err) - } - if strings.TrimSpace(buf.String()) != "[]" { - t.Errorf("output = %q, want an empty array", buf.String()) - } -} - -func TestFollowPrintsEachEventOnceAndStopsWhenCancelled(t *testing.T) { - prev := logsFollowInterval - logsFollowInterval = time.Millisecond - t.Cleanup(func() { logsFollowInterval = prev }) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - first := logEvent("a", 0, "engine", "i-1", "loading weights") - second := logEvent("b", time.Second, "engine", "i-1", "serving") - - polls := 0 - var windows []time.Time - stubLogsFetch(t, func(q remote.LogQuery) (remote.LogResult, error) { - polls++ - windows = append(windows, q.Start) - switch polls { - case 1: - return remote.LogResult{Events: []remote.LogEvent{first}}, nil - case 2: - // The overlap re-reads the event already printed, alongside a new one. - return remote.LogResult{Events: []remote.LogEvent{first, second}}, nil - default: - cancel() - return remote.LogResult{Events: []remote.LogEvent{first, second}}, nil - } - }) - - var buf bytes.Buffer - if err := followLogsLoop(ctx, remote.Config{}, remote.LogQuery{}, "text", &buf); err != nil { - t.Fatalf("a cancelled follow is a clean exit, got: %v", err) - } - if n := strings.Count(buf.String(), "loading weights"); n != 1 { - t.Errorf("printed the first event %d times, want exactly once:\n%s", n, buf.String()) - } - if n := strings.Count(buf.String(), "serving"); n != 1 { - t.Errorf("printed the second event %d times, want exactly once:\n%s", n, buf.String()) - } - if len(windows) < 2 || !windows[1].After(windows[0]) { - t.Errorf("windows = %v, want each poll to start from the last event seen", windows) - } -} - -// Once a follow has seen a second origin it keeps labelling, so a poll that -// happens to return one instance's lines does not silently drop the prefix the -// lines before it carried. -func TestFollowKeepsLabellingOnceOriginsAreMixed(t *testing.T) { - prev := logsFollowInterval - logsFollowInterval = time.Millisecond - t.Cleanup(func() { logsFollowInterval = prev }) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - polls := 0 - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - polls++ - switch polls { - case 1: - return remote.LogResult{Events: []remote.LogEvent{ - logEvent("a", 0, "boot", "i-1", "downloading"), - logEvent("b", time.Second, "engine", "i-1", "serving"), - }}, nil - case 2: - return remote.LogResult{Events: []remote.LogEvent{ - logEvent("c", 2*time.Second, "engine", "i-1", "still serving"), - }}, nil - default: - cancel() - return remote.LogResult{}, nil - } - }) - - var buf bytes.Buffer - if err := followLogsLoop(ctx, remote.Config{}, remote.LogQuery{}, "text", &buf); err != nil { - t.Fatal(err) - } - if !strings.Contains(buf.String(), "engine/i-1 still serving") { - t.Errorf("output =\n%s\nwant the later single-origin batch still labelled", buf.String()) - } -} - -func TestFollowReturnsARealError(t *testing.T) { - prev := logsFollowInterval - logsFollowInterval = time.Millisecond - t.Cleanup(func() { logsFollowInterval = prev }) - - stubLogsFetch(t, func(remote.LogQuery) (remote.LogResult, error) { - return remote.LogResult{}, errFetchFailed - }) - - var buf bytes.Buffer - err := followLogsLoop(context.Background(), remote.Config{}, remote.LogQuery{}, "text", &buf) - if err == nil { - t.Fatal("a failure that is not a cancellation should be reported") - } -} diff --git a/cmd/spinloop/remote_test.go b/cmd/spinloop/remote_test.go index 58201baf..cde88c89 100644 --- a/cmd/spinloop/remote_test.go +++ b/cmd/spinloop/remote_test.go @@ -69,8 +69,10 @@ func TestRemoteDispatch(t *testing.T) { func TestRemote_Unconfigured(t *testing.T) { isolateConfig(t) - // deploy needs a Spinloop, covered separately. - subs := []string{"start", "restart", "stop", "metrics"} + // deploy needs a Spinloop, covered separately. metrics, logs and status + // moved to the top level — cmd/spinloop/commands_test.go covers their + // signpost under the old spelling. + subs := []string{"start", "restart", "stop"} // Naming no environment is its own failure: these commands act on one // instance, and an instance nobody named is not one to act on. for _, sub := range subs { @@ -197,15 +199,19 @@ func TestRemoteStatus_PrintsState(t *testing.T) { writeRemoteConfig(t, server.URL) out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Errorf("cmdStatus: %v", err) } }) - for _, want := range []string{"state: running", "healthy: true", "base_url: http://198.51.100.1:8000/v1"} { + for _, want := range []string{"running", "http://198.51.100.1:8000/v1"} { if !strings.Contains(out, want) { t.Errorf("status output missing %q:\n%s", want, out) } } + // healthy:true reads as the absence of the not-ready mark, not a separate line. + if strings.Contains(out, "not ready") { + t.Errorf("a healthy endpoint should not be marked not ready:\n%s", out) + } } // Remote status includes the spinloop version from the stats Lambda when the @@ -242,11 +248,11 @@ func TestRemoteStatus_PrintsVersion(t *testing.T) { } out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Errorf("cmdStatus: %v", err) } }) - if !strings.Contains(out, "version: 1.18.0") { + if !strings.Contains(out, "1.18.0") { t.Errorf("status output missing version:\n%s", out) } } @@ -269,7 +275,7 @@ func TestRemoteStop_PrintsState(t *testing.T) { t.Errorf("cmdRemoteStop: %v", err) } }) - if !strings.Contains(out, "state: stopping") { + if !strings.Contains(out, "stopping") { t.Errorf("stop should print the state, got:\n%s", out) } } @@ -505,11 +511,11 @@ func TestRemote_RestartInGeneratedHelp(t *testing.T) { } } -// The ./Spinloop in the working directory is still read before any -// control-plane work — here for its ENV instructions, which name the control -// plane a command with no other configuration would not find. +// A Spinloop is read only when named with --spinloop/-O — never implicitly +// from the working directory — for its ENV instructions, which here name the +// control plane a command with no other configuration would not find. func TestRemote_SpinloopDiscovery(t *testing.T) { - isolateConfig(t) // no per-user config exists, so success proves discovery + isolateConfig(t) // no per-user config exists, so success proves the ENV reached it stubAWSEnv(t) unsetEnvOnCleanup(t, "SPINLOOP_REMOTE_START_URL", "SPINLOOP_REMOTE_STOP_URL", "SPINLOOP_REMOTE_REGION") server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -526,28 +532,28 @@ func TestRemote_SpinloopDiscovery(t *testing.T) { } out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default", "--spinloop", "Spinloop"}); err != nil { + t.Errorf("cmdStatus: %v", err) } }) - if !strings.Contains(out, "state: running") { + if !strings.Contains(out, "running") { t.Errorf("status via the Spinloop's ENV should work, got:\n%s", out) } } -// An explicit Spinloop no longer names an environment: read for its ENV -// instructions and the adjacent .env, it falls through to the --env flag and -// the per-user config, failing on the missing configuration — not on the -// Spinloop. +// --spinloop reads a Spinloop for its ENV instructions and the adjacent +// .env; it never selects an environment by itself. One with no ENV lines +// supplies nothing, so with no --env or --fleet either the command fails on +// the missing target, not on the Spinloop. func TestRemote_ExplicitSpinloopDoesNotNameAnEnvironment(t *testing.T) { isolateConfig(t) t.Chdir(t.TempDir()) if err := os.WriteFile("Spinloop", []byte("PROVIDER ollama\n"), 0o600); err != nil { t.Fatal(err) } - err := cmdRemoteStatus([]string{"--env", "default", "Spinloop"}) - if err == nil || !strings.Contains(err.Error(), "remote is not configured") { - t.Errorf("want the not-configured error, got %v", err) + err := cmdStatus([]string{"--spinloop", "Spinloop"}) + if err == nil || !strings.Contains(err.Error(), "no fleet at") { + t.Errorf("want the no-target error, got %v", err) } } @@ -566,11 +572,11 @@ func TestRemote_SpinloopFallsBackToTheUserConfig(t *testing.T) { t.Fatal(err) } out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Errorf("cmdStatus: %v", err) } }) - if !strings.Contains(out, "state: stopped") { + if !strings.Contains(out, "stopped") { t.Errorf("a Spinloop with no --env should fall back to the user config, got:\n%s", out) } } @@ -593,11 +599,11 @@ func TestRemote_IgnoresLowercaseSpinloopFile(t *testing.T) { t.Fatal(err) } out := captureStdout(t, func() { - if err := cmdRemoteStatus([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteStatus: %v", err) + if err := cmdStatus([]string{"--env", "default"}); err != nil { + t.Errorf("cmdStatus: %v", err) } }) - if !strings.Contains(out, "state: stopped") { + if !strings.Contains(out, "stopped") { t.Errorf("a lowercase spinloop file should not shadow discovery, got:\n%s", out) } } @@ -638,12 +644,12 @@ func TestRemoteMetrics_Running(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=table"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=table"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) for _, want := range []string{ - "environment: dev", + "node: default", "state: running", "instance: i-abc123", "instanceType: g6e.xlarge", @@ -682,8 +688,8 @@ func TestRemoteMetrics_Stopped(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=table"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=table"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) if !strings.Contains(out, "state: stopped") { @@ -711,8 +717,8 @@ func TestRemoteMetrics_WithErrors(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) errOut := captureStderr(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) for _, want := range []string{"metric collection errors", "nvidia-smi failed", "vmstat timeout"} { @@ -746,11 +752,11 @@ func TestRemoteMetrics_DefaultFormat(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) - if !strings.Contains(out, "dev") || !strings.Contains(out, "running") { + if !strings.Contains(out, "default") || !strings.Contains(out, "running") { t.Errorf("default format missing header, got:\n%s", out) } // The default is the gauge format: the current reading filled, with no @@ -783,22 +789,27 @@ func TestRemoteMetrics_JsonFormat(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=json"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=json"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) - var result map[string]any - if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &result); err != nil { + var results []map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &results); err != nil { t.Fatalf("JSON output is not valid: %v\n%s", err, out) } - if result["environment"] != "dev" { - t.Errorf("expected environment=dev, got %v", result["environment"]) + if len(results) != 1 { + t.Fatalf("decoded %d nodes, want 1", len(results)) + } + result := results[0] + if result["node"] != "default" { + t.Errorf("expected node=default, got %v", result["node"]) } - if result["state"] != "running" { - t.Errorf("expected state=running, got %v", result["state"]) + metrics, _ := result["metrics"].(map[string]any) + if metrics["state"] != "running" { + t.Errorf("expected state=running, got %v", metrics["state"]) } - if result["instanceId"] != "i-abc123" { - t.Errorf("expected instanceId=i-abc123, got %v", result["instanceId"]) + if result["instance"] != "i-abc123" { + t.Errorf("expected instance=i-abc123, got %v", result["instance"]) } } @@ -820,16 +831,19 @@ func TestRemoteMetrics_JsonFormatWithCost(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=json", "--cost"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=json", "--cost"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) - var result map[string]any - if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &result); err != nil { + var results []map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &results); err != nil { t.Fatalf("JSON output is not valid: %v\n%s", err, out) } - if result["environment"] != "dev" { - t.Errorf("expected environment=dev, got %v", result["environment"]) + if len(results) != 1 { + t.Fatalf("decoded %d nodes, want 1", len(results)) + } + if results[0]["node"] != "default" { + t.Errorf("expected node=default, got %v", results[0]["node"]) } } @@ -838,7 +852,7 @@ func TestRemoteMetrics_InvalidFormat(t *testing.T) { stubAWSEnv(t) writeRemoteConfig(t, "http://localhost:0") - err := cmdRemoteMetrics([]string{"--env", "default", "--format=csv"}) + err := cmdMetrics([]string{"--env", "default", "--format=csv"}) if err == nil || !strings.Contains(err.Error(), "format") { t.Errorf("expected format error, got %v", err) } @@ -868,13 +882,13 @@ func TestRemoteMetrics_BarFormat(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - err := cmdRemoteMetrics([]string{"--env", "default", "--format=bar"}) + err := cmdMetrics([]string{"--env", "default", "--format=bar"}) if err != nil { t.Errorf("bar format failed: %v", err) } }) - if !strings.Contains(out, "dev") || !strings.Contains(out, "running") { + if !strings.Contains(out, "default") || !strings.Contains(out, "running") { t.Errorf("bar output missing header:\n%s", out) } if !strings.Contains(out, "CPU") { @@ -913,7 +927,7 @@ func TestRemoteMetrics_BarFormatStopped(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - err := cmdRemoteMetrics([]string{"--env", "default", "--format=bar"}) + err := cmdMetrics([]string{"--env", "default", "--format=bar"}) if err != nil { t.Errorf("bar format failed: %v", err) } @@ -989,7 +1003,7 @@ func TestRemoteMetrics_WatchMode(t *testing.T) { defer func() { metricsWatchInterval = oldInterval }() out := captureStdout(t, func() { - err := cmdRemoteMetrics([]string{"--env", "default", "--watch", "--format=table"}) + err := cmdMetrics([]string{"--env", "default", "--watch", "--format=table"}) // Expects error from the 3rd call. if err == nil { t.Error("watch should exit with error when server fails") @@ -1000,7 +1014,7 @@ func TestRemoteMetrics_WatchMode(t *testing.T) { t.Errorf("watch should have polled at least 3 times, got %d calls", callCount) } // Should see the metrics output multiple times (watch polls repeatedly). - count := strings.Count(out, "environment: dev") + count := strings.Count(out, "node: default") if count < 2 { t.Errorf("watch should have produced at least 2 outputs, got %d:\n%s", count, out) } @@ -1029,12 +1043,12 @@ func TestRemoteMetrics_WatchShortFlag(t *testing.T) { defer func() { metricsWatchInterval = oldInterval }() out := captureStdout(t, func() { - err := cmdRemoteMetrics([]string{"--env", "default", "-w", "--format=table"}) + err := cmdMetrics([]string{"--env", "default", "-w", "--format=table"}) if err == nil { t.Error("-w should exit with error when server fails") } }) - if !strings.Contains(out, "environment: dev") { + if !strings.Contains(out, "node: default") { t.Errorf("-w flag should work like --watch:\n%s", out) } if callCount < 2 { @@ -1061,8 +1075,8 @@ func TestRemoteMetrics_MultiGPU(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=table"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=table"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) for _, want := range []string{"GPU 0: GPU A", "GPU 1: GPU B", "avg util:", "total mem:"} { @@ -1084,16 +1098,20 @@ func TestRemoteMetrics_JsonStopped(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=json"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=json"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) - var result map[string]any - if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &result); err != nil { + var results []map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &results); err != nil { t.Fatalf("JSON output is not valid: %v\n%s", err, out) } - if result["state"] != "stopped" { - t.Errorf("expected state=stopped, got %v", result["state"]) + if len(results) != 1 { + t.Fatalf("decoded %d nodes, want 1", len(results)) + } + metrics, _ := results[0]["metrics"].(map[string]any) + if metrics["state"] != "stopped" { + t.Errorf("expected state=stopped, got %v", metrics["state"]) } } @@ -1114,15 +1132,19 @@ func TestRemoteMetrics_JsonWithErrors(t *testing.T) { t.Setenv("SPINLOOP_REMOTE_STATS_URL", server.URL) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=json"}); err != nil { - t.Errorf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=json"}); err != nil { + t.Errorf("cmdMetrics: %v", err) } }) - var result map[string]any - if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &result); err != nil { + var results []map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &results); err != nil { t.Fatalf("JSON output is not valid: %v\n%s", err, out) } - errors, ok := result["errors"].([]any) + if len(results) != 1 { + t.Fatalf("decoded %d nodes, want 1", len(results)) + } + metrics, _ := results[0]["metrics"].(map[string]any) + errors, ok := metrics["errors"].([]any) if !ok || len(errors) == 0 { t.Errorf("expected errors in JSON output:\n%s", out) } @@ -1269,7 +1291,7 @@ func TestRemoteMetrics_WatchBuffersBeforeClear(t *testing.T) { defer func() { metricsWatchInterval = oldInterval }() out := captureStdout(t, func() { - cmdRemoteMetrics([]string{"--env", "default", "--watch", "--format=table"}) + cmdMetrics([]string{"--env", "default", "--watch", "--format=table"}) }) // The clear-screen escape sequence. @@ -1278,7 +1300,7 @@ func TestRemoteMetrics_WatchBuffersBeforeClear(t *testing.T) { // First render must NOT have a clear-screen prefix — there's nothing to // clear yet, and writing clear before content causes a visible flash. firstClear := strings.Index(out, clearScreen) - firstEnv := strings.Index(out, "environment:") + firstEnv := strings.Index(out, "node:") if firstClear != -1 && firstClear < firstEnv { t.Error("first render must not clear screen before rendering content") } @@ -1286,16 +1308,16 @@ func TestRemoteMetrics_WatchBuffersBeforeClear(t *testing.T) { // Subsequent renders DO have clear-screen before the new content, so the // update is in-place. With 3 calls (2 good, 1 error), we expect at least // 2 renders and thus at least 1 clear between them. - count := strings.Count(out, "environment:") + count := strings.Count(out, "node:") if count < 2 { t.Fatalf("expected at least 2 renders, got %d", count) } // There should be at least one clear-screen that appears between the first - // and second "environment:" line. - firstIdx := strings.Index(out, "environment:") - secondIdx := strings.Index(out[firstIdx+len("environment:"):], "environment:") - secondIdx += firstIdx + len("environment:") + // and second "node:" line. + firstIdx := strings.Index(out, "node:") + secondIdx := strings.Index(out[firstIdx+len("node:"):], "node:") + secondIdx += firstIdx + len("node:") between := out[firstIdx:secondIdx] if !strings.Contains(between, clearScreen) { diff --git a/cmd/spinloop/retain_render_test.go b/cmd/spinloop/retain_render_test.go index 0ad97d49..75270c73 100644 --- a/cmd/spinloop/retain_render_test.go +++ b/cmd/spinloop/retain_render_test.go @@ -58,8 +58,8 @@ func TestRemoteMetricsBarKeepsOnTheActiveLine(t *testing.T) { }`) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=bar"}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=bar"}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if line := aLineContaining(out, "2m 5s ago", "keep for 2h"); line == "" { @@ -89,8 +89,8 @@ func TestRemoteMetricsTableKeepsOnTheActiveRow(t *testing.T) { }`) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=table"}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=table"}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if line := aLineContaining(out, "active:", "2m 5s ago", "keep for 2h"); line == "" { @@ -123,8 +123,8 @@ func TestKeepDurationRendersRelatively(t *testing.T) { "retainUntil": "`+deadline+`" }`) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=bar"}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=bar"}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if !strings.Contains(out, want) { @@ -149,8 +149,8 @@ func TestRemoteMetricsStoppedKeptStillShowsKeep(t *testing.T) { "retainUntil": "`+deadline+`" }`) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=" + format}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=" + format}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if !strings.Contains(out, "keep for 4h") { @@ -173,8 +173,8 @@ func TestRemoteMetricsOmitsKeepWhenAbsent(t *testing.T) { "idleSeconds": 125 }`) out := captureStdout(t, func() { - if err := cmdRemoteMetrics([]string{"--env", "default", "--format=" + format}); err != nil { - t.Fatalf("cmdRemoteMetrics: %v", err) + if err := cmdMetrics([]string{"--env", "default", "--format=" + format}); err != nil { + t.Fatalf("cmdMetrics: %v", err) } }) if strings.Contains(out, "keep for") { diff --git a/cmd/spinloop/root_dispatch_test.go b/cmd/spinloop/root_dispatch_test.go index 501d2ee9..cbd7959f 100644 --- a/cmd/spinloop/root_dispatch_test.go +++ b/cmd/spinloop/root_dispatch_test.go @@ -78,6 +78,27 @@ func TestRoot_MovedSpellingsNameTheirNewHome(t *testing.T) { } } +// TestRoot_MovedSubcommandSpellingsNameTheirNewHome pins the same signpost one +// level down, for a subcommand a group used to have: "fleet metrics" and +// "remote status" and their siblings must each name the top-level verb that +// replaced them, rather than cobra's ordinary unknown-command error. +func TestRoot_MovedSubcommandSpellingsNameTheirNewHome(t *testing.T) { + isolateConfig(t) + + for spelling, newHome := range movedSubcommands { + words := strings.Fields(spelling) + _, err := rootExec(t, words...) + if err == nil { + t.Errorf("%s: expected an error, got none", spelling) + continue + } + want := fmt.Sprintf("%q moved: run spinloop %s", spelling, newHome) + if !strings.Contains(err.Error(), want) { + t.Errorf("%s: error = %q, want it to contain %q", spelling, err.Error(), want) + } + } +} + // TestRoot_HarnessSubcommandsDispatch pins that each of harness's six // subcommands runs the subcommand and never launches the agent — the dispatch // the grouping depends on. diff --git a/cmd/spinloop/status.go b/cmd/spinloop/status.go index 3ffd6a96..1387213a 100644 --- a/cmd/spinloop/status.go +++ b/cmd/spinloop/status.go @@ -15,7 +15,7 @@ import ( ) func statusCmd() *cobra.Command { - var path, envName string + var path, envName, spinloopPath string c := &cobra.Command{ Use: "status", Short: "report every engine's state", @@ -36,6 +36,9 @@ applies to every node.`, SilenceUsage: true, RunE: func(c *cobra.Command, _ []string) error { resolve(c) + if err := applyReadSpinloopEnv(spinloopPath); err != nil { + return err + } cfg, err := resolveFleetTarget(fleetTarget{envName: envName, fleetPath: path}) if err != nil { return err @@ -48,6 +51,7 @@ applies to every node.`, fs := c.Flags() fs.StringVarP(&path, "fleet", "f", "", fleetFileUsage) fs.StringVar(&envName, "env", "", envFlagTargetUsage) + registerSpinloopEnvFlag(fs, &spinloopPath) c.ValidArgsFunction = noPositionals compRegister(c, "fleet", compFiles) compRegister(c, "env", compEnvs) diff --git a/cmd/spinloop/status_render.go b/cmd/spinloop/status_render.go index 727ba427..785d72c3 100644 --- a/cmd/spinloop/status_render.go +++ b/cmd/spinloop/status_render.go @@ -31,6 +31,14 @@ type statusFact struct { // running engine that has answered is the ordinary case and needs no mark, // and an absent reading is not evidence of anything to report. Ready string + // Endpoint is where this node's engine answers, for a node whose address + // the control plane publishes. Empty for a daemon, whose address a client + // composes from the host it already has. + Endpoint string + // RetainUntil is the instance's retention deadline while it has one, RFC + // 3339. Empty for a node that is not retained, and for every node that is + // a machine rather than an instance. + RetainUntil string } // servingText is the "what it serves" text: runner and model, then the uptime and @@ -69,5 +77,16 @@ func (f statusFact) servingText() string { if f.Version != "" { serving += fmt.Sprintf(" (%s)", f.Version) } + // How long an instance is kept, where it is kept at all — the same figure + // and wording the metrics views draw, so one fact reads one way. + if keep := keepText(f.RetainUntil, metricsNow()); keep != "" { + serving += " (" + keep + ")" + } + // Where its engine answers, last because it is the longest and the least + // often read: a node's address matters when you are about to use it, not + // when you are scanning states. + if f.Endpoint != "" { + serving += " " + f.Endpoint + } return serving } diff --git a/docs/commands/alias.md b/docs/commands/alias.md index 64280377..42b4b934 100644 --- a/docs/commands/alias.md +++ b/docs/commands/alias.md @@ -40,7 +40,7 @@ It is stored and resolved just like a local one — see export SPINLOOP_ALIAS=big spinloop harness apply # the same as `spinloop harness apply big` spinloop serve -spinloop remote status +spinloop status --env ``` The order is the argument you typed, then `SPINLOOP_ALIAS`, then `./Spinloop`. Two diff --git a/docs/commands/dashboard.md b/docs/commands/dashboard.md index 110b6a00..af87e6be 100644 --- a/docs/commands/dashboard.md +++ b/docs/commands/dashboard.md @@ -10,7 +10,7 @@ spinloop dashboard --env prod # one environment, as a board of one ``` Each tile draws what the gauge format of -[`spinloop fleet metrics`](fleet.md#metrics) prints for that node — its state, +[`spinloop metrics`](metrics.md) prints for that node — its state, what it serves, the resource gauges, the token counters — so the board and the metrics view cannot word the same reading differently. @@ -44,9 +44,10 @@ node whose token reference cannot be resolved holds its reason for the life of the view. The board needs a terminal. To stream the same readings into a pipe, use -`spinloop fleet metrics --watch`. +`spinloop metrics --watch`. ## See also - [`spinloop status`](status.md) — the same facts as one table, scriptable +- [`spinloop metrics`](metrics.md) — the same readings as text - [`spinloop fleet`](fleet.md) — the fleet file, and driving nodes from the CLI diff --git a/docs/commands/fleet.md b/docs/commands/fleet.md index d48ae75a..bd3025be 100644 --- a/docs/commands/fleet.md +++ b/docs/commands/fleet.md @@ -7,8 +7,8 @@ Observe and drive every engine you run, from one place. Each machine runs ```sh spinloop status # one row per node: state and what it serves spinloop dashboard # the interactive tiled view — watch it, drive it -spinloop fleet metrics # each node's engine + system metrics -spinloop fleet metrics -w # the same, redrawn in place until interrupted +spinloop metrics # each node's engine + system metrics +spinloop metrics -w # the same, redrawn in place until interrupted spinloop fleet route my-spinloop # which node a harness launch would pick spinloop fleet start gpu-box # start one or more nodes' engines spinloop fleet start --all # start every node in the fleet @@ -48,7 +48,7 @@ thing, so `--env` lets you skip writing the file: ```sh spinloop status --env qwen # the same row a one-node fleet file gives spinloop dashboard --env qwen # the tiled view, on one environment -spinloop fleet logs --env qwen # its engine's log +spinloop logs --env qwen # its engine's log ``` Because such a fleet has no file, it carries none of the settings a fleet file @@ -128,9 +128,9 @@ has been started at all. ## Metrics -`spinloop fleet metrics` renders each node's engine and system metrics in the +`spinloop metrics` renders each node's engine and system metrics in the same `gauge` (default), `bar`, `table`, and `json` formats as -[`spinloop remote metrics`](remote.md) — they share the renderers, so a node in +[`spinloop metrics --env `](remote.md) — they share the renderers, so a node in your fleet and a cloud endpoint look the same. `gauge` draws the current reading per series as a filled progress gauge; `--format=bar` draws each series as a sparkline of the node's daemon's retained history instead, and a @@ -171,7 +171,7 @@ silently missing whatever was down: ## The dashboard `spinloop dashboard` is that same board as a live view: one tile per -node, repainted in place, each drawing exactly what `fleet metrics`' gauge +node, repainted in place, each drawing exactly what `metrics`' gauge format prints for the node — state and uptime, what it serves, the CPU/GPU/RAM gauges, the token counters — so the view and the one-shot command never word a number differently. `g` toggles every tile between the gauge drawing @@ -241,14 +241,14 @@ next round rather than waiting out its full cadence. Everything else in the view is `status`/`metrics`/`logs` in place — it is read-only apart from those four action keys. It needs a real terminal: a -piped run is refused, and it says so by way of `fleet metrics --watch`, which +piped run is refused, and it says so by way of `metrics --watch`, which is the streamable surface. ### The node detail view `Enter` on a tile opens a full-screen view of that node in place of the grid: its metrics, unclipped to the tile's 42 columns, its engine log tailed and -followed the way `fleet logs -f` follows one node, and a footer naming the +followed the way `logs -f` follows one node, and a footer naming the keys the view answers to. `Esc` closes it and returns to the grid with the same node still selected. @@ -266,7 +266,7 @@ under you. The one exception is the keep prompt: while it is open it answers to `q`/`Ctrl+C` the way the stop confirmation does, cancelling and leaving. The rest of the fleet keeps refreshing behind the view, and any action already in flight on another node keeps running. A node whose engine has never run shows the same -explanation `fleet logs` gives for it, not an empty pane. +explanation `logs` gives for it, not an empty pane. `f` pauses and resumes the log's follow, independently of everything else in the view — the metrics section keeps refreshing either way. The header names @@ -276,14 +276,14 @@ poll that simply ran late. ## Logs -`spinloop fleet logs` prints what your engines actually said — the answer to the +`spinloop logs` prints what your engines actually said — the answer to the question `status` raises when it reports a node as `crashed`. ```sh -spinloop fleet logs # the tail of every node's engine log -spinloop fleet logs gpu-box # just that node -spinloop fleet logs -f # follow, until you interrupt it -spinloop fleet logs --limit 500 # more backlog per node +spinloop logs # the tail of every node's engine log +spinloop logs gpu-box # just that node +spinloop logs -f # follow, until you interrupt it +spinloop logs --limit 500 # more backlog per node ``` Each node's daemon captures its engine's stdout and stderr to a file, and diff --git a/docs/commands/gateway.md b/docs/commands/gateway.md index 9c8a7e43..9f11e055 100644 --- a/docs/commands/gateway.md +++ b/docs/commands/gateway.md @@ -85,7 +85,7 @@ The list is what a request can reach, so it is bounded by what the gateway can start: a running node contributes only what it reports — a running engine is never displaced to make room. A deployed-but-stopped `kind: remote` environment contributes the model id its own stats reply reports — read -directly from its stored deploy config, the way `spinloop remote metrics` +directly from its stored deploy config, the way `spinloop metrics --env ` already reads it, since its status reply carries no such facts while stopped — since the gateway can wake it the same way it wakes a `kind: daemon` node; one with nothing deployed contributes nothing, and diff --git a/docs/commands/index.md b/docs/commands/index.md index 007ae9b6..f7a30547 100644 --- a/docs/commands/index.md +++ b/docs/commands/index.md @@ -7,6 +7,8 @@ help` the usage summary. | ------- | ------------ | | [`spinloop status`](status.md) | What every engine you run is doing, one row each — a fleet, or one environment | | [`spinloop dashboard`](dashboard.md) | The live tiled view of the same, with the keys to drive it | +| [`spinloop metrics`](metrics.md) | What each engine is doing with its hardware, and what it has cost | +| [`spinloop logs`](logs.md) | What each engine has said — a daemon's log file or an environment's log store | | [`spinloop code`](code.md) | Launch the active harness — a one-word shortcut for `spinloop harness open` | | [`spinloop harness`](harness.md) | Configure the agent (add, remove, apply, unapply, show, export), launch it (open), and set the default (config) | | [`spinloop provider`](provider.md) | Work with the provider catalogue (list, init) | diff --git a/docs/commands/logs.md b/docs/commands/logs.md new file mode 100644 index 00000000..cf36a786 --- /dev/null +++ b/docs/commands/logs.md @@ -0,0 +1,57 @@ +# spinloop logs + +What every engine you run has said — through whatever the node answers with, a +daemon's log file or a cloud environment's log store. + +```sh +spinloop logs # every node in this directory's fleet.yaml +spinloop logs studio # just that node +spinloop logs --env prod # one registered environment, no file needed +spinloop logs -f # follow +spinloop logs --env prod --source boot --since 30m +``` + +With no node named it reads every node in the target; naming one reads only +that one. Nodes are read concurrently, so the command takes as long as the +slowest rather than all of them added up. Lines are prefixed with their node's +name only when more than one node produced output — reading one node reads like +that node's own log. + +## Which target + +| | target | +| --- | --- | +| `--env ` | one registered environment | +| `--fleet ` | that fleet file | +| neither | the `fleet.yaml` in the working directory | + +**`--fleet` has no `-f` here.** `-f` is `--follow`, as it is on every other +tool that follows something, and a flag cannot carry two meanings on one +command line. + +## Which flags apply to which nodes + +A daemon's log is one file on one machine, read from a byte offset. A cloud +environment's is a store that can be queried. The three query flags apply to +the nodes that have one and leave the rest read as they would be without them — +so `--source boot` against a fleet of daemons returns their output rather than +nothing. + +| flag | cloud environment | daemon node | +| --- | --- | --- | +| `--source engine\|boot\|all` | selects which log | — | +| `--since ` | bounds how far back (default 1h) | — | +| `--instance ` | restricts to one instance | — | +| `--follow`, `--limit`, `--format` | applies | applies | + +## Following + +`-f`/`--follow` keeps printing as output arrives, across every node in the +target at once. A node that goes unreachable mid-follow is reported rather +than dropped silently. + +## See also + +- [`spinloop status`](status.md) — what each engine is +- [`spinloop metrics`](metrics.md) — what they are doing with the hardware +- [`spinloop serve`](serve.md) — the log of an engine you are running in the foreground diff --git a/docs/commands/metrics.md b/docs/commands/metrics.md new file mode 100644 index 00000000..8861d52e --- /dev/null +++ b/docs/commands/metrics.md @@ -0,0 +1,87 @@ +# spinloop metrics + +What every engine you run is doing with its hardware — resource use, token and +request counters, and what the node it runs on is. + +```sh +spinloop metrics # the fleet.yaml in this directory +spinloop metrics --fleet ./cluster.yaml +spinloop metrics --env prod # one registered environment, no file needed +spinloop metrics -w # redraw in place every 60 seconds +spinloop metrics --cost # add what each priceable node has spent +``` + +## Which target + +The three ways every command that reads engines takes one: + +| | target | +| --- | --- | +| `--env ` | one registered environment | +| `--fleet ` (`-f`) | that fleet file | +| neither | the `fleet.yaml` in the working directory | + +`--env` and `--fleet` name two different things, so passing both fails saying +so. With none of the three resolvable the command fails naming all of them. + +## Formats + +`--format` selects one of four. All four report the same facts; they differ in +how much of the screen they spend. + +| format | what it draws | +| --- | --- | +| `gauge` (default) | a header line per node, then the current reading as gauges | +| `bar` | the same, with each series as a sparkline of the retained history | +| `table` | every fact on its own line, then the counters and figures | +| `json` | one object per node, for a program to read | + +`table` is the one to use when you want everything a node can say: + +``` +node: prod +state: running +instance: i-0abc123 +instanceType: g6e.xlarge +runner: llamacpp +model: org/qwen3-27b +version: 1.41.0 +endpoint: http://198.51.100.1:8000/v1 +uptime: 2h 15m 0s +active: 2m 5s ago + + running: 1 + prompt tokens: 12000 + … +``` + +## Which flags apply to which nodes + +Some facts only one kind of node has. A flag that asks for one applies to the +nodes that can answer it and leaves the rest exactly as they read without it — +a fleet of daemons asked for a cost shows no cost and still succeeds. + +| flag / field | cloud environment | daemon node | +| --- | --- | --- | +| `--cost` | priced from its instance type and region | — | +| `instance`, `instanceType` | reported | — | +| `version` | reported | — | +| `endpoint` | reported | — | +| everything else | reported | reported | + +A daemon node runs on a machine you already own: it has no hourly rate, no +instance id, and no address the control plane publishes. Its release is on its +[status](status.md) row instead. + +## Watching + +`-w`/`--watch` redraws every 60 seconds, clearing the screen each time rather +than accumulating scrollback, and exits cleanly on interrupt. For an +interactive view that also drives the nodes, use +[`spinloop dashboard`](dashboard.md). + +## See also + +- [`spinloop status`](status.md) — what each engine is, one row each +- [`spinloop logs`](logs.md) — what they have said +- [`spinloop dashboard`](dashboard.md) — these readings, live and interactive diff --git a/docs/commands/remote.md b/docs/commands/remote.md index a846a467..41a786ae 100644 --- a/docs/commands/remote.md +++ b/docs/commands/remote.md @@ -10,8 +10,8 @@ spinloop remote auth # store or report the credential this machine signs w spinloop remote bake # bake the runner AMI(s) an environment runs from spinloop remote deploy # create an endpoint (environment) and tell it what to serve spinloop remote start # boot it; with --print-env, prints the exports your agent needs -spinloop remote status # is it up? is it healthy? -spinloop remote logs # what did it say? (readable after it's gone) +spinloop status --env # is it up? is it healthy? +spinloop logs --env # what did it say? (readable after it's gone) spinloop remote pause # stop it now; a later start re-wakes it spinloop remote restart # fresh engine, same address: stop it and wake it again spinloop remote keep 4h # prevent the idle sweep from stopping it for 4 hours @@ -122,7 +122,7 @@ the deployment that owns it. A `BASEURL` in the Spinloop wins if you set one. Name the environment with the `--env` flag: ```sh -spinloop remote status --env qwen3.6-27b-prod +spinloop status --env --env qwen3.6-27b-prod ``` The flag selects a **named environment** from the @@ -231,9 +231,9 @@ a stored key covers them too. ## Checking on an endpoint ```sh -spinloop remote status # is it up, is it healthy, where is it -spinloop remote metrics # what is it doing — tokens, GPU, CPU, RAM -spinloop remote metrics -w # the same, redrawn every 60 seconds +spinloop status --env # is it up, is it healthy, where is it +spinloop metrics --env # what is it doing — tokens, GPU, CPU, RAM +spinloop metrics --env -w # the same, redrawn every 60 seconds ``` `metrics` draws its resource series as **gauge** format by default: the @@ -317,10 +317,10 @@ endpoint's base URL when it serves, so you can check the address is unchanged. ## Reading the logs ```sh -spinloop remote logs # the last hour of engine output -spinloop remote logs --source boot # the start-up log, before the engine ran -spinloop remote logs --since 6h --limit 500 -spinloop remote logs -f # follow, until you interrupt it +spinloop logs --env # the last hour of engine output +spinloop logs --env --source boot # the start-up log, before the engine ran +spinloop logs --env --since 6h --limit 500 +spinloop logs --env -f # follow, until you interrupt it ``` Instances ship two logs to CloudWatch: the inference engine's own output, and diff --git a/docs/commands/status.md b/docs/commands/status.md index 8a46e868..13bc9dbe 100644 --- a/docs/commands/status.md +++ b/docs/commands/status.md @@ -13,7 +13,7 @@ spinloop status --env prod # one registered environment, no file need NODE STATE SERVING studio running llamacpp qwen3-27b (up 2h 14m) (active 3m ago) (1.40.0) gpu-box stopped -prod running vllm org/model (active 12s ago) +prod running vllm org/model (active 12s ago) (1.41.0) (kept 3h 20m) http://198.51.100.1:8000/v1 dead unreachable dial tcp 198.51.100.9:4242: connect: connection refused ``` @@ -50,15 +50,23 @@ and name that machine in a `fleet.yaml`. cloud environment whose endpoint the control plane reports unhealthy reads the same way. -Two things are deliberately not columns, because only one kind of node has -them: an environment's endpoint address, which is -[`spinloop remote env`](remote.md), and its retention deadline, which -[`spinloop fleet metrics`](fleet.md#metrics) and -[`spinloop dashboard`](dashboard.md) show. +A node that runs on a cloud instance adds what only it can report: the +release on it, where its engine answers, and how long it is retained. A daemon +node shows none of those — it has no answer for them — and the row is shorter +for it. + +## Which flags apply to which nodes + +| flag | applies to | +| --- | --- | +| `--env`, `--fleet`, `-O`/`--spinloop` | every target | + +`status` takes no flag that only some node kinds can answer. `metrics` and +`logs` do — see their pages. ## See also - [`spinloop dashboard`](dashboard.md) — the same facts, live and interactive +- [`spinloop metrics`](metrics.md) — what the engines are doing with the hardware +- [`spinloop logs`](logs.md) — what they have said - [`spinloop fleet`](fleet.md) — driving the nodes rather than reading them -- [`spinloop remote status`](remote.md) — one environment, with its address, - version and keep deadline in full diff --git a/docs/maintainer/internals.md b/docs/maintainer/internals.md index e5e5d92e..f5a263e8 100644 --- a/docs/maintainer/internals.md +++ b/docs/maintainer/internals.md @@ -21,7 +21,7 @@ These are mistakes already made here; each was silent rather than loud, which is **A preset dialect is not interchangeable.** `internal/preset` parses any INI the same way, so an oMLX preset fed through llama.cpp's dialect parses cleanly and produces a *wrong* command: the alias table rewrites `m` to `--model` and `c` to `--ctx-size`, and the boolean table drops a `key = 0` entirely. Nothing errors — the server just receives flags it does not accept, or silently loses a setting. The dialect always comes from the engine `PROVIDER` names, never from the file. -**A busy engine does not answer its own metrics endpoint.** llama.cpp serves `/metrics` from the same queue it serves inference from, so a scrape taken while a prompt is being processed waits for that prompt to finish — tens of seconds on a long context. `/v1/metrics` used to scrape inline, so the handler blocked for as long as the engine had work, and `spinloop fleet metrics` (5s client timeout, against a handler whose scrape timeout was also 5s) could not win that race: the view went blank exactly when there was something worth watching, reporting the node as `unreachable`. The counters now come from the background sampler's last reading (`engineSample`), which the daemon takes every 15s regardless of who is asking — so the handler never waits on the engine, and staleness is bounded by the sample interval. Three things to preserve if you touch this: the *cached scrape error* is still reported, because silently omitting the token block is what once hid a scraper pointed at the wrong port; the sample is forgotten on start, so one engine's counters are never reported against the next; and the sampler retries at `catchUpInterval` (1s) until a reading lands, dropping to the full interval only afterwards — without that, a freshly started engine reports no counters for up to 15s, which is exactly the window someone watching a node they just started is looking at. +**A busy engine does not answer its own metrics endpoint.** llama.cpp serves `/metrics` from the same queue it serves inference from, so a scrape taken while a prompt is being processed waits for that prompt to finish — tens of seconds on a long context. `/v1/metrics` used to scrape inline, so the handler blocked for as long as the engine had work, and `spinloop metrics` (5s client timeout, against a handler whose scrape timeout was also 5s) could not win that race: the view went blank exactly when there was something worth watching, reporting the node as `unreachable`. The counters now come from the background sampler's last reading (`engineSample`), which the daemon takes every 15s regardless of who is asking — so the handler never waits on the engine, and staleness is bounded by the sample interval. Three things to preserve if you touch this: the *cached scrape error* is still reported, because silently omitting the token block is what once hid a scraper pointed at the wrong port; the sample is forgotten on start, so one engine's counters are never reported against the next; and the sampler retries at `catchUpInterval` (1s) until a reading lands, dropping to the full interval only afterwards — without that, a freshly started engine reports no counters for up to 15s, which is exactly the window someone watching a node they just started is looking at. **An exported-but-empty variable is a gap, not a choice.** `setEnvIfAbsent` keys on the variable being *present*, so an `OPENAI_BASE_URL=` in the environment counts as set and suppresses the value routing meant to supply — leaving the agent pointed at nothing. `setEnvIfBlank` is the one to use for an address or a key, matching what `harnessEnv` already does for the remote endpoint's values; the distinction only shows up when something exports an empty string, which shells do more often than you would think. @@ -54,7 +54,7 @@ A few Bubble Tea/lipgloss specifics that are easy to break by "simplifying": - One function, `dashNodeView`, produces both a panel's lines and its health tier, from the reading, the action, the current time, and how old a reading of that node may be. Nothing in it reads a clock, so every pairing of a start's phase against a reading can be enumerated in a test. - A tile's first line is a header bar drawn in raw ANSI — the body is one plain string under a single lipgloss style, so per-character colour cannot be lipgloss's. The board's own title bar (`dashTitleBar`) uses lipgloss instead, and the two share one surface index (`barSurface`) because they are set through different mechanisms and would otherwise drift. - A grid row joins the *corresponding lines* of the tiles it places, not the tile blocks — joining whole blocks glues the second tile's top border to the first tile's bottom border and shifts its body down a line. -- A tile's content is exactly the lines `fleet metrics` prints for the node in the board's current format (`renderStatBars`/`renderStatGauges`/`renderTokenLines` are shared, not reimplemented), so the panel and `fleet metrics` can never disagree on a number. The format is board-wide, in `dashModel.gauge`, toggled by `g`, opening in gauge; the tile draws the sparkline at `dashBarLineW`, chosen so a full row (label, glyphs, trailing percentage) fits the tile's width exactly. +- A tile's content is exactly the lines `metrics` prints for the node in the board's current format (`renderStatBars`/`renderStatGauges`/`renderTokenLines` are shared, not reimplemented), so the panel and `metrics` can never disagree on a number. The format is board-wide, in `dashModel.gauge`, toggled by `g`, opening in gauge; the tile draws the sparkline at `dashBarLineW`, chosen so a full row (label, glyphs, trailing percentage) fits the tile's width exactly. Behavior (panel contents, refresh cadence, start/stop/abort semantics, the detail view) is specified in `openspec/specs/fleet-client/spec.md`. diff --git a/examples/fleet-docker/README.md b/examples/fleet-docker/README.md index 50d6dee4..28dc5a8b 100644 --- a/examples/fleet-docker/README.md +++ b/examples/fleet-docker/README.md @@ -14,7 +14,7 @@ set -a && . ./.env && set +a spinloop status --fleet ./fleet.yaml spinloop fleet start studio --fleet ./fleet.yaml -spinloop fleet metrics -w --fleet ./fleet.yaml +spinloop metrics -w --fleet ./fleet.yaml ``` ``` diff --git a/examples/fleet-docker/compose.yaml b/examples/fleet-docker/compose.yaml index abcfefca..8c56127b 100644 --- a/examples/fleet-docker/compose.yaml +++ b/examples/fleet-docker/compose.yaml @@ -4,7 +4,7 @@ # cp .env.example .env # docker compose up -d --build # spinloop status -# spinloop fleet metrics -w +# spinloop metrics -w # # Every node carries the fleet's shared token: the daemon refuses to listen on # a non-loopback address without one, so any node reachable across the network diff --git a/examples/fleet-docker/run-tests.sh b/examples/fleet-docker/run-tests.sh index 5511d17b..1d5d224f 100755 --- a/examples/fleet-docker/run-tests.sh +++ b/examples/fleet-docker/run-tests.sh @@ -166,6 +166,33 @@ status() { "${SPINLOOP_BIN}" status --fleet "${HERE}/fleet.yaml" 2>/dev/null } +####################################### +# `spinloop metrics` against this example's fleet file. metrics is a top-level +# verb rather than a fleet subcommand, so it needs its own wrapper. +# Globals: +# SPINLOOP_BIN, HERE +# Arguments: +# Arguments to pass to `spinloop metrics`. +# Outputs: +# The command's stdout; stderr is discarded so assertions read cleanly. +####################################### +metrics() { + "${SPINLOOP_BIN}" metrics "$@" --fleet "${HERE}/fleet.yaml" 2>/dev/null +} + +####################################### +# `spinloop logs` against this example's fleet file, for the same reason. +# Globals: +# SPINLOOP_BIN, HERE +# Arguments: +# Arguments to pass to `spinloop logs`. +# Outputs: +# The command's stdout; stderr is discarded so assertions read cleanly. +####################################### +logs() { + "${SPINLOOP_BIN}" logs "$@" --fleet "${HERE}/fleet.yaml" 2>/dev/null +} + ####################################### # As fleet(), but merging stderr — for assertions about error messages, which # the CLI writes to stderr. @@ -289,7 +316,7 @@ wait_for_metrics() { local deadline=$((SECONDS + timeout)) local out while (( SECONDS < deadline )); do - out="$(fleet metrics)" + out="$(metrics)" if [[ "${out}" == *"prompt tokens"* && "${out}" == *"RAM"* ]]; then return 0 fi @@ -503,7 +530,7 @@ STUB # own argv into the engine log, which is where that can be checked — in the # process list the shim has already exec'd and replaced itself. local enginelog - enginelog="$(fleet logs studio --limit 50 2>/dev/null || true)" + enginelog="$(logs studio --limit 50 2>/dev/null || true)" assert_contains "the engine was gated by file" "${enginelog}" "--api-key-file" assert_not_contains "the key itself never reaches the command line" \ "${enginelog}" "${FLEET_TOKEN}" @@ -530,13 +557,13 @@ test_metrics() { wait_for_metrics 30 || true local out - out="$(fleet metrics)" + out="$(metrics)" # The counters the fake engine serves, parsed by spinloop's own collector. assert_contains "token counters reach the fleet view" "${out}" "prompt tokens" assert_contains "prompt token count is the engine's" "${out}" "4096" assert_contains "resource bars are rendered" "${out}" "RAM" - out="$(fleet metrics --format=json)" + out="$(metrics --format=json)" assert_contains "json is labelled by node" "${out}" '"node": "gpu-box"' assert_contains "json reports the outcome" "${out}" '"outcome": "ok"' } diff --git a/examples/fleet-mixed/README.md b/examples/fleet-mixed/README.md index d4b4f512..2100bbf1 100644 --- a/examples/fleet-mixed/README.md +++ b/examples/fleet-mixed/README.md @@ -49,7 +49,7 @@ remote`](../../docs/commands/remote.md) — just one command for both. ```sh spinloop status # one row per node: the machine and the environments -spinloop fleet metrics -w # a live dashboard +spinloop metrics -w # a live dashboard spinloop fleet start qwen # wake a sleeping environment from zero spinloop fleet stop gpu-box # stop the machine's engine ``` diff --git a/examples/fleet-mixed/fleet.yaml b/examples/fleet-mixed/fleet.yaml index ebfd0bba..c4256d2b 100644 --- a/examples/fleet-mixed/fleet.yaml +++ b/examples/fleet-mixed/fleet.yaml @@ -2,7 +2,7 @@ # environments, observed side by side. The same command reaches every node, # whatever kind it is. # -# spinloop status, or spinloop fleet metrics -w +# spinloop status, or spinloop metrics -w # # A daemon node names the machine (host) and the variable holding its bearer # token (tokenEnv). A remote node names its environment — its node name *is* the diff --git a/examples/fleet-remote/README.md b/examples/fleet-remote/README.md index c56a47db..e78d4f8e 100644 --- a/examples/fleet-remote/README.md +++ b/examples/fleet-remote/README.md @@ -61,7 +61,7 @@ From any machine your AWS credentials reach: ```sh spinloop status # one row per environment -spinloop fleet metrics -w # a live dashboard +spinloop metrics -w # a live dashboard spinloop fleet start qwen # wake a sleeping environment from zero spinloop fleet stop qwen # scale it back down ``` diff --git a/examples/fleet-remote/fleet.yaml b/examples/fleet-remote/fleet.yaml index 6eafd174..b69e1035 100644 --- a/examples/fleet-remote/fleet.yaml +++ b/examples/fleet-remote/fleet.yaml @@ -1,7 +1,7 @@ # A fleet of `spinloop remote` environments, observed like machines. # # spinloop status # one row per environment: state and what it serves -# spinloop fleet metrics -w # a live dashboard +# spinloop metrics -w # a live dashboard # spinloop fleet deploy --all # create both environments from this file # # Each node's `name` is the registered environment it drives — one per diff --git a/examples/fleet/README.md b/examples/fleet/README.md index 7e00082c..538c4c44 100644 --- a/examples/fleet/README.md +++ b/examples/fleet/README.md @@ -20,7 +20,7 @@ automatically since the subdirectory's name matches the node's), so ```sh cp .env.example .env # fill in the fleet's shared token spinloop status -spinloop fleet metrics -w +spinloop metrics -w spinloop fleet start gpu-box ``` diff --git a/examples/fleet/fleet.yaml b/examples/fleet/fleet.yaml index eeb3d5d4..c6352136 100644 --- a/examples/fleet/fleet.yaml +++ b/examples/fleet/fleet.yaml @@ -1,7 +1,7 @@ # A fleet: the machines running `spinloop daemon`, and how to reach each one. # # spinloop status # one row per node -# spinloop fleet metrics -w # a live dashboard +# spinloop metrics -w # a live dashboard # # This file holds no secrets. A node that needs a bearer token names the # environment variable holding it (tokenEnv); the value comes from your diff --git a/examples/gateway-docker/run-tests.sh b/examples/gateway-docker/run-tests.sh index 39fb25fe..a490dad2 100755 --- a/examples/gateway-docker/run-tests.sh +++ b/examples/gateway-docker/run-tests.sh @@ -169,6 +169,33 @@ status() { "${SPINLOOP_BIN}" status --fleet "${HERE}/fleet.yaml" 2>/dev/null } +####################################### +# `spinloop metrics` against this example's fleet file. metrics is a top-level +# verb rather than a fleet subcommand, so it needs its own wrapper. +# Globals: +# SPINLOOP_BIN, HERE +# Arguments: +# Arguments to pass to `spinloop metrics`. +# Outputs: +# The command's stdout; stderr is discarded so assertions read cleanly. +####################################### +metrics() { + "${SPINLOOP_BIN}" metrics "$@" --fleet "${HERE}/fleet.yaml" 2>/dev/null +} + +####################################### +# `spinloop logs` against this example's fleet file, for the same reason. +# Globals: +# SPINLOOP_BIN, HERE +# Arguments: +# Arguments to pass to `spinloop logs`. +# Outputs: +# The command's stdout; stderr is discarded so assertions read cleanly. +####################################### +logs() { + "${SPINLOOP_BIN}" logs "$@" --fleet "${HERE}/fleet.yaml" 2>/dev/null +} + ####################################### # As fleet(), but merging stderr — for assertions about error messages. # Globals: @@ -458,7 +485,7 @@ test_engine_key_gating() { assert_not_contains "the node never discloses the key" "${status}" "${NODE_A_ENGINE_KEY}" local enginelog - enginelog="$(fleet logs node-a --limit 50 2>/dev/null || true)" + enginelog="$(logs node-a --limit 50 2>/dev/null || true)" assert_contains "the engine was gated by file" "${enginelog}" "--api-key-file" assert_not_contains "the key itself never reaches the command line" \ "${enginelog}" "${NODE_A_ENGINE_KEY}" diff --git a/examples/llamacpp/qwen3.8-27b/README.md b/examples/llamacpp/qwen3.8-27b/README.md index a0ce1985..5326ef80 100644 --- a/examples/llamacpp/qwen3.8-27b/README.md +++ b/examples/llamacpp/qwen3.8-27b/README.md @@ -231,7 +231,7 @@ doesn't have these weights cached yet, deploy fetches them in the background eval "$(spinloop remote start --env qwen3.8-27b)" # boots the instance # (~10 min cold), exports # OPENAI_BASE_URL / OPENAI_API_KEY -spinloop remote status --env qwen3.8-27b # is it up, is it healthy +spinloop status --env --env qwen3.8-27b # is it up, is it healthy spinloop harness apply --env qwen3.8-27b # point opencode at the running endpoint spinloop harness open --env qwen3.8-27b # work spinloop remote stop --env qwen3.8-27b # done — shut it down now rather diff --git a/internal/fleet/capabilities_test.go b/internal/fleet/capabilities_test.go index 2c8a8abe..a3c0dacb 100644 --- a/internal/fleet/capabilities_test.go +++ b/internal/fleet/capabilities_test.go @@ -57,8 +57,8 @@ func TestDaemonNodeImplementsNoCloudCapability(t *testing.T) { if _, ok := node.(SourceLogger); ok { t.Error("a daemon node should not implement SourceLogger: its log is a byte offset") } - if _, ok := node.(Versioner); ok { - t.Error("a daemon node should not implement Versioner: its status reply carries the version") + if _, ok := node.(InstanceReporter); ok { + t.Error("a daemon node should not implement InstanceReporter: it runs on a machine, not an instance") } } @@ -82,16 +82,16 @@ func TestRemoteNodeImplementsTheCloudCapabilities(t *testing.T) { if _, ok := node.(SourceLogger); !ok { t.Error("a cloud node should implement SourceLogger") } - if _, ok := node.(Versioner); !ok { - t.Error("a cloud node should implement Versioner") + if _, ok := node.(InstanceReporter); !ok { + t.Error("a cloud node should implement InstanceReporter") } } -// The version comes from the reading already taken, not from a second call. -func TestRemoteNodeVersionComesFromTheMetricsReading(t *testing.T) { +// The instance facts come from the reading already taken, not a second call. +func TestRemoteNodeInstanceComesFromTheMetricsReading(t *testing.T) { stubAWSCreds(t) calls := 0 - srv := countingStatsServer(t, `{"state":"running","version":"1.40.0","instanceType":"g6e.xlarge","uptimeSeconds":7200}`, &calls) + srv := countingStatsServer(t, `{"state":"running","version":"1.40.0","instanceId":"i-0abc","instanceType":"g6e.xlarge","uptimeSeconds":7200}`, &calls) registerStatsEnv(t, "prod", srv) cfg, err := ForEnvironment("prod") if err != nil { @@ -102,19 +102,20 @@ func TestRemoteNodeVersionComesFromTheMetricsReading(t *testing.T) { t.Fatal(err) } - v, _ := node.(Versioner) - if got := v.Version(); got != "" { - t.Errorf("version before any reading = %q, want empty", got) + i, _ := node.(InstanceReporter) + if got := i.Instance(); got != (Instance{}) { + t.Errorf("instance before any reading = %+v, want empty", got) } if _, err := node.Metrics(context.Background()); err != nil { t.Fatal(err) } before := calls - if got := v.Version(); got != "1.40.0" { - t.Errorf("version = %q, want the reading's", got) + got := i.Instance() + if got.Version != "1.40.0" || got.ID != "i-0abc" || got.Type != "g6e.xlarge" { + t.Errorf("instance = %+v, want the reading's facts", got) } if calls != before { - t.Errorf("reading the version cost %d extra call(s), want none", calls-before) + t.Errorf("reading the instance cost %d extra call(s), want none", calls-before) } } diff --git a/internal/fleet/capability_calls_test.go b/internal/fleet/capability_calls_test.go new file mode 100644 index 00000000..4a72b871 --- /dev/null +++ b/internal/fleet/capability_calls_test.go @@ -0,0 +1,191 @@ +package fleet + +import ( + "context" + "testing" + "time" + + "github.com/spinloop-ai/spinloop/internal/daemon" + "github.com/spinloop-ai/spinloop/internal/inference" + "github.com/spinloop-ai/spinloop/internal/metrics" + "github.com/spinloop-ai/spinloop/internal/remote" +) + +// stubNode is a node that answers whatever the test gives it and implements +// nothing else, standing in for a kind with no cloud capabilities. +type stubNode struct { + name string + stats metrics.Stats + logs daemon.LogsResponse + err error + // readOffset records that the node was read the ordinary way, which is + // what a node with no queryable log must fall back to. + readOffset bool +} + +func (n *stubNode) Name() string { return n.name } +func (n *stubNode) Status(context.Context) (daemon.StatusResponse, error) { + return daemon.StatusResponse{}, n.err +} +func (n *stubNode) Metrics(context.Context) (metrics.Stats, error) { return n.stats, n.err } +func (n *stubNode) Start(context.Context) (daemon.StatusResponse, error) { + return daemon.StatusResponse{}, nil +} +func (n *stubNode) StartWith(context.Context, *inference.DeployConfig, string) (daemon.StatusResponse, error) { + return daemon.StatusResponse{}, nil +} +func (n *stubNode) Stop(context.Context) (daemon.StatusResponse, error) { + return daemon.StatusResponse{}, nil +} +func (n *stubNode) Logs(_ context.Context, _ int64, _ int) (daemon.LogsResponse, error) { + n.readOffset = true + return n.logs, n.err +} + +// costingNode is a node that can be priced, for asserting the priced call +// reaches its capability. +type costingNode struct { + *stubNode + cost Cost + err error +} + +func (n *costingNode) Cost(context.Context) (Cost, error) { return n.cost, n.err } + +// queryingNode is a node whose log can be narrowed. +type queryingNode struct { + *stubNode + got LogQuery + resp daemon.LogsResponse +} + +func (n *queryingNode) LogsMatching(_ context.Context, q LogQuery) (daemon.LogsResponse, error) { + n.got = q + return n.resp, nil +} + +// A node that can be priced carries its cost; one that cannot is read exactly +// as the unpriced call reads it, and neither fails. +func TestPricedMetricsCall(t *testing.T) { + plain := &stubNode{name: "box", stats: metrics.Stats{State: "running"}} + r := PricedMetricsCall(context.Background(), plain) + if !r.OK() { + t.Fatalf("an unpriceable node should still read: %+v", r) + } + if r.Cost.Reported() { + t.Errorf("a node that cannot be priced reported %+v", r.Cost) + } + + priced := &costingNode{ + stubNode: &stubNode{name: "prod", stats: metrics.Stats{State: "running"}}, + cost: Cost{SoFar: 3.94, PerHour: 1.75}, + } + r = PricedMetricsCall(context.Background(), priced) + if !r.Cost.Reported() || r.Cost.SoFar != 3.94 { + t.Errorf("cost = %+v, want the node's", r.Cost) + } +} + +// A price that cannot be fetched leaves the cost unreported rather than +// failing a reading that otherwise succeeded. +func TestPricedMetricsCallSwallowsAPriceFailure(t *testing.T) { + n := &costingNode{ + stubNode: &stubNode{name: "prod", stats: metrics.Stats{State: "running"}}, + err: context.DeadlineExceeded, + } + r := PricedMetricsCall(context.Background(), n) + if !r.OK() { + t.Fatalf("a failed price should not fail the reading: %+v", r) + } + if r.Cost.Reported() { + t.Errorf("cost = %+v, want none", r.Cost) + } +} + +// A node whose metrics call fails is not asked for a price: there is nothing +// to price and the failure is already the answer. +func TestPricedMetricsCallSkipsAFailedNode(t *testing.T) { + n := &costingNode{ + stubNode: &stubNode{name: "prod", err: context.Canceled}, + cost: Cost{SoFar: 9, PerHour: 9}, + } + r := PricedMetricsCall(context.Background(), n) + if r.OK() { + t.Fatal("a node that did not answer should not read as OK") + } + if r.Cost.Reported() { + t.Errorf("a node that did not answer reported a cost: %+v", r.Cost) + } +} + +// A queryable log is narrowed by what the caller asked; one that is not is +// read from its tail, so the flag narrows what it can and leaves the rest. +func TestQueriedLogsCall(t *testing.T) { + q := LogQuery{Source: "boot", Since: 30 * time.Minute, Instance: "i-42"} + + queryable := &queryingNode{ + stubNode: &stubNode{name: "prod"}, + resp: daemon.LogsResponse{Content: "boot output"}, + } + r := QueriedLogsCall(q, 500)(context.Background(), queryable) + if !r.OK() { + t.Fatalf("queried read failed: %+v", r) + } + if queryable.got.Source != "boot" || queryable.got.Instance != "i-42" { + t.Errorf("the query did not reach the node: %+v", queryable.got) + } + if queryable.got.Limit != 500 { + t.Errorf("limit = %d, want the caller's", queryable.got.Limit) + } + if r.Logs.Content != "boot output" { + t.Errorf("content = %q, want the node's", r.Logs.Content) + } + + plain := &stubNode{name: "box", logs: daemon.LogsResponse{Content: "engine output"}} + r = QueriedLogsCall(q, 500)(context.Background(), plain) + if !plain.readOffset { + t.Error("a node with no queryable log should be read the ordinary way") + } + if r.Logs.Content != "engine output" { + t.Errorf("content = %q, want the node's own output", r.Logs.Content) + } +} + +// A remote environment's queried read reaches its log store with the window +// the caller named. The store is CloudWatch, reached through the AWS SDK +// rather than the control plane's HTTP endpoints, so this substitutes the +// FetchLogsFn variable rather than an httptest server. +func TestRemoteNodeLogsMatching(t *testing.T) { + registerStatsEnv(t, "prod", "http://unused.invalid") + cfg, err := ForEnvironment("prod") + if err != nil { + t.Fatal(err) + } + node, err := cfg.NewNode(cfg.Nodes[0]) + if err != nil { + t.Fatal(err) + } + s, ok := node.(SourceLogger) + if !ok { + t.Fatal("a cloud node should implement SourceLogger") + } + + restore := FetchLogsFn + t.Cleanup(func() { FetchLogsFn = restore }) + var got remote.LogQuery + FetchLogsFn = func(_ context.Context, _ remote.Config, q remote.LogQuery) (remote.LogResult, error) { + got = q + return remote.LogResult{Events: []remote.LogEvent{{Message: "boot line"}}}, nil + } + + resp, err := s.LogsMatching(context.Background(), LogQuery{Source: "boot", Limit: 10, Instance: "i-1"}) + if err != nil { + t.Fatalf("LogsMatching: %v", err) + } + if got.Source != "boot" || got.Limit != 10 || got.Instance != "i-1" { + t.Errorf("query reaching the log store = %+v, want the caller's filters", got) + } + if resp.Content != "boot line\n" { + t.Errorf("content = %q, want the stub event rendered", resp.Content) + } +} diff --git a/internal/fleet/fanout.go b/internal/fleet/fanout.go index 4ed87608..16a20617 100644 --- a/internal/fleet/fanout.go +++ b/internal/fleet/fanout.go @@ -16,18 +16,23 @@ func StatusCall(ctx context.Context, n Node) NodeResult { status, err := n.Status(ctx) r := result(n.Name(), err) r.Status = status + // A node that runs on an instance describes it; the status reply alone + // carries none of that, and asking costs nothing once the call is made. + if i, ok := n.(InstanceReporter); ok { + r.Instance = i.Instance() + } return r } -// MetricsCall reads a node's engine and system metrics, and the release it -// reports alongside them where its kind reports one there. The version costs -// nothing: a node that answers it does so from the reading just taken. +// MetricsCall reads a node's engine and system metrics, and what its kind can +// say about the instance it runs on. The latter costs nothing: a node that +// answers it does so from the reading just taken. func MetricsCall(ctx context.Context, n Node) NodeResult { stats, err := n.Metrics(ctx) r := result(n.Name(), err) r.Metrics = stats - if v, ok := n.(Versioner); ok { - r.Version = v.Version() + if i, ok := n.(InstanceReporter); ok { + r.Instance = i.Instance() } return r } diff --git a/internal/fleet/node.go b/internal/fleet/node.go index 4a639e06..cae44afe 100644 --- a/internal/fleet/node.go +++ b/internal/fleet/node.go @@ -124,14 +124,33 @@ type LogQuery struct { Limit int } -// Versioner is an optional node capability: a node that reports the spinloop -// release it is running somewhere other than its status reply. A daemon node -// carries its version in that reply, so it does not implement this; a cloud -// environment's arrives with its metrics, which is a different call. A caller -// with a metrics reading in hand asserts for it rather than making a second -// call of its own. -type Versioner interface { - Version() string +// InstanceReporter is an optional node capability: a node running on an +// instance it can describe. A daemon node runs on a machine its operator +// already knows about and does not implement it; a cloud environment reports +// the instance it launched, the release on it, the address it answers at and +// how long it is retained — the facts the environment-only commands used to +// render and the shared engine reply has no room for. +// +// The values come from the replies the node has already received, so asking +// costs nothing. A field the node has not been told is empty. +type InstanceReporter interface { + Instance() Instance +} + +// Instance is what a node reports about the instance it runs on. Every field +// is optional: a reply that did not carry one leaves it empty, and a renderer +// omits what is empty rather than printing a blank. +type Instance struct { + // ID is the instance identifier the control plane assigned. + ID string + // Type is the instance type it launched as. + Type string + // Version is the spinloop release running on it. + Version string + // BaseURL is where its engine answers, as the control plane published it. + BaseURL string + // RetainUntil is its retention deadline, RFC 3339, while it has one. + RetainUntil string } // Node is one member of the fleet. Only daemonNode implements it today; the @@ -231,11 +250,10 @@ type NodeResult struct { // a call that asked for it, and only for a node that can be priced; see // Cost.Reported for the difference between "nothing" and "no figure". Cost Cost - // Version is the spinloop release this node reported somewhere other than - // its status reply. Empty for a node that carries it there instead — the - // status views read it from the status, and this is the metrics views' - // equivalent. - Version string + // Instance describes the instance this node runs on, for a node that runs + // on one it can describe. The zero value means the node reported none, + // which is every node that is a machine rather than an instance. + Instance Instance // At is when this reading was taken — set by the fan-out as the call // returns. Reads are concurrent and of uneven duration, so a reading can diff --git a/internal/fleet/remote_node.go b/internal/fleet/remote_node.go index 1da6ae32..772bfd07 100644 --- a/internal/fleet/remote_node.go +++ b/internal/fleet/remote_node.go @@ -39,13 +39,12 @@ type remoteNode struct { // A board refreshing several nodes reads and writes these from different // goroutines, so they are not left bare. mu sync.Mutex - // instanceType, uptime and version are the last metrics reading's answers - // to questions the shared stats shape has no room for. Cost and Version - // read them rather than calling the control plane again — the reply that - // carried them has already been paid for. - instanceType string - uptime int - version string + // instance is what the replies so far have said about the instance this + // environment runs on, and uptime how long it has been running. They are + // what Cost and Instance answer from, so neither costs a call of its own: + // the replies that carried them have already been paid for. + instance Instance + uptime int } // NewRemoteNode builds the live node for a named remote environment. The config @@ -68,6 +67,24 @@ func (n *remoteNode) Status(ctx context.Context) (daemon.StatusResponse, error) if err != nil { return daemon.StatusResponse{}, err } + n.mu.Lock() + n.instance.BaseURL = resp.BaseURL + n.instance.RetainUntil = resp.RetainUntil + n.mu.Unlock() + // The release is not in this reply — the stats one carries it, read from + // the daemon on the box — so a running environment is asked for it, as the + // environment-only status command asked. A stopped one has no daemon to + // answer, and a failure to reach it leaves the version empty rather than + // failing a status that otherwise succeeded. + if resp.State == "running" || resp.State == "ready" { + if stats, err := remote.Stats(ctx, n.cfg); err == nil { + n.mu.Lock() + n.instance.Version = stats.Version + n.instance.ID = stats.InstanceID + n.instance.Type = stats.InstanceType + n.mu.Unlock() + } + } return statusFromRemote(*resp), nil } @@ -80,7 +97,10 @@ func (n *remoteNode) Metrics(ctx context.Context) (metrics.Stats, error) { // Retaining them here is what lets Cost and Version answer without a // second call for a reading already in hand. n.mu.Lock() - n.instanceType, n.uptime, n.version = resp.InstanceType, resp.UptimeSeconds, resp.Version + n.uptime = resp.UptimeSeconds + n.instance.ID = resp.InstanceID + n.instance.Type = resp.InstanceType + n.instance.Version = resp.Version n.mu.Unlock() return statsFromRemote(*resp), nil } @@ -95,7 +115,7 @@ func (n *remoteNode) Metrics(ctx context.Context) (metrics.Stats, error) { // with no price to show reads the same however it came to have none. func (n *remoteNode) Cost(ctx context.Context) (Cost, error) { n.mu.Lock() - instanceType, uptime := n.instanceType, n.uptime + instanceType, uptime := n.instance.Type, n.uptime n.mu.Unlock() if instanceType == "" || uptime <= 0 { return Cost{}, nil @@ -107,13 +127,14 @@ func (n *remoteNode) Cost(ctx context.Context) (Cost, error) { return Cost{SoFar: float64(uptime) / 3600.0 * price, PerHour: price}, nil } -// Version is the spinloop release this environment's daemon reported with its -// last metrics reading. Empty before one has been taken, or where the control -// plane could not reach the daemon — the same absence, reported the same way. -func (n *remoteNode) Version() string { +// Instance is what the replies so far have said about the instance this +// environment runs on. Empty fields are ones no reply carried — a stopped +// environment has no instance id, and a control plane that could not reach the +// daemon reports no version. +func (n *remoteNode) Instance() Instance { n.mu.Lock() defer n.mu.Unlock() - return n.version + return n.instance } // LogsMatching reads this environment's log store, narrowed by what the caller @@ -137,7 +158,7 @@ func (n *remoteNode) LogsMatching(ctx context.Context, q LogQuery) (daemon.LogsR if q.Since > 0 { rq.Start = time.Now().Add(-q.Since) } - res, err := remote.FetchLogs(ctx, n.cfg, rq) + res, err := FetchLogsFn(ctx, n.cfg, rq) if err != nil { return daemon.LogsResponse{}, err } @@ -219,6 +240,12 @@ func (n *remoteNode) Keep(ctx context.Context, d time.Duration) (string, error) return deadline.UTC().Format(time.RFC3339), nil } +// FetchLogsFn is exported so a test in this package or in cmd/spinloop can +// substitute it. The read goes to the cloud's log store through the AWS SDK +// rather than an HTTP Function URL, so there is no HTTP test server that can +// stand in for it. +var FetchLogsFn = remote.FetchLogs + // remoteEngineTail caps how many engine log events a node read pulls. Remote logs // are a bounded tail pulled from the log store, not a byte cursor, so a follow of // a chatty engine must not page through an unbounded window. @@ -232,7 +259,7 @@ func (n *remoteNode) Logs(ctx context.Context, offset int64, limit int) (daemon. n.logs.Reset() } start := n.logs.Start() - res, err := remote.FetchLogs(ctx, n.cfg, remote.LogQuery{ + res, err := FetchLogsFn(ctx, n.cfg, remote.LogQuery{ Environment: n.cfg.Environment, Source: remote.LogSourceEngine, Limit: remoteEngineTail, diff --git a/openspec/changes/top-level-metrics-and-logs/design.md b/openspec/changes/top-level-metrics-and-logs/design.md index 9d683303..a1191719 100644 --- a/openspec/changes/top-level-metrics-and-logs/design.md +++ b/openspec/changes/top-level-metrics-and-logs/design.md @@ -46,9 +46,10 @@ See proposal.md — Why. The implementation-relevant current state: **D1: Three capabilities, each a node-kind ability rather than a reply field.** -`Coster`, `SourceLogger` and `Versioner` join `ProgressStarter` and `Keeper` on -`fleet.Node`. A kind that cannot answer does not implement one; a caller that -offers the flag asserts for it and leaves the node as it reads without it. +`Coster`, `SourceLogger` and `InstanceReporter` join `ProgressStarter` and +`Keeper` on `fleet.Node`. A kind that cannot answer does not implement one; a +caller that offers the flag asserts for it and leaves the node as it reads +without it. The alternative — widening `metrics.Stats` with an `InstanceType` and letting the renderer price it — was rejected twice over. It puts a cloud-only field on @@ -75,11 +76,13 @@ requirement obliges each verb's page to say which flags apply to which kinds. **D3: `statsFromRemote` stops dropping the version and instance type.** -Both are already in the reply the cloud node receives. `Versioner` reads the -version from the node's retained reply rather than making a call, so the -version costs nothing on the metrics path — unlike on `status`, where the -version lives in a different endpoint and was what made `remote status` hard to -move in the first place. +Both are already in the reply the cloud node receives. `InstanceReporter` +reads them from the node's retained replies rather than making a call, so +these facts cost nothing extra on the metrics path — unlike on `status`, where +the version lives in a different endpoint and was what made `remote status` +hard to move in the first place. The same capability also carries the +endpoint address and the retention deadline, so `status` and `metrics` read +them the same way instead of each recomputing which reply carried what. `metrics.Stats` gains no field: the node keeps what it needs to answer its own capabilities, which is D1 applied consistently. @@ -95,6 +98,23 @@ values come from a Spinloop loses them when the command they used is removed. This is the piece that made the last change stop short of `remote status`. Implementing it once here serves all three removed reads. +**D4a: The Spinloop is named by a flag, and is never implicit.** + +The `remote` subcommands took their Spinloop as a positional and *also* +consulted `./Spinloop` when none was given. Neither carries over as-is: `logs` +already spends its positional on a node name, and a file sitting in the working +directory should not silently set environment variables for a command that +reads a fleet. + +So the verbs take `-O`/`--spinloop`, the spelling the launch already uses, and +apply it only when given. An operator who relied on the implicit pickup names +the file; that is one flag, and it makes the command say what it reads. + +Alternatives: a positional (rejected — ambiguous with `logs`'s node name, and +inconsistent across the four verbs); keeping the implicit consult (rejected — +it is the kind of at-a-distance behaviour this whole sequence has been +removing, and it would newly apply to fleet reads that never had it). + **D5: Five signposts, through the existing mechanism.** `fleet metrics`, `fleet logs`, `remote status`, `remote metrics` and diff --git a/openspec/changes/top-level-metrics-and-logs/tasks.md b/openspec/changes/top-level-metrics-and-logs/tasks.md index 526e40e6..7345b0d3 100644 --- a/openspec/changes/top-level-metrics-and-logs/tasks.md +++ b/openspec/changes/top-level-metrics-and-logs/tasks.md @@ -8,10 +8,11 @@ - [x] 1.2 Add `SourceLogger`: reading a node's log by source, time window and instance. Verify the cloud node answers all three and the daemon does not implement it. -- [x] 1.3 Add `Versioner`: the spinloop release a node reports. Verify it is - answered from the reply the node already holds, with no extra call - (design D3), and that `statsFromRemote` stops dropping the version and - instance type. +- [x] 1.3 Add `InstanceReporter`: the instance a node runs on — its id, type, + the spinloop release on it, its endpoint address and retention deadline. + Verify it is answered from the replies the node already holds, with no + extra call (design D3), and that `statsFromRemote` stops dropping the + version and instance type. - [x] 1.4 Verify no caller branches on node kind: a test or a grep confirming the capabilities are reached only by type assertion. @@ -31,7 +32,7 @@ - [x] 2.4 Wire `--source`/`--since`/`--instance` to `SourceLogger` on the same terms. Verify a mixed target reads the environment's boot log and the daemon's ordinary output. -- [ ] 2.5 Read a Spinloop given to either verb for its `ENV` instructions and +- [x] 2.5 Read a Spinloop given to either verb for its `ENV` instructions and adjacent `.env` only, never to select a target (design D4). Verify a Spinloop whose `ENV` supplies `SPINLOOP_REMOTE_*` configures the command. - [x] 2.6 Register both at the root with the completion the fleet spellings @@ -41,28 +42,28 @@ - [x] 3.1 Delete `fleetMetricsCmd` and `fleetLogsCmd` and unregister them, keeping their renderers. Verify `spinloop fleet --help` lists neither. -- [ ] 3.2 Delete `remoteStatusCmd`, `remoteMetricsCmd`, `remoteLogsCmd` and +- [x] 3.2 Delete `remoteStatusCmd`, `remoteMetricsCmd`, `remoteLogsCmd` and their bodies, keeping the formatters the top-level verbs use. Verify `spinloop remote --help` lists none of the three. -- [ ] 3.3 Signpost all five moved spellings through `movedSubcommands`, giving +- [x] 3.3 Signpost all five moved spellings through `movedSubcommands`, giving the `remote` group the same `Args: groupArgs` the fleet group has (design D5). Verify each names its replacement and an unknown subcommand in either group still gets cobra's own error. ## 4. Consumers, docs and verification -- [ ] 4.1 Update every invocation of the five moved spellings across `docs/`, +- [x] 4.1 Update every invocation of the five moved spellings across `docs/`, `README.md` and `examples/` — including the two CI-run `run-tests.sh` scripts, whose `fleet()` shell helper means a bare rename breaks them. Verify `bash -n` on each script and that none invokes a removed spelling. -- [ ] 4.2 Add `docs/commands/metrics.md` and `logs.md`, each saying which +- [x] 4.2 Add `docs/commands/metrics.md` and `logs.md`, each saying which flags apply to which node kinds (required by the capability requirement), and point `fleet.md` and `remote.md` at them. Verify `docs/README.md`'s command table lists both. -- [ ] 4.3 Verify nothing an operator could get from the removed commands is +- [x] 4.3 Verify nothing an operator could get from the removed commands is unreachable: the cost, the version, the log sources, the `ENV` path, and the endpoint address via `remote env`. -- [ ] 4.4 Run `gofmt -l .` (expect no output), `go vet ./...` and +- [x] 4.4 Run `gofmt -l .` (expect no output), `go vet ./...` and `go test ./... -cover`, confirming total coverage is unchanged and still >= 80%. From 74c7c885bea45ea99b59b36660dad5aa4ccdc86c Mon Sep 17 00:00:00 2001 From: spinloop-agent Date: Sat, 19 Sep 2026 22:01:53 +0100 Subject: [PATCH 4/4] chore: archive top-level-metrics-and-logs, sync its specs into main 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. --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/fleet-client/spec.md | 0 .../specs/remote-endpoint/spec.md | 0 .../specs/remote-logs/spec.md | 0 .../specs/remote-stats/spec.md | 0 .../specs/remote-version-reporting/spec.md | 0 .../tasks.md | 0 openspec/specs/fleet-client/spec.md | 98 ++++++++++++++++--- openspec/specs/remote-endpoint/spec.md | 41 ++++---- openspec/specs/remote-logs/spec.md | 47 ++++----- openspec/specs/remote-stats/spec.md | 77 ++++++++------- .../specs/remote-version-reporting/spec.md | 18 ++-- 14 files changed, 179 insertions(+), 102 deletions(-) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/.openspec.yaml (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/design.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/proposal.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/specs/fleet-client/spec.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/specs/remote-endpoint/spec.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/specs/remote-logs/spec.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/specs/remote-stats/spec.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/specs/remote-version-reporting/spec.md (100%) rename openspec/changes/{top-level-metrics-and-logs => archive/2026-09-19-top-level-metrics-and-logs}/tasks.md (100%) diff --git a/openspec/changes/top-level-metrics-and-logs/.openspec.yaml b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/.openspec.yaml similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/.openspec.yaml rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/.openspec.yaml diff --git a/openspec/changes/top-level-metrics-and-logs/design.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/design.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/design.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/design.md diff --git a/openspec/changes/top-level-metrics-and-logs/proposal.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/proposal.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/proposal.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/proposal.md diff --git a/openspec/changes/top-level-metrics-and-logs/specs/fleet-client/spec.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/fleet-client/spec.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/specs/fleet-client/spec.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/fleet-client/spec.md diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-endpoint/spec.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-endpoint/spec.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/specs/remote-endpoint/spec.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-endpoint/spec.md diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-logs/spec.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-logs/spec.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/specs/remote-logs/spec.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-logs/spec.md diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-stats/spec.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-stats/spec.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/specs/remote-stats/spec.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-stats/spec.md diff --git a/openspec/changes/top-level-metrics-and-logs/specs/remote-version-reporting/spec.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-version-reporting/spec.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/specs/remote-version-reporting/spec.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/specs/remote-version-reporting/spec.md diff --git a/openspec/changes/top-level-metrics-and-logs/tasks.md b/openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/tasks.md similarity index 100% rename from openspec/changes/top-level-metrics-and-logs/tasks.md rename to openspec/changes/archive/2026-09-19-top-level-metrics-and-logs/tasks.md diff --git a/openspec/specs/fleet-client/spec.md b/openspec/specs/fleet-client/spec.md index 0c03ba76..cb649231 100644 --- a/openspec/specs/fleet-client/spec.md +++ b/openspec/specs/fleet-client/spec.md @@ -97,9 +97,9 @@ render, and the command SHALL succeed. ### Requirement: Fleet metrics -`spinloop fleet metrics` SHALL query every node's metrics endpoint and render -each node's engine and system metrics using the same bar, gauge, table, and -json formats `spinloop remote metrics` provides, selected by `--format`. +`spinloop metrics` SHALL query every node in the resolved target and render +each node's engine and system metrics using the same bar, gauge, table and +json formats, selected by `--format`. Unreachable nodes SHALL be reported as in status rather than omitted. The command SHALL support a `--watch`/`-w` mode that refreshes on an interval, clearing and redrawing the screen in place with no scrollback accumulation, @@ -107,7 +107,7 @@ and exiting cleanly on interrupt. #### Scenario: Gauge format per node -- **WHEN** `spinloop fleet metrics` runs without `--format` +- **WHEN** `spinloop metrics` runs without `--format` - **THEN** each reachable node's resource series render in gauge format under its name @@ -128,6 +128,22 @@ and exiting cleanly on interrupt. - **THEN** each refresh clears the screen and redraws the fleet, and Ctrl+C exits cleanly +The target SHALL resolve as it does for every other command that acts on a +fleet — a named environment, a named fleet file, or the working directory's +fleet file — so one environment and a fleet holding it are read by the one +command. + +#### Scenario: A named environment is read by the same command + +- **WHEN** `spinloop metrics --env prod` runs +- **THEN** that environment's metrics are rendered with no fleet file + required, in the same format a fleet file naming it would produce + +#### Scenario: The fleet-scoped spelling names its replacement + +- **WHEN** the operator runs `spinloop fleet metrics` +- **THEN** it fails naming `spinloop metrics` as the command that replaced it + ### Requirement: Driving one node `spinloop fleet start ` SHALL call each named node's daemon start @@ -449,21 +465,21 @@ aborting the rest of the fleet. ### Requirement: Fleet logs -`spinloop fleet logs` SHALL read the engine output of the fleet's nodes through -each node's daemon, so "what did that engine say?" is answerable from the same +`spinloop logs` SHALL read the engine output of the target's nodes, so "what did that engine say?" is answerable from the same place as "what is it doing?" — without shell access to any machine. With no node named it SHALL read every node in the fleet; naming a node SHALL restrict it to that one. Nodes SHALL be read concurrently, so the command's latency is that of the slowest reachable node rather than their sum. -The fleet file SHALL be named by the long form `--fleet` only: unlike the other -`spinloop fleet` commands, `logs` SHALL NOT accept `-f` for it, because `-f` is -that command's `--follow` short form and a flag cannot carry two meanings on one -command line. +The target SHALL resolve as it does for every other command that acts on a +fleet. The fleet file SHALL be named by the long form `--fleet` only: unlike +the other commands that take one, `logs` SHALL NOT accept `-f` for it, because +`-f` is that command's `--follow` short form and a flag cannot carry two +meanings on one command line. #### Scenario: Reading the whole fleet -- **WHEN** the operator runs `spinloop fleet logs` with no node named +- **WHEN** the operator runs `spinloop logs` with no node named - **THEN** every node's engine output is read and printed #### Scenario: Reading one node @@ -474,7 +490,7 @@ command line. #### Scenario: A crashed node's output is readable -- **WHEN** a node's engine has crashed, as `spinloop fleet status` reports +- **WHEN** a node's engine has crashed, as `spinloop status` reports - **THEN** its output up to the crash is printed, explaining what status can only report @@ -484,6 +500,17 @@ command line. - **THEN** the flag is accepted as follow mode plus the fleet file, with `-f` not treated as a fleet-file flag +#### Scenario: A named environment is read by the same command + +- **WHEN** `spinloop logs --env prod` runs +- **THEN** that environment's engine output is read with no fleet file + required + +#### Scenario: The fleet-scoped spelling names its replacement + +- **WHEN** the operator runs `spinloop fleet logs` +- **THEN** it fails naming `spinloop logs` as the command that replaced it + ### Requirement: Fleet log lines are attributed to their node When output from more than one node is printed, every line SHALL identify the @@ -1586,3 +1613,50 @@ the sweep has already moved past as though the node were still held. - **WHEN** the operator opens the detail screen of a retained node - **THEN** the detail screen's deadline line is the same line, worded the same, the tile draws for the same read + +### Requirement: A node answers only what its kind can + +A node kind SHALL answer the operations its kind supports and no more. Where a +read verb offers a flag whose answer only some node kinds carry — an instance's +price, a log store's source, time window or instance id — the flag SHALL apply +to the nodes that can answer it and SHALL leave the rest as they read without +it. + +A target holding no node that can answer SHALL NOT be an error. Rendering a +column blank for a node that has no such fact is what these views already do +for every fact only one kind reports, and a flag is not a different case: a +fleet of daemons asked for a cost is a fleet with no cost to show, not a +malformed command. + +The capability SHALL be a property of the node kind, asserted by the caller, +not a field on the shared reply. A kind that cannot answer SHALL simply not +implement it, so adding a kind that can requires no change to the callers and +adding one that cannot requires no exception. + +Which flags apply to which kinds SHALL be documented on each verb's page, since +a blank column is not self-explaining. + +#### Scenario: A priced node and an unpriced one in one table + +- **WHEN** `spinloop metrics --cost` runs against a target holding both a cloud + environment and a daemon node +- **THEN** the environment's row carries its cost and the daemon's does not, + and the command succeeds + +#### Scenario: No node can answer, and that is not an error + +- **WHEN** `spinloop metrics --cost` runs against a fleet of daemon nodes only +- **THEN** no cost is shown for any node and the command succeeds + +#### Scenario: A log query narrows the nodes that support it + +- **WHEN** `spinloop logs --source boot` runs against a target holding both a + cloud environment and a daemon node +- **THEN** the environment's boot log is read, and the daemon's output is read + as it would be without the flag + +#### Scenario: A kind that cannot answer implements nothing + +- **WHEN** a node kind has no answer for a capability +- **THEN** it does not implement that capability, and no caller carries a + branch naming that kind diff --git a/openspec/specs/remote-endpoint/spec.md b/openspec/specs/remote-endpoint/spec.md index fffd5888..b7f2e877 100644 --- a/openspec/specs/remote-endpoint/spec.md +++ b/openspec/specs/remote-endpoint/spec.md @@ -8,9 +8,9 @@ what to serve from a Spinloop: the `spinloop remote` command group. ### Requirement: Remote command group The system SHALL provide a `remote` command group with the subcommands -`bootstrap`, `bake`, `auth`, `start`, `stop`, `restart`, `status`, `deploy`, -`ls`, `metrics`, and `keep`. `start`, `stop`, `restart`, `status`, `metrics` and -`deploy` each take an optional Spinloop path: +`bootstrap`, `bake`, `auth`, `start`, `stop`, `restart`, `deploy`, +`ls`, and `keep`. `start`, `stop`, `restart` and `deploy` each take an +optional Spinloop path: `start` SHALL boot the endpoint and block until it is serving, then perform a quick TCP probe of the inference endpoint — if the probe fails, a warning is printed to stderr explaining the network mismatch (see the Remote Start Probe @@ -26,14 +26,9 @@ and reporting progress as `start` does (see the Reporting a start in progress specification); `restart` SHALL accept a `--force` flag with a `-F` short form that, when set, performs the stop without first asking the engine to shut down (see the Endpoint Lifecycle specification for forced stops); -`status` SHALL report instance state and endpoint health without side effects -and SHALL NOT perform any TCP probe, and SHALL include the `Retain-Until` -deadline when the instance has an active retention tag; `keep` SHALL set the `Retain-Until` tag on the environment's instance for the given duration, without starting or stopping the instance (see the Remote Keep -specification); `metrics` SHALL report instance state, token usage, resource -consumption, and GPU information for a running instance; `deploy` SHALL set -what the endpoint serves. `ls` SHALL list the registered remote environments +specification); `deploy` SHALL set what the endpoint serves. `ls` SHALL list the registered remote environments (see the Remote Environments specification). `bootstrap` SHALL stand up the account-level AWS control plane (once per account) by obtaining and driving the CDK project, and takes its own flags rather than a Spinloop path (see the @@ -105,11 +100,6 @@ SHALL fail naming the accepted ones. - **WHEN** the user runs `spinloop remote keep 2h` - **THEN** the instance retention tag is set and the deadline is reported -#### Scenario: Metrics reports instance figures - -- **WHEN** the user runs `spinloop remote metrics` with a running instance -- **THEN** token counts, resource usage, and GPU information are displayed - #### Scenario: Bootstrap is a recognised subcommand - **WHEN** the user runs `spinloop remote bootstrap` @@ -132,7 +122,14 @@ SHALL fail naming the accepted ones. - **WHEN** the user runs `spinloop remote frobnicate` - **THEN** the command fails listing the accepted subcommands, which include - `bootstrap`, `bake`, `metrics`, and `keep` + `bootstrap`, `bake`, `auth`, and `keep` + +#### Scenario: The read verbs are not in the group + +- **WHEN** the operator runs `spinloop remote status`, `spinloop remote + metrics` or `spinloop remote logs` +- **THEN** each fails naming the top-level verb that replaced it, and the + group's help lists none of them ### Requirement: Reporting a start in progress @@ -425,9 +422,9 @@ the instance. - **THEN** the key is sent to the control plane to be stored for the environment, and the report says a key was applied without printing the value -### Requirement: Status reports when the endpoint last did work +### Requirement: An environment reports when it last did work -`spinloop remote status` SHALL report how long it has been since the endpoint's +`spinloop status --env ` SHALL report how long it has been since the endpoint's engine last did any work, alongside the instance state and health it reports already. The figure SHALL come from the activity the on-instance daemon tracks, not from a measurement the control plane makes itself — one answer, @@ -444,20 +441,20 @@ read, and SHALL still perform no TCP probe. #### Scenario: A running endpoint reports its last activity -- **WHEN** the user runs `spinloop remote status` against a running endpoint +- **WHEN** the user runs `spinloop status --env ` against a running endpoint whose engine has served work - **THEN** the output reports how long ago that work happened, labelled "last active", beside the state and health lines #### Scenario: Status stays a read -- **WHEN** the user runs `spinloop remote status` +- **WHEN** the user runs `spinloop status --env ` - **THEN** nothing is started, stopped or probed in order to obtain the last-active figure -### Requirement: Status degrades when activity cannot be read +### Requirement: An environment's status degrades when activity cannot be read -`spinloop remote status` SHALL omit the last-active figure rather than fail, +`spinloop status --env ` SHALL omit the last-active figure rather than fail, report zero, or imply inactivity, whenever the figure cannot be obtained. That covers an endpoint whose engine has not yet done any work, a daemon that cannot be reached or answers unrecognisably, and an instance that is not @@ -470,7 +467,7 @@ SHALL still succeed. #### Scenario: A stopped instance reports no activity figure -- **WHEN** the user runs `spinloop remote status` and the instance is stopped or +- **WHEN** the user runs `spinloop status --env ` and the instance is stopped or undeployed - **THEN** the output reports the state as it does today and shows no last-active figure diff --git a/openspec/specs/remote-logs/spec.md b/openspec/specs/remote-logs/spec.md index c207873d..eaef5d21 100644 --- a/openspec/specs/remote-logs/spec.md +++ b/openspec/specs/remote-logs/spec.md @@ -11,28 +11,32 @@ which is when they are wanted most. ## Requirements ### Requirement: An environment's shipped logs are readable from the CLI -`spinloop remote logs` SHALL print the logs an environment's instances have +`spinloop logs --env ` SHALL print the logs an environment's instances have shipped, without the operator needing to know the log group or stream naming, open the AWS console, or connect to an instance. It SHALL select which -environment to read using the same rules as the other remote subcommands — the -`--env ` flag naming a registered environment, else the `default` -environment — so `spinloop remote logs` and `spinloop remote status` given the -same `--env` always speak about the same environment. +environment to read using the same `--env ` flag every other read verb +uses, so `spinloop logs --env ` and `spinloop status --env ` given +the same `--env` always speak about the same environment. -#### Scenario: Reading the current environment's logs +#### Scenario: Reading an environment's logs -- **WHEN** the operator runs `spinloop remote logs` where `spinloop remote status` - would report on an environment +- **WHEN** the operator runs `spinloop logs --env ` where `spinloop status --env ` + would report on that environment - **THEN** the log events that environment's instances shipped are printed - **AND** the operator is not required to name a log group, stream, or instance #### Scenario: Reading a named environment's logs -- **WHEN** the operator runs `spinloop remote logs --env dev-2` -- **THEN** `dev-2`'s logs are printed rather than the default - environment's +- **WHEN** the operator runs `spinloop logs --env dev-2` +- **THEN** `dev-2`'s logs are printed rather than any other environment's -### Requirement: Logs are readable after the instance is gone +#### Scenario: The remote spelling names its replacement + +- **WHEN** the operator runs `spinloop remote logs` +- **THEN** it fails naming `spinloop logs --env ` as the command that + replaced it + +### Requirement: An environment's logs are readable after the instance is gone Reading logs SHALL NOT depend on an instance being running, nor on the environment's control endpoints answering. Logs SHALL be read from the durable @@ -42,14 +46,14 @@ instance that has since terminated, is still available. #### Scenario: A terminated instance's logs are still readable - **WHEN** an instance has produced logs and has since terminated -- **THEN** `spinloop remote logs` still prints that instance's shipped events +- **THEN** `spinloop logs --env ` still prints that instance's shipped events #### Scenario: A stopped environment can be diagnosed - **WHEN** an environment is stopped, so its status reports no running instance -- **THEN** `spinloop remote logs` still prints the logs from its previous runs +- **THEN** `spinloop logs --env ` still prints the logs from its previous runs -### Requirement: Both engine and boot logs are reachable +### Requirement: An environment's both engine and boot logs are reachable The command SHALL be able to read either log source an instance ships — the inference engine's output and the boot (user-data) output — and both together. @@ -60,7 +64,7 @@ the engine started is reachable even though the engine log is empty. #### Scenario: Engine output by default -- **WHEN** the operator runs `spinloop remote logs` with no source selected +- **WHEN** the operator runs `spinloop logs --env ` with no source selected - **THEN** the environment's engine log events are printed #### Scenario: Boot output on request @@ -83,7 +87,7 @@ the engine started is reachable even though the engine log is empty. - **THEN** the engine's logs are found regardless of which supported engine produced them -### Requirement: The volume fetched is bounded and controllable +### Requirement: An environment's the volume fetched is bounded and controllable The command SHALL bound what it fetches by default rather than pulling an environment's entire retained history, and SHALL let the operator widen or @@ -94,7 +98,7 @@ truncated view as complete. #### Scenario: A default window applies -- **WHEN** the operator runs `spinloop remote logs` with no window stated +- **WHEN** the operator runs `spinloop logs --env ` with no window stated - **THEN** only events from a bounded recent window are fetched #### Scenario: The window is widened @@ -115,7 +119,7 @@ truncated view as complete. - **THEN** only that instance's events are printed, and events from the environment's other instances are excluded -### Requirement: Output is ordered, timestamped and attributable +### Requirement: An environment's output is ordered, timestamped and attributable Events SHALL be printed oldest first, each carrying its timestamp, so the output reads like a log rather than an unordered dump. When the printed events @@ -147,7 +151,7 @@ fields for scripting. - **THEN** the events are emitted as structured records carrying at least the timestamp, source, instance and message -### Requirement: New output can be followed +### Requirement: An environment's new output can be followed The command SHALL be able to keep running and print events as they arrive, rather than exiting after one fetch, so an operator can watch a start or a @@ -171,7 +175,7 @@ SHALL NOT be repeated on a later poll — and SHALL stop cleanly on interrupt. - **WHEN** the operator interrupts a follow - **THEN** the command exits without reporting an error -### Requirement: Missing logs and missing access are explained +### Requirement: An environment's missing logs and missing access are explained When no output can be produced, the command SHALL distinguish the causes an operator can act on and say what to do, rather than printing nothing or a raw @@ -205,4 +209,3 @@ environment that simply has not logged anything in the window asked for. environment in the window asked for - **THEN** the command reports that there are no events for that environment in that window, and exits without an error - diff --git a/openspec/specs/remote-stats/spec.md b/openspec/specs/remote-stats/spec.md index 9ce1c86c..61e7576c 100644 --- a/openspec/specs/remote-stats/spec.md +++ b/openspec/specs/remote-stats/spec.md @@ -2,52 +2,55 @@ ## Purpose -Define the `spinloop remote metrics` command: reading token usage, resource consumption, and GPU information from a running remote inference instance. +Define the `spinloop metrics` command's reading of a remote environment's +instance: token usage, resource consumption, and GPU information from a +running remote inference instance. ## Requirements -### Requirement: Metrics subcommand +### Requirement: An environment's metrics are reported -The system SHALL provide a `metrics` subcommand (`spinloop remote metrics`) that reports the current state of a remote inference instance. It SHALL select which environment it reports on using the same rule as the other `remote` subcommands: the `--env ` flag naming a registered environment, falling back to the `default` environment when the flag is absent. A Spinloop given as an argument is read only for its `ENV` instructions and adjacent `.env`, never to select the environment. When the instance is running, the report SHALL include the spinloop version from the daemon, carried by the stats Lambda reply. +The system SHALL provide a `metrics` command (`spinloop metrics --env `) that reports the current state of a remote inference instance. It SHALL select which environment it reports on using the same `--env ` flag every other read verb uses. A Spinloop given as an argument is read only for its `ENV` instructions and adjacent `.env`, never to select the environment. When the instance is running, the report SHALL include the spinloop version from the daemon, carried by the stats Lambda reply. #### Scenario: Stats with a running instance -- **WHEN** the user runs `spinloop remote metrics` with a running instance +- **WHEN** the user runs `spinloop metrics --env ` with a running instance - **THEN** the command reports the instance state, runner, model, spinloop version, GPU info, CPU/RAM usage, token counts, and request counts #### Scenario: Stats with a stopped instance -- **WHEN** the user runs `spinloop remote metrics` and the instance is stopped +- **WHEN** the user runs `spinloop metrics --env ` and the instance is stopped - **THEN** the command reports `state: stopped` and no metrics #### Scenario: Stats names the environment with the flag -- **WHEN** the user runs `spinloop remote metrics --env dev-2` +- **WHEN** the user runs `spinloop metrics --env dev-2` - **THEN** the command reports on the `dev-2` environment's instance -#### Scenario: Stats falls back to the default environment - -- **WHEN** the user runs `spinloop remote metrics` with no `--env` flag -- **THEN** the command reports on the `default` environment's instance - #### Scenario: Version is shown in stats output -- **WHEN** the user runs `spinloop remote metrics` with a running instance +- **WHEN** the user runs `spinloop metrics --env ` with a running instance - **THEN** the output includes the spinloop version -### Requirement: Optional cost estimation +#### Scenario: The remote spelling names its replacement + +- **WHEN** the operator runs `spinloop remote metrics` +- **THEN** it fails naming `spinloop metrics --env ` as the command that + replaced it + +### Requirement: An environment's cost is reported on request When the user passes `--cost`, the stats report SHALL include an estimated on-demand cost for the current running session. The cost SHALL be computed from the instance type's on-demand price (fetched from the AWS Price List API for the deployed region) multiplied by the elapsed time since launch. Without `--cost`, no price lookup is performed and no cost is shown. #### Scenario: Cost is shown with flag -- **WHEN** the user runs `spinloop remote metrics --cost` with a running instance +- **WHEN** the user runs `spinloop metrics --env --cost` with a running instance - **THEN** the report includes the estimated cost for the current session #### Scenario: Cost is not shown by default -- **WHEN** the user runs `spinloop remote metrics` without `--cost` +- **WHEN** the user runs `spinloop metrics --env ` without `--cost` - **THEN** the report does not include a cost line -### Requirement: Tabular display +### Requirement: The metrics report's tabular display The stats output SHALL support four formats via the `--format` flag: `gauge` (default), `bar`, `table`, and `json`. The `gauge` format SHALL produce a compact display with horizontal progress gauges for the current reading, colour-coded by utilization level. The `bar` format SHALL produce a compact display drawing each resource series as a sparkline of the daemon's retained history, with the latest point colour-coded by utilization level. The `table` format SHALL produce a tab-separated key-value table, one line per metric, with the key column left-aligned and values right of it. The `json` format SHALL output the response as a JSON object to standard output. Progress and error messages SHALL go to standard error regardless of format. @@ -58,69 +61,69 @@ The stats output SHALL support four formats via the `--format` flag: `gauge` (de #### Scenario: Default format is gauge -- **WHEN** the user runs `spinloop remote metrics` without `--format` +- **WHEN** the user runs `spinloop metrics --env ` without `--format` - **THEN** the output is in gauge format #### Scenario: Table format is explicit -- **WHEN** the user runs `spinloop remote metrics --format=table` +- **WHEN** the user runs `spinloop metrics --env --format=table` - **THEN** the output is in table format #### Scenario: Bar format is explicit -- **WHEN** the user runs `spinloop remote metrics --format=bar` +- **WHEN** the user runs `spinloop metrics --env --format=bar` - **THEN** the output is in bar format, drawing each resource series as a sparkline of the daemon's retained history #### Scenario: Gauge format is explicit -- **WHEN** the user runs `spinloop remote metrics --format=gauge` +- **WHEN** the user runs `spinloop metrics --env --format=gauge` - **THEN** the output is in gauge format with progress gauges for the current reading #### Scenario: JSON format -- **WHEN** the user runs `spinloop remote metrics --format=json` +- **WHEN** the user runs `spinloop metrics --env --format=json` - **THEN** the output is valid JSON containing the instance state, runner, model, GPU info, CPU/RAM usage, and token counts #### Scenario: JSON format with cost -- **WHEN** the user runs `spinloop remote metrics --format=json --cost` +- **WHEN** the user runs `spinloop metrics --env --format=json --cost` - **THEN** the JSON output includes a cost estimate field #### Scenario: Invalid format errors -- **WHEN** the user runs `spinloop remote metrics --format=csv` +- **WHEN** the user runs `spinloop metrics --env --format=csv` - **THEN** the command exits with an error and usage message -### Requirement: Watch mode +### Requirement: The metrics report refreshes on request The system SHALL support a `--watch`/`-w` flag that repeatedly queries metrics every 60 seconds. When enabled, the command SHALL clear the screen and redraw the output in place for each refresh, producing no scrollback accumulation. Each refresh SHALL pre-render the metrics output into a buffer before clearing the screen, so the redisplay is instantaneous after the network round-trip. The command SHALL continue until the user sends `SIGINT` (Ctrl+C) or `SIGTERM`, at which point it SHALL exit cleanly. #### Scenario: Watch mode repeats output -- **WHEN** the user runs `spinloop remote metrics --watch` +- **WHEN** the user runs `spinloop metrics --env --watch` - **THEN** the command prints metrics, waits 60 seconds, clears the screen, and prints updated metrics #### Scenario: Watch redraws in place -- **WHEN** the user runs `spinloop remote metrics -w` +- **WHEN** the user runs `spinloop metrics --env -w` - **THEN** each refresh after the first clears the screen before displaying new output, with no separator lines #### Scenario: Watch with JSON format -- **WHEN** the user runs `spinloop remote metrics --watch --format=json` +- **WHEN** the user runs `spinloop metrics --env --watch --format=json` - **THEN** each refresh clears the screen and outputs a JSON object #### Scenario: Watch with cost -- **WHEN** the user runs `spinloop remote metrics --watch --cost` +- **WHEN** the user runs `spinloop metrics --env --watch --cost` - **THEN** each refresh includes the cost estimate #### Scenario: Watch stops on interrupt -- **WHEN** the user runs `spinloop remote metrics -w` and presses Ctrl+C +- **WHEN** the user runs `spinloop metrics --env -w` and presses Ctrl+C - **THEN** the command exits cleanly without error -### Requirement: Reporting when the endpoint last did work +### Requirement: An environment's metrics report when it last did work The metrics report SHALL include how long it has been since the endpoint's engine last did any work, taken from the activity the on-instance daemon @@ -140,20 +143,20 @@ show one implying the endpoint has been quiet since it started. #### Scenario: A working endpoint reports its last activity -- **WHEN** the user runs `spinloop remote metrics` against a running endpoint +- **WHEN** the user runs `spinloop metrics --env ` against a running endpoint whose engine has served work - **THEN** the report shows how long ago that work happened, labelled "last active" #### Scenario: Every format carries the figure -- **WHEN** the user runs `spinloop remote metrics` with `--format=bar`, +- **WHEN** the user runs `spinloop metrics --env ` with `--format=bar`, `--format=table`, or `--format=json` - **THEN** each output carries the last-active figure in its own idiom #### Scenario: An endpoint that has done nothing omits the figure -- **WHEN** the user runs `spinloop remote metrics` against an endpoint whose +- **WHEN** the user runs `spinloop metrics --env ` against an endpoint whose engine has not yet done any work - **THEN** the report shows no last-active figure @@ -164,23 +167,23 @@ show one implying the endpoint has been quiet since it started. - **THEN** the report shows no last-active figure, and the rest of the report renders as it does today -### Requirement: History in the report +### Requirement: History in the metrics report When the on-instance daemon's metrics reply carries a history of system readings, the report SHALL carry it through to the command's output: the `json` format SHALL include the readings, and the `bar` format SHALL draw them. Where the daemon's reply carries no history, the report SHALL omit the field and the `bar` format SHALL fall back per the bar format specification. The control plane's relay of the daemon's reply SHALL NOT alter the readings it carries. #### Scenario: JSON carries the daemon's history -- **WHEN** the instance's daemon reports a retained history and the user runs `spinloop remote metrics --format=json` +- **WHEN** the instance's daemon reports a retained history and the user runs `spinloop metrics --env --format=json` - **THEN** the JSON output includes the history's readings #### Scenario: Bar draws the relayed history -- **WHEN** the instance's daemon reports a retained history and the user runs `spinloop remote metrics --format=bar` +- **WHEN** the instance's daemon reports a retained history and the user runs `spinloop metrics --env --format=bar` - **THEN** each resource series is drawn as a sparkline from the readings the control plane relayed #### Scenario: A daemon without history degrades -- **WHEN** the instance runs a daemon whose reply carries no history and the user runs `spinloop remote metrics --format=bar` +- **WHEN** the instance runs a daemon whose reply carries no history and the user runs `spinloop metrics --env --format=bar` - **THEN** the report omits the history field and bar format draws the current reading in the gauge's filled style ### Requirement: The stats reply carries the retention deadline diff --git a/openspec/specs/remote-version-reporting/spec.md b/openspec/specs/remote-version-reporting/spec.md index 46a416aa..8710bcfc 100644 --- a/openspec/specs/remote-version-reporting/spec.md +++ b/openspec/specs/remote-version-reporting/spec.md @@ -3,37 +3,37 @@ ## Purpose Reports the spinloop version running on a remote instance or fleet node so the operator can answer "is this node on the release I expect?" without SSH access. ## Requirements -### Requirement: Remote status shows version +### Requirement: An environment's status shows version -`spinloop remote status` SHALL display the spinloop version running on the remote instance alongside its existing state, health, and base URL fields. +`spinloop status --env ` SHALL display the spinloop version running on the remote instance alongside its existing state, health, and base URL fields. #### Scenario: Version is shown when the instance is running -- **WHEN** the user runs `spinloop remote status` against a running instance +- **WHEN** the user runs `spinloop status --env ` against a running instance - **THEN** the output includes a `version` line with the spinloop version string (e.g. `version: 1.16.0`) #### Scenario: Version is unavailable when the instance is stopped -- **WHEN** the user runs `spinloop remote status` against a stopped instance +- **WHEN** the user runs `spinloop status --env ` against a stopped instance - **THEN** the output omits the version line, since the daemon is not reachable -### Requirement: Remote metrics shows version +### Requirement: An environment's metrics show version -`spinloop remote metrics` SHALL display the spinloop version in its output, as the stats Lambda already reads the daemon and can carry the version alongside its existing fields. +`spinloop metrics --env ` SHALL display the spinloop version in its output, as the stats Lambda already reads the daemon and can carry the version alongside its existing fields. #### Scenario: Version is shown in table format -- **WHEN** the user runs `spinloop remote metrics --format=table` against a running instance +- **WHEN** the user runs `spinloop metrics --env --format=table` against a running instance - **THEN** the table output includes a `version` line #### Scenario: Version is shown in JSON format -- **WHEN** the user runs `spinloop remote metrics --format=json` against a running instance +- **WHEN** the user runs `spinloop metrics --env --format=json` against a running instance - **THEN** the JSON output includes a `version` field #### Scenario: Version is omitted from bar header when unavailable -- **WHEN** the user runs `spinloop remote metrics --format=bar` and the version is not available +- **WHEN** the user runs `spinloop metrics --env --format=bar` and the version is not available - **THEN** the bar header omits the version without error ### Requirement: Daemon status endpoint reports version