Skip to content

Commit c73bbfb

Browse files
authored
fix(e2e/discovery): Capture fixture service diagnostics on failure (#54205)
### What does this PR do? Makes `dumpDebugInfo` in the discovery E2E suite produce usable output again. It queried `/discovery/debug`, which was removed in fd2b24d ("discovery: remove old discovery check", #39417). Every failure since has logged a 404 body instead of any discovery state — `Not found` in `system-probe-lite` mode (system-probe-lite's `NOTFOUND` constant) and `404 page not found` in `system-probe` mode (Go's `ServeMux` default). Both strings are visible in every failure of the two jobs behind DSCVR-621. Replaced with endpoints that still exist, plus the information needed to tell *"discovery did not report the service"* apart from *"the fixture service was not running"* — a distinction the previous output could not make at all: - `GET /discovery/state` — which implementation is live. - `POST /discovery/services` for the fixture services' PIDs. This is the only endpoint that reports what discovery actually sees. It is caller-driven, so it must be told which PIDs to inspect, and it is POST-only with a JSON body in `system-probe-lite`. - `ps` `ELAPSED` per fixture PID. Discovery never asks about a process younger than `discovery.service_collection_min_process_age` (1 minute by default), so a value below that explains a missing service on its own, and one that keeps resetting across dumps means the service is crash-looping rather than booting slowly. - `systemctl status` and `journalctl` per fixture service. The journal is the only place a fixture's own output ends up. - `ss -ltnp`. Discovery only reports a process once it has a listening socket. PIDs come from each unit's cgroup rather than its main PID, because the process the tests match on is not always the main one: `node-json-server` runs through npm, which spawns the `node` process that serves the port. ### Motivation DSCVR-621. Investigating that flake, the suite's own diagnostics contributed nothing: `/discovery/debug` returned a 404 body in all four failing subtests of both jobs, and nothing recorded the fixture services' state, so it was impossible to tell from CI output whether discovery had failed to report a running service or the service had never started. Determining that required reconstructing process lifetimes from `workload-list` dumps. In the two failures behind the ticket the fixture service was in fact not running — `ruby3.0` appears in only one of four `workload-list` dumps of job 1874204604, created 9s before the dump was taken. `journalctl` would have said why in one line. ### Describe how you validated your changes Since a green CI run does not exercise any of this — `dumpDebugInfo` only runs after a failure, and I confirmed the passing `new-e2e-discovery` job contains none of the new log markers — the endpoints and commands were verified directly. **Endpoints**, against a `system-probe-lite` built from this tree and driven with the exact command strings this PR sends: | Request | Response | |---|---| | `GET /discovery/state` | `{"implementation":"system-probe-lite"}` | | `POST /discovery/services`, `{"new_pids":[…]}` + `Content-Type` | `{"services":[{"pid":…,"generated_name":…}],…}` | | `GET /discovery/debug` (the call being removed) | `Not found` — the exact string seen in every DSCVR-621 failure | | `POST /discovery/services` with no `Content-Type` | `Bad request` | | `POST` with a misspelled field | `{"services":[],…}` — unknown fields ignored silently | | malformed JSON | `Bad request` | The misspelled-field row is why the field name is called out in a comment: an empty result is indistinguishable from "matched no services", and `core.Params` cannot be imported here because it belongs to the main module, which `test/new-e2e` does not depend on. **Commands and their exit codes**, on Ubuntu 22.04 / systemd 249 / cgroup v2, matching the image the tests run on: `ps` for an exited PID exits 1 and `curl` to a missing socket exits 7, so both are routed through a helper that keeps the output and drops the status — otherwise a crash-looping fixture (a PID that dies between the cgroup read and `ps`) would have replaced the real failure with an error about the diagnostic. Also confirmed there that `ps`'s header is `ELAPSED`, comma-separated PID lists work, a missing `cgroup.procs` is handled, and `ss -ltn` prints `0.0.0.0:<port>` followed by padding. Additionally: - `dda inv linter.go --targets=./test/new-e2e/tests/discovery` — clean. - `new-e2e-discovery` passes on this branch. - `ss` is already used against these images elsewhere in the E2E suite (`test/new-e2e/tests/agent-metric-pipelines/common/adp.go`, `test/new-e2e/tests/agent-platform/common/bound-port/unix.go`). - No assertions are added or changed — this only affects what is logged when a test has already failed. Not verified: the `system.slice/<svc>.service/cgroup.procs` path against the actual fixture units. The cgroup v2 layout was checked on a matching host, but not with these unit names. ### Additional Notes Diagnostics only, deliberately: this does not attempt to fix the flake. #54206 builds on this one and addresses the cause. Co-authored-by: vincent.whitchurch <vincent.whitchurch@datadoghq.com>
1 parent 2271844 commit c73bbfb

1 file changed

Lines changed: 83 additions & 6 deletions

File tree

test/new-e2e/tests/discovery/linux_test.go

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,18 +169,95 @@ func (s *linuxTestSuite) testLogs(t *testing.T) {
169169
}, 2*time.Minute, 10*time.Second)
170170
}
171171

