feat(#95): agent — phase 3 detection (web servers + docker + host) - #100
Merged
Conversation
…aefik, Docker, host) Completes the remaining detection work for issue #95 on top of #96's primary-source selection. Six new probe-only gears land their verdicts in the capability manifest so the dashboard sees a complete picture of which HTTP / container sources exist on each host. No metrics collection yet — that lands in Phase 4+ per-source. The four web-server detectors declare CategoryHTTPRequests via MetricSourceGear, so the resolver from #96 now actually picks between alternatives — set GEARBOX_AGENT_HTTP_SOURCE=nginx on a host with both HAProxy and nginx and the manifest flips. Per-source env overrides for non-default surfaces: NGINX_STATUS_URL / NGINX_CONFIG_FILE APACHE_STATUS_URL / APACHE_CONFIG_FILE CADDY_ADMIN_URL TRAEFIK_METRICS_URL DOCKER_SOCKET Also drops the dead internal/framework/discovery/ package (superseded by the ProbeableGear interface from #93; nothing imported it). The discovery/docker.go detection logic was modest — binary lookup + os.Stat + systemctl is-active — and lives on in the new docker gear with the new manifest plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Completes Phase 3 of agent-side source detection (#95) by adding probe-only gears for common HTTP servers plus Docker and Host, wiring new per-source override env vars into config/dependencies, and removing the obsolete internal/framework/discovery/ detection package.
Changes:
- Added probe-only gears for
nginx,apache,caddy,traefik,docker, and always-presenthost, and registered them in the agent. - Introduced shared bounded HTTP probe helper (
internal/framework/probe) and added per-source override env vars/plumbing (config+Dependencies). - Removed dead legacy discovery detectors (
internal/framework/discovery/*) and documented the new detection/override behavior.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| gearbox-agent/README.md | Documents HTTP primary-source overrides and new source-detection override env vars. |
| gearbox-agent/cmd/gearbox-agent/main.go | Registers new gears and wires per-source override fields into dependencies. |
| gearbox-agent/internal/framework/probe/probe.go | Adds shared bounded HTTP probe helper used by detectors. |
| gearbox-agent/internal/framework/gear/dependencies.go | Adds per-source override fields passed into gear probes. |
| gearbox-agent/internal/framework/config/config.go | Loads per-source override env vars into agent config. |
| gearbox-agent/internal/framework/config/config_test.go | Adds tests for per-source override env-var parsing behavior. |
| gearbox-agent/internal/gears/nginx/plugin.go | Adds nginx detection gear (stub_status probe + capability facts). |
| gearbox-agent/internal/gears/nginx/plugin_probe_test.go | Unit tests for nginx probe logic and config-path resolution. |
| gearbox-agent/internal/gears/apache/plugin.go | Adds Apache detection gear (mod_status probe + binary/config detection). |
| gearbox-agent/internal/gears/apache/plugin_probe_test.go | Unit tests for Apache probe logic and config-path resolution. |
| gearbox-agent/internal/gears/caddy/plugin.go | Adds Caddy detection gear (admin/metrics probe + version capture). |
| gearbox-agent/internal/gears/caddy/plugin_probe_test.go | Unit tests for Caddy probe logic including override short-circuit. |
| gearbox-agent/internal/gears/traefik/plugin.go | Adds Traefik detection gear (Prometheus probe with default URL fallback + dashboard API fact). |
| gearbox-agent/internal/gears/traefik/plugin_probe_test.go | Unit tests for Traefik probe fallback order, override behavior, and dashboard API fact recording. |
| gearbox-agent/internal/gears/docker/plugin.go | Adds Docker detection gear (binary/version + socket-path detection). |
| gearbox-agent/internal/gears/docker/plugin_probe_test.go | Unit tests for Docker probe outcomes and DOCKER_SOCKET override. |
| gearbox-agent/internal/gears/host/plugin.go | Adds always-available host “facts” gear surfaced via capabilities. |
| gearbox-agent/internal/gears/host/plugin_probe_test.go | Unit tests for host fact collection and graceful degradation. |
| gearbox-agent/docs/source-detection.md | New long-form documentation for detection precedence and troubleshooting. |
| gearbox-agent/internal/framework/discovery/detector.go | Deletes superseded legacy detection framework. |
| gearbox-agent/internal/framework/discovery/haproxy.go | Deletes legacy HAProxy detector (superseded by gear probes). |
| gearbox-agent/internal/framework/discovery/docker.go | Deletes legacy Docker detector (ported into new docker gear). |
| gearbox-agent/internal/framework/discovery/systemd.go | Deletes legacy systemd detector (superseded by host/systemd presence reporting). |
Substantive: - probe.isLoopback: parse URL with net/url + check net.IP.IsLoopback so userinfo-spoofed URLs like https://localhost@evil.com/ can't trick the helper into skipping TLS verification against evil.com. - probe.HTTPGet: validate maxBody > 0; return error instead of silently returning empty body that would hide sentinel mismatches. - docker probe: capture stat's FileInfo and check Mode()&ModeSocket so a regular file or directory at the socket path is flagged Inaccessible with a reason that points at the bind-mount mismatch. - traefik probe: track the last non-matching HTTP response across the fallback URL list; the Inaccessible reason now distinguishes "wrong service on this port (200 without sentinel)" from "no listener (connection refused)" so operators debug the right cause. Comment/text alignment: - caddy/traefik/apache Info().Description: Phase 4+ → Phase 7+ to match each file's package header and the docs. - nginx wellKnownConfigPaths comment: "all four" → "all three" (slice has three entries). - nginx Probe comment: "two `nginx -V` runs" → "one `nginx -v` and one `nginx -V`" — accurate process-invocation count. - docs/source-detection.md: "six-step decision tree" → "four-step" to match the rendered numbered list. New tests: - probe_test.go: isLoopback covers userinfo spoof regression, plain loopback variants, and unparseable input. HTTPGet rejects non-positive maxBody. - docker plugin_probe_test.go: new regression test for "path exists but is not a socket". Existing fakeFileInfo now carries a Mode field. - traefik plugin_probe_test.go: new test asserts the Inaccessible reason distinguishes "200 without Traefik sentinel" from generic "unreachable". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
8 tasks
sarg3nt
added a commit
that referenced
this pull request
May 15, 2026
…dy, Traefik) + access-log (#101) * feat(#91): agent — phase 4/5/7 metrics collection (nginx, Apache, Caddy, Traefik) + access-log endpoint Builds on PR #100's detection layer with periodic metric scraping for the four web servers plus a structured access-log endpoint. This finishes the agent-side work for issue #91; the dashboard side (per-source chart cards, multi-source Error Insights, DB migration) ships as the next PR. Per-source collectors (each adds a CollectorGear collector + /api/v1/{name}/stats endpoint, cached snapshot, force=true synchronous re-scrape): - nginx: parses stub_status (active/reading/writing/waiting + monotonic accepts/handled/requests). - apache: parses mod_status?auto's key-value format (Total Accesses, worker pool, CPU load, ReqPerSec, etc.). - caddy: scrapes Prometheus at :2019/metrics; sums caddy_http_requests_total + request_errors_total; flags admin status via caddy_admin_http_requests_total presence. - traefik: scrapes Prometheus; buckets traefik_router_requests_total by status-class label so the dashboard gets a real 2xx/3xx/4xx/5xx breakdown; also surfaces the entrypoints list. Access-log endpoint (Phase 5): - New internal/framework/services/accesslog/ package with 5 profile parsers: haproxy, nginx-combined, apache-common, apache-combined, caddy-json. The dashboard's existing parseHAProxyLogLine is ported into the haproxy profile byte-for-byte (with one regex tightening: the syslog [pid] bracket no longer claims the date match). - New internal/gears/accesslog/ gear: GET /api/v1/access-log/{source}/recent?status_min=500&limit=500 reads the last N lines of the source's access log via tail, parses each line with the matching profile, filters by status_min, returns newest-first. - 4 new env vars to override default log paths (HAPROXY_ACCESS_LOG, NGINX_ACCESS_LOG, APACHE_ACCESS_LOG, CADDY_ACCESS_LOG). Apache falls back from /var/log/apache2/ to /var/log/httpd/ for RHEL hosts. Capability manifest reports which sources have a readable log on this host. Shared helper: internal/framework/services/promtext/ — minimal Prometheus exposition-format parser (samples + label maps; counter sums; SumByNameWithLabel for status-class extraction). Scoped to the agent's needs to avoid pulling in prometheus/common's 50+-package transitive footprint just for two scrape routines. Test coverage: - Each new collector has unit tests covering parser correctness, scrape success / failure modes, 503 before first scrape, cached response shape, force=true behaviour, override resolution. - Access-log gear tests cover probe verdict, capabilities map, unknown source 404, no-log available=false envelope, status_min filtering, limit cap, tail-failure surfacing, isReadable's non-regular-file rejection. - 5 parser profiles each have happy-path + reject-noise tests including HAProxy negative-Tt handling and Caddy non-HTTP entries. - promtext tests cover summation, label-value escapes, malformed lines, trailing scrape timestamps. 23 files added, 7 modified. go build / vet / test / gofmt all clean. Out of scope (Phase 6, 8 + dashboard wire-up): - DB migration adding source column to traffic_flows. - Multi-source Error Insights (dashboard refactor). - Cross-source aggregates (Phase 8 — optional). - Source-aware ingest from these endpoints to traffic_flows. - Per-source chart cards / KPIs / capability gates. These all live in the dashboard repo and ship as the next PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#91): address Copilot review findings on PR #101 - access-log: implement the documented Apache CLF fallback. Previously the handler used a single parser from sourceProfile (ApacheCombined) and the comments + PR body claimed a per-record fallback to ApacheCommon that didn't exist. RHEL hosts running default CLF would have produced zero parsed records. New parseWithFallback helper + sourceFallbackProfile map drive the actual fallback now; Apache is the only source using it today. - access-log: status_min query parameter now accepts an explicit 0 to disable the filter. Was previously clamped to a 100 minimum, which silently coerced 0 to 100 and broke the "give me all records" intent. Default when the param is absent stays 500 (the dashboard's primary use case). Lock the new defaults in via two new tests. - caddy: drop the AdminRunning field. The previous heuristic relied on caddy_admin_http_requests_total existing, which Prometheus doesn't emit for counters with zero increments. A freshly-started Caddy with admin enabled but no admin traffic yet would have falsely read "admin disconnected." The real signal is "did the scrape succeed?" — which the handler already conveys via 503 before the first successful scrape — so the field was redundant on success and misleading on cold start. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the remaining detection work for #95 on top of #96's primary-source selection. Six new probe-only gears (
nginx,apache,caddy,traefik,docker,host) land their verdicts in the capability manifest so the dashboard sees a complete picture of which HTTP / container sources exist on each host. No metrics collection yet — that's Phase 4+, shipped per-source as separate issues.The four web-server detectors declare
CategoryHTTPRequestsviaMetricSourceGear, so the resolver from #96 now actually picks between alternatives — setGEARBOX_AGENT_HTTP_SOURCE=nginxon a host that has both HAProxy and nginx and the manifest flips. Until this PR, the resolver could only ever pick HAProxy because nothing else declared the category.Also drops the dead
internal/framework/discovery/package (superseded byProbeableGearin #93; nothing imported it). The discovery/docker.go detection logic lives on in the newdockergear with the new manifest plumbing.Per-source detection
Each new gear follows the precedence model from #95:
NGINX_STATUS_URL, etc.) → trust operator, returnavailablewithout a synchronous probe.not_installed.available.inaccessiblewith an operator-readable reason that names the fix.Sentinel matching (e.g.
Active connections:for nginx stub_status,caddy_http_requests_totalfor Caddy's Prometheus) protects against catch-all vhosts returning 200 with unrelated bodies. Each gear'sProbe()is cheap and bounded — 1s timeout on HTTP probes, no retries, no log noise.Per-source override env vars:
NGINX_STATUS_URLstub_statusURL (skips the default probe).NGINX_CONFIG_FILEnginx.confpath.APACHE_STATUS_URLmod_statusURL (e.g.?autovariant).APACHE_CONFIG_FILEhttpd.conf/apache2.confpath.CADDY_ADMIN_URLTRAEFIK_METRICS_URLDOCKER_SOCKETFiles
internal/framework/probe/probe.go(new)internal/gears/host/(new)hostentry (hostname, kernel, CPU count, systemd presence).internal/gears/docker/(new)discovery/docker.gointo a gear; honoursDOCKER_SOCKET.internal/gears/nginx/(new)--with-http_api_moduleflagging.internal/gears/apache/(new)apache2(Debian) andhttpd(RHEL) binaries; mod_status probe.internal/gears/caddy/(new)internal/gears/traefik/(new):8082→:8080fallback + dashboard API capability.internal/framework/discovery/(deleted)ProbeableGear.internal/framework/config/config.gointernal/framework/gear/dependencies.goDependenciesfields.cmd/gearbox-agent/main.godocs/source-detection.md(new)README.mdOverlap with
feature/containers-gearThe in-progress
feature/containers-gearbranch adds a much richer Docker integration (talks to dockerd viagithub.com/moby/moby/client, lists containers, has dashboard UI). That branch:internal/framework/discovery/files — its owninternal/gears/containers/detector.gouses the moby/client library directly. Confirmed before opening this PR.discovery/files in its tree. After this PR lands, the rebase onto new main will conflict on those four files; accept the deletions.docker(cheap probe-only) and containers-gear's richercontainers. They have differentInfo().Names so the registry handles both fine. Potential follow-up cleanup: fold the cheap detector intocontainers, or havecontainersdepend on thedockercapability map.Test plan
go test ./...— all packages pass (23 files changed, 2643 lines added).go vet ./...clean.gofmt -lclean on every file added or modified in this PR (existing repo-wide formatting drift is unchanged).markdownlint-cliclean on the new doc + updated README.light-hugger: confirm the manifest'sgearsmap now includes the six new entries with the right verdicts; HAProxy still winsprimary_sources.http_requestssince no other producer is installed there.Out of scope (deferred to followup issues)
stub_status, parsing access logs for status-code breakdowns, surfacing in the dashboard.docs/source-detection.md.Backwards compatibility
Purely additive. Capability manifest shape grows new top-level keys per gear; the dashboard already iterates and handles unknown keys gracefully (#93). Older dashboards talking to a phase-3 agent simply ignore the new gears until per-source UI lands in Phase 4+. Older agents talking to a phase-3 dashboard look the same as today — manifest carries only
haproxy/metrics/ etc.Closes #95.
🤖 Generated with Claude Code