Skip to content

Commit 8edae66

Browse files
sarg3ntclaude
andauthored
feat(logs): derive default log sources from agent capabilities (#116)
* feat(logs): derive default log sources from agent capabilities (#112) APILogSourcesHandler returned a hardcoded [haproxy, system] pair whenever no explicit per-box settings existed. On a box whose agent has no `logs` (journalctl) or `access-log` gear — e.g. mjolnir's distroless container agent — the Logs page would still load both defaults and then trip every fetch against the agent, which has nothing to serve. Combined with the plain-text 5xx error responses (see #112 / sibling PR #114) this produced the user-visible "Error loading logs: Unexpected token 'F'" toast. Drive the no-settings default off the agent's probe table: * `haproxy` is offered when either access-log OR logs gear is available (access-log streams directly from /var/log/haproxy; the logs gear can serve it via journalctl/file too). * `system` is offered only when the `logs` gear is available (it needs journalctl/tail). * If the agent has neither, return an empty source list — the JS renders an empty dropdown and skips the immediate fetch instead of 5xx-storming. * If capabilities aren't reachable at all (agent down, older agent that pre-dates probing) fall back to the legacy pair so existing deployments still work through transient outages. Phase 2 slice of #112. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(logs): address Copilot review on capability presence vs availability (#112) Copilot pointed out that defaultLogSourcesForBox conflated two distinct cases: 1. The agent doesn't surface a gear name at all (older agent that pre-dates the gear). 2. The agent reports the gear as unavailable. Both used IsAvailable() and collapsed into "return empty list", which violates the function comment's fail-open promise: a forward-compatibility gap would silently strip the source list. Use caps.Has() to distinguish presence from availability: - If the agent surfaces neither access-log nor logs by name → fail open to the legacy [haproxy, system] pair (older-agent path). - If the agent surfaces at least one and reports both unavailable → return empty (the only case where the JS shows an empty dropdown). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 852cb98 commit 8edae66

1 file changed

Lines changed: 77 additions & 7 deletions

File tree

gearbox/internal/framework/handler/api_logs.go

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,22 @@ func (h *Handler) APILogsHandler(w http.ResponseWriter, r *http.Request) {
4848
}
4949

5050
// APILogSourcesHandler returns the enabled log sources for a server.
51-
// If no explicit settings exist, it fetches available sources from the agent.
51+
//
52+
// Source resolution precedence (issue #112):
53+
// 1. Explicit per-box settings in the database — operator opted in.
54+
// 2. Defaults derived from the agent's probe table — only offer sources
55+
// the agent can actually serve. `haproxy` requires the agent's
56+
// `access-log` or `logs` gear; `system` requires `logs`.
57+
// 3. Fail-open: if the agent is unreachable, fall back to the legacy
58+
// `[haproxy, system]` pair so an existing deployment still works
59+
// through a transient outage.
60+
//
61+
// Historically this method returned the legacy pair unconditionally
62+
// when no settings existed, which caused the Logs page on a box without
63+
// a `logs` / `access-log` gear (e.g. the mjolnir agent in a distroless
64+
// container) to immediately ask for haproxy logs the agent couldn't
65+
// serve, producing the "Failed to..." JSON parse error the page used
66+
// to render.
5267
func (h *Handler) APILogSourcesHandler(w http.ResponseWriter, r *http.Request) {
5368
boxID := chi.URLParam(r, "boxID")
5469
if boxID == "" {
@@ -64,14 +79,13 @@ func (h *Handler) APILogSourcesHandler(w http.ResponseWriter, r *http.Request) {
6479
return
6580
}
6681

67-
// If no explicit settings, return default sources (haproxy, system)
82+
// If no explicit settings, derive defaults from the agent's probe
83+
// table so a Logs page on a box without those gears doesn't immediately
84+
// ask for sources the agent can't serve.
6885
if len(sources) == 0 {
6986
h.writeJSON(w, map[string]interface{}{
70-
"server_id": boxID,
71-
"sources": []map[string]string{
72-
{"name": "haproxy", "display_name": "HAProxy"},
73-
{"name": "system", "display_name": "System"},
74-
},
87+
"server_id": boxID,
88+
"sources": h.defaultLogSourcesForBox(boxID),
7589
"has_settings": false,
7690
})
7791
return
@@ -92,3 +106,59 @@ func (h *Handler) APILogSourcesHandler(w http.ResponseWriter, r *http.Request) {
92106
"has_settings": true,
93107
})
94108
}
109+
110+
// defaultLogSourcesForBox returns the default Logs-page source picker
111+
// entries for a box, derived from the agent's probe table. Used when
112+
// the operator hasn't saved explicit log-source settings.
113+
//
114+
// Heuristic:
115+
// - `haproxy` is offered when EITHER access-log OR logs gear is
116+
// available. access-log streams the HAProxy access log directly;
117+
// logs streams it via journalctl/file when journalctl is present.
118+
// - `system` is offered only when the `logs` gear is available, since
119+
// system journal access requires journalctl/tail.
120+
//
121+
// Fail-open posture (matches filterGearsByAgentCapabilities):
122+
// - Capabilities unreachable (agent down, no API key) → legacy pair.
123+
// - Agent didn't surface BOTH gears (older agent that pre-dates
124+
// access-log/logs probing) → legacy pair. We distinguish "the
125+
// agent didn't tell us about this gear" (caps.Has == false) from
126+
// "the agent said this gear is unavailable" (caps.Has == true,
127+
// IsAvailable == false) so a forward-compatibility gap doesn't
128+
// silently strip the source list.
129+
// - Agent surfaced both gears and reported both unavailable → empty
130+
// list. This is the only case the JS renders an empty dropdown
131+
// and skips the immediate fetch — better than 5xx storming.
132+
func (h *Handler) defaultLogSourcesForBox(boxID string) []map[string]string {
133+
legacy := []map[string]string{
134+
{"name": "haproxy", "display_name": "HAProxy"},
135+
{"name": "system", "display_name": "System"},
136+
}
137+
caps, ok := h.getBoxCapabilities(boxID)
138+
if !ok {
139+
return legacy
140+
}
141+
142+
knowsAccessLog := caps.Has("access-log")
143+
knowsLogs := caps.Has("logs")
144+
if !knowsAccessLog && !knowsLogs {
145+
// Agent doesn't surface either gear name yet — likely an older
146+
// agent that pre-dates this probe. Fail open.
147+
return legacy
148+
}
149+
150+
hasAccessLog := knowsAccessLog && caps.IsAvailable("access-log")
151+
hasLogs := knowsLogs && caps.IsAvailable("logs")
152+
153+
out := make([]map[string]string, 0, 2)
154+
if hasAccessLog || hasLogs {
155+
out = append(out, map[string]string{"name": "haproxy", "display_name": "HAProxy"})
156+
}
157+
if hasLogs {
158+
out = append(out, map[string]string{"name": "system", "display_name": "System"})
159+
}
160+
// Result may be empty here — agent explicitly reported both gears
161+
// unavailable. JS renders an empty dropdown rather than 5xx-storming
162+
// fetches for sources the agent can't serve.
163+
return out
164+
}

0 commit comments

Comments
 (0)