mrproliu opened a new pull request, #212: URL: https://github.com/apache/skywalking-rover/pull/212
## Summary This PR fixes a production crash and several stability/observability problems found by inspecting a running rover DaemonSet (repeated restarts, a growing memory footprint, and noisy logs). The changes are grouped into one crash fix plus three stability/observability improvements, all in the access log and process-discovery paths. ## Motivation While investigating a rover pod that kept restarting, the following issues were identified from the logs, `pprof`, and `kubectl`: - The pod crashed with `SIGSEGV` in the access log HTTP/2 handler whenever a monitored HTTP/2 stream had no request or response body (for example a `GET` with a `204`/`304`/redirect or a gRPC trailers-only response). - Resident memory grew to several GB. A heap profile showed the dominant consumer was ELF symbol tables retained per monitored process, even though most processes are never profiled. - `socket_data_upload_queue full, dropped N samples` warnings were logged on every burst, flooding the log during traffic spikes. - Thousands of `query the process connection error: ... /proc/<pid>/net/udp: no such file or directory` warnings were produced by short-lived processes exiting between discovery and the connection query. ## Changes ### Bug fix: HTTP/2 access log panic - The root cause is in `AppendSocketDetailsFromBuffer`: an absent (nil) buffer was treated the same as an incomplete one, and the error path then dereferenced the nil buffer to build the message, panicking. - `AppendSocketDetailsFromBuffer` now skips a nil buffer (a legitimately absent request/response body) instead of marking the whole message incomplete; only a buffer that is present but missing some of its detail events (e.g. dropped perf samples) is treated as incomplete. - Both `HTTP2Protocol.HandleWholeStream` and `HTTP1Protocol.HandleHTTPData` now guard the error path (`!allInclude || idRange == nil || len(details) == 0`) and use the shared, nil-safe `DataIDRangeBounds` helper so the message is built without dereferencing a possibly-nil range. ### Improvement: reduce memory by not retaining ELF symbols - Process discovery previously called `ProfilingStat()` only to set the `support_ebpf_profiling` property, which eagerly parsed and permanently retained the full ELF symbol tables of every discovered process. - A new lightweight `SupportProfiling()` reports whether a process is profilable (has symbols and is not excluded) without retaining the parsed data, and discovery now uses it. - `ProfilingStat()` no longer caches the heavy `Info` on the process; it is built on demand by a profiling task's runner and released when the task ends, so symbols are only resident while a process is actually being profiled. ### Improvement: dropped perf sample observability and tuning - Dropped perf event samples are now accumulated per queue and reported in a periodic aggregated summary instead of a warning on every burst; a `drain()` reads and resets the counter each interval so the map does not grow unbounded. - The default access log protocol per-CPU buffer (`ROVER_ACCESS_LOG_PROTOCOL_ANALYZE_PER_CPU_BUFFER`) is raised from `400KB` to `1MB` to absorb the observed bursts, since the agent had ample CPU/memory headroom. ### Improvement: quieter connection query for short-lived processes - A connection query that fails because the process already exited (`os.ErrNotExist` / `no such file or directory`) is now logged at debug level, since it is an expected race for ephemeral processes rather than a real error. ## Behavior changes worth noting - HTTP/1.x and HTTP/2 streams that have no body are now reported instead of being dropped; the send path was already nil-safe (`DataSize()` and `BufferSizeOfZero` handle nil buffers), so this only affects which streams are emitted, not stability. - `support_ebpf_profiling` now reflects "the executable has symbols and is not excluded" rather than "the full profiling info could be built"; the two differ only when reading `/proc/<pid>/maps` fails after the symbol check passes. ## Tests - `AppendSocketDetailsFromBuffer`: nil buffer is skipped, incompleteness is propagated, an empty buffer and a buffer whose details fall outside its captured range are incomplete, and a fully-captured buffer contributes its detail. - `DataIDRangeBounds`: returns `(0, 0)` for a nil range and the real bounds otherwise. - `HandleWholeStream`: incomplete/all-nil streams return an error without panicking (reproduces the original `SIGSEGV` on the pre-fix code), and a bodiless stream is reported through the queue. - `ParseHTTPMethod`, `FirstDetail`, and `BufferSizeOfZero`: cover the default/unknown-method handling and the nil-safe helpers used by the send path. - `lostSampleCounter.drain`: accumulates counts and resets on drain. ## Verification - `go build ./...`, `go vet`, and the changed-package tests pass on `linux` (the eBPF packages only build on Linux). - `golangci-lint` (the project's pinned version) reports `0 issues` on the changed packages. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