172+
// sysprobeSocket is the socket on which system-probe serves the discovery
173+
// endpoints.
174+
const sysprobeSocket = "/opt/datadog-agent/run/sysprobe.sock"
175+
176+
// logDiagnostic runs a diagnostic command and logs its output.
177+
//
178+
// Failures are deliberately tolerated. These only run once a test has already
179+
// failed, and several of them legitimately exit non-zero in exactly the
180+
// situations worth debugging: systemctl status for an inactive unit, ps for a
181+
// process which has since exited (which is what a crash-looping service looks
182+
// like), curl if the agent is not running.
183+
//
184+
// The exit status is swallowed by the remote shell rather than handled here
185+
// because Execute() returns no output at all once the command fails: it blanks
186+
// stdout and folds it into the error instead, so the output being collected
187+
// would only be reachable by formatting an error. The redirect is redundant
188+
// with the framework's use of CombinedOutput, and only kept so that wanting
189+
// stderr is stated here rather than relying on that.
190+
func (s *linuxTestSuite) logDiagnostic(t *testing.T, label, cmd string) {
191+
t.Logf("%s:\n%s", label, s.Env().RemoteHost.MustExecute(cmd+" 2>&1 || true"))
192+
}
193+
172194
func (s *linuxTestSuite) dumpDebugInfo(t *testing.T) {
173195
// This is very useful for debugging, but we probably don't want to decode
174196
// and assert based on this in this E2E test since this is an internal
175197
// interface between the agent and system-probe.
176-
discoveredServices := s.Env().RemoteHost.MustExecute("sudo curl -s --unix-socket /opt/datadog-agent/run/sysprobe.sock http://unix/discovery/debug")
177-
t.Log("system-probe services", discoveredServices)
198+
s.logDiagnostic(t, "system-probe discovery state",
199+
"sudo curl -s --unix-socket "+sysprobeSocket+" http://unix/discovery/state")
200+
201+
pids := s.fixtureServicePIDs(services)
202+
if len(pids) > 0 {
203+
// ELAPSED is the first thing to look at: discovery never asks about a
204+
// process younger than discovery.service_collection_min_process_age (1
205+
// minute by default), so a value below that explains a service which
206+
// was never reported, and one which keeps resetting across dumps means
207+
// the service is crash-looping rather than starting slowly.
208+
s.logDiagnostic(t, "fixture service processes",
209+
"sudo ps -o pid,ppid,etimes,comm,args -p "+strings.Join(pids, ","))
210+
211+
// /discovery/services is the only endpoint which reports what
212+
// discovery actually sees. It is caller-driven, so it has to be told
213+
// which PIDs to look at, and system-probe-lite serves it for POST only
214+
// and rejects a request without an explicit Content-Type.
215+
//
216+
// The body is built by hand because it is defined by core.Params in
217+
// pkg/discovery/core, which belongs to the main module that this one
218+
// does not depend on. Keep the field name in sync with it: unknown
219+
// fields are ignored without an error, so a rename there would make
220+
// this log an empty service list rather than fail visibly.
221+
params := fmt.Sprintf(`{"new_pids":[%s]}`, strings.Join(pids, ","))
222+
s.logDiagnostic(t, "system-probe discovery services",
223+
"sudo curl -s -X POST -H 'Content-Type: application/json' -d '"+params+"'"+
224+
" --unix-socket "+sysprobeSocket+" http://unix/discovery/services")
225+
}
226+
227+
s.dumpServiceDiagnostics(t, services)
228+
229+
s.logDiagnostic(t, "workloadmeta store", "sudo datadog-agent workload-list --verbose")
230+
s.logDiagnostic(t, "agent status", "sudo datadog-agent status")
231+
}
178232

179-
workloadmetaStore := s.Env().RemoteHost.MustExecute("sudo datadog-agent workload-list --verbose")
180-
t.Log("workloadmeta store", workloadmetaStore)
233+
// fixtureServicePIDs returns the PIDs in the cgroup of each of the given
234+
// services. Using the cgroup rather than the unit's main PID matters because
235+
// the process the tests match on is not always the main one: node-json-server
236+
// runs through npm, which spawns the node process that serves the port.
237+
func (s *linuxTestSuite) fixtureServicePIDs(servicesList []string) []string {
238+
var pids []string
239+
for _, service := range servicesList {
240+
procs := s.Env().RemoteHost.MustExecute(fmt.Sprintf(
241+
"cat /sys/fs/cgroup/system.slice/%s.service/cgroup.procs 2>/dev/null || true", service))
242+
pids = append(pids, strings.Fields(procs)...)
243+
}
244+
return pids
245+
}
181246

182-
status := s.Env().RemoteHost.MustExecute("sudo datadog-agent status")
183-
t.Log("agent status", status)
247+
// dumpServiceDiagnostics logs the state of the fixture services themselves.
248+
// The journal is the only place their output ends up, so without this a
249+
// service which fails to start, or which crash-loops (the units are
250+
// Restart=always with RestartSec=1), leaves no trace in the test output and is
251+
// indistinguishable from discovery being slow to report it.
252+
func (s *linuxTestSuite) dumpServiceDiagnostics(t *testing.T, servicesList []string) {
253+
s.logDiagnostic(t, "listening sockets", "sudo ss -ltnp")
254+
255+
for _, service := range servicesList {
256+
s.logDiagnostic(t, service+" systemctl status",
257+
"sudo systemctl status --no-pager --full "+service)
258+
s.logDiagnostic(t, service+" journal",
259+
"sudo journalctl --no-pager -n 100 -u "+service)
260+
}
184261
}
185262

186263
func (s *linuxTestSuite) testProcessCheckWithServiceDiscovery(agentConfigStr string, systemProbeConfigStr string, mode discoveryMode) {

0 commit comments

Comments
 (0)