feat(workspace): log workspace and agent state changes - #1075
Conversation
a1a27aa to
163a89b
Compare
163a89b to
082b4bb
Compare
082b4bb to
163a89b
Compare
163a89b to
d29fcbd
Compare
7f7439a to
370185b
Compare
370185b to
b69ba41
Compare
b69ba41 to
4e8b8c8
Compare
3c2be68 to
1fc0b0b
Compare
|
Should we include an entry in the changelog for the past 2 PRs + this? We only mention user-visible changes and these are worthy of it since it means sharing the logs or browsing them would give them an easy way to correlate logs to sessions. (also more logs so they can easily debug more) |
1fc0b0b to
0991842
Compare
0991842 to
b1a22a8
Compare
…gers Address review feedback on the combined WorkspaceStateLogger: it logged once per agent for a single workspace-status change and duplicated the transition-detection already in the telemetry observers. - Add a shared TransitionTracker and use it in WorkspaceStateTelemetry and WorkspaceAgentTelemetry (no behavior change). - Split the logger into WorkspaceStateLogger (one log per workspace status change) and WorkspaceAgentLogger (per-agent status/lifecycle), mirroring the two telemetry observers. Both are wired in WorkspaceMonitor.
Mirror WorkspaceStateTelemetry: key the workspace state logger on status, transition, and reason, and include transition and reason in the log payload so a state-change line matches the telemetry event.
…lemetry Reshape workspace/agent state handling into three layers, one job each: - detect: WorkspaceStateObserver / WorkspaceAgentObserver report a transition object; stateful but effect-free (no logger/telemetry imports) - record: recordWorkspaceState / recordAgentState emit telemetry - log: inline in WorkspaceMonitor.update, matching the repo's other log sites Deletes the parallel WorkspaceStateLogger / WorkspaceAgentLogger classes and the WorkspaceStateTelemetry / WorkspaceAgentTelemetry classes. Agent state is now observed once in WorkspaceMonitor for every agent over the whole session, so WorkspaceStateMachine no longer tracks agent telemetry. Logs use flat scalar payloads and say "state observed" on first sight, "state changed" after, and log when an agent disappears.
The event now covers every agent for the whole monitored session (deduped), not just the connected agent during connection setup, and transitions are sampled from the workspace event stream so intermediate hops may coalesce.
Per CONVENTIONS.md, src/instrumentation is telemetry-only. The pure detection code has no telemetry or logger imports, so move TransitionTracker, WorkspaceStateObserver, WorkspaceAgentObserver, and the transition types to src/workspace/observers.ts. The record* functions stay in instrumentation and import the transition types from there. Observer/tracker tests move to test/unit/workspace/observers.test.ts.
0c10123 to
8400c8b
Compare
Drop the generic TransitionTracker. It was only used by the two observers, and the shared wrapper forced the awkward "nothing changed" / "first observation" double-check. Each observer now keeps its own previous-state memory and compares inline, shaping its result directly.
There was a problem hiding this comment.
Shape looks right now, the three layers each do one job and the earlier duplication is gone. Just some small clean up left 🙏
Changelog. Still missing. The Unreleased section has no entry for the session ID or the state logs.
src/instrumentation/CONVENTIONS.md:9. The checklist says one instrumentation class per domain wrapping TelemetryService, and this PR replaces two such classes with the stateless recordWorkspaceState and recordAgentState functions. Either keep the methods but update the conventions or convert them into a class if there's some shared state that is useful.
…ntStateTransition
…into AgentStateTransition" This reverts commit 18bcfa2.
…Transition without breaking tests
Drop the single-field RemovedAgent wrapper. AgentObservation.removed is now string[], so the monitor iterates names directly and the observer test asserts ["second"].
buildStartedAtMs is set only while provisioning and cleared otherwise, so wasProvisioning is exactly buildStartedAtMs !== undefined. Drop both locals and fold the check into the else branch.
Dedup, build-duration resolution, and duration measurements are observer behavior already covered in observers.test.ts. Keep the monitor telemetry test to wiring only: the initial state records once and a later change records again.
Mirror the workspace observer's transition/reason-only case: same status, new lifecycle_state reports one transition.
observers.test.ts and workspaceMonitor.test.ts had the same workspace builder. Move it to test/mocks/testHelpers, typed to take WorkspaceAgent[], and use it from both.
The agent tests hand-rolled { type: "data", data }. Widen workspaceEvent to
accept a full Workspace and use it at both call sites.
What
Add info-level logging of workspace, agent, and lifecycle status transitions so connection debugging has a record of state changes — correlated by the session ID from #1073.
This is Phase 3 of 3 for DEVEX-661 (RFC req 7). It builds on #1074.
Changes
State handling is split into three layers, one job each:
WorkspaceStateObserver/WorkspaceAgentObserver(src/instrumentation/workspace.ts) each detect a transition once and report a transition object. Stateful but effect-free (no logger/telemetry imports). The agent observer tracks every agent by ID and reports agents that disappear.recordWorkspaceState/recordAgentStateemit theworkspace.state_transitioned/workspace.agent.state_transitionedtelemetry events.WorkspaceMonitor.update(), with flat scalar payloads matching the repo's other log sites. Logsstate observedon first sight,state changedafterward, andagent <name> removedwhen an agent disappears.WorkspaceMonitoris now the single agent-observation site, soWorkspaceStateMachineno longer tracks agent telemetry.Important
Telemetry data change.
workspace.agent.state_transitionednow covers every agent for the whole monitored session (deduped per agent), rather than only the connected agent during connection setup. In practice most workspaces have one agent, so volume is unchanged; multi-agent workspaces and flaky connections emit more. There is no sampling insrc/telemetry/, so this lands as-is.EVENTS.mdis updated; heads-up to whoever owns the funnel queries.Testing
pnpm typecheck, targetedpnpm lint, fullpnpm test(2456 passing).record*event shapes, and the monitor's inline logging (observed vs changed, agent transitions across all agents, agent removal).Refactor design (reviewer's three-layer shape)
Per the review, the goal was to remove the duplication between the telemetry observers and the state loggers by making transition detection happen once:
recordWorkspaceState/recordAgentStateinsrc/instrumentation/, telemetry only.This deletes both
src/workspace/*Logger.tsfiles and leaves one tracker per concern.Observer API decision: observers return a transition object (workspace →
WorkspaceStateTransition | undefined; agents →{ transitions, removed }) andWorkspaceMonitor.update()logs inline and callsrecord*. This was chosen over a constructor callback becauseobserve()has a single synchronous caller, so a callback would only hide the side effects and complicate tests.🤖 Generated by Coder Agents.