Repository navigation
Add TCP in/out segment counters to netstat - #19
Merged
Merged
Conversation
- Add topsrv_netstat_tcp_in_segs_total and _out_segs_total from the
Tcp: line of /proc/net/snmp, already parsed into the same
proto.Stats map that RetransSegs is read from
- OutSegs is the canonical denominator for the retransmit ratio. The
only egress counter available until now was
topsrv_network_packets_total{direction="tx"}, which also counts pure
ACKs, UDP, ARP and ICMP, so a ratio built on it is not comparable to
the 1-2% industry threshold and understates hosts whose tx is mostly
non-TCP, such as WireGuard UDP encapsulation
- Take InSegs in the same change: it is the matching denominator for
tcp_in_errs_total, and a second agent rollout for one counter would
cost more than these three lines
- Assert both in TestNetstatCollectorProtoCounters: they share the
Tcp: line with RetransSegs, so a host reporting retransmits must
report these too. An alert dividing by a missing denominator drops
the series silently instead of firing
sergeyfast
force-pushed
the
feat/netstat-tcp-segs
branch
from
August 4, 2026 15:08
3a36399 to
8a26ac6
Compare
- Bump actions/checkout v4->v7, actions/setup-go v5->v7 and goreleaser/goreleaser-action v6->v7. All three declared runs.using=node20; GitHub already forces them onto Node 24 and warns on every run, so the shim is the only thing keeping them alive - anchore/sbom-action already ships node24; staticcheck-action and action-shellcheck are composite actions with no Node runtime of their own, so neither needs a bump - goreleaser-action v7 keeps the default version input at "~> v2", so the goreleaser major stays in step with version: 2 in .goreleaser.yml - One warning survives, for actions/cache v4.3.0. It is pinned by SHA inside dominikh/staticcheck-action, which is composite, so it is not ours to bump and v1.4.1 is already the newest. Its use-cache input would skip that step, but only by giving up the staticcheck cache on every run — not worth trading CI time for a cosmetic warning
sergeyfast
force-pushed
the
feat/netstat-tcp-segs
branch
from
August 4, 2026 15:33
999cd8b to
7b994ce
Compare
dominikh/staticcheck-action is composite and pins actions/cache v4.3.0, which declares runs.using=node20. Runners drop Node 20 entirely in autumn 2026, which would fail the required "test" check and block every merge to master. The action's use-cache input guards that step with an if:, so setting it to false keeps Node 20 out of the job. Cost is staticcheck re-analysing from scratch each run. Upstream fix is open as #23 but untouched since 18 June and the repo has had no commits since March, so waiting on it is not a plan.
Replaces the cache dropped in the previous commit rather than doing without it. staticcheck-action points STATICCHECK_CACHE at $RUNNER_TEMP/staticcheck, so caching that path with actions/cache@v6 restores exactly what its own step would have, minus the node20 runtime. Key strategy mirrors upstream: a unique key per commit for storing, with a prefix restore-key to pick up the newest earlier entry.
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
topsrv_netstat_tcp_in_segs_totalandtopsrv_netstat_tcp_out_segs_total, read from theTcp:line of/proc/net/snmp— the sameproto.Statsmap thatRetransSegs,InErrsandOutRstsalready come from.OutSegs— it is the canonical denominator for the TCP retransmit ratio, and until now the agent exposed no TCP-only egress counter at all. The closest substitute,topsrv_network_packets_total{direction="tx"}, counts every egress packet of the interface: TCP segments, pure ACKs, UDP, ARP, ICMP. The numerator (RetransSegs) is TCP-only, so a ratio built from the pair has no physical meaning, cannot be compared against the industry 1–2% action threshold, and systematically understates hosts whose tx is largely non-TCP — worst on WireGuard hosts, where tx is UDP encapsulation while retransmits are counted on the inner TCP.InSegstoo — it is the matching denominator fortcp_in_errs_total, which has exactly the same normalisation gap, and shipping a second agent release for one counter would cost more than these three lines.Verification of the source data
psnet.ProtoCountersreads/proc/net/snmpon Linux but shells out tonetstat -son macOS, where key names may differ — andemitCountersilently skips a missing key, so a typo or a platform gap would produce no metric and no error. Checked rather than assumed:/proc/net/snmpon Linux exposes, within oneTcp:line:InSegs(field 11),OutSegs(12),RetransSegs(13),InErrs(14),OutRsts(15). Any host reporting retransmits reports the new counters too — they cannot diverge.in_segs=34630,out_segs=21393,retransmits=0— a 0% ratio on a healthy host, which is the expected shape.Test plan
go test -race ./...on macOS — green;TestNetstatCollectorProtoCounterstakes the skip path as beforegolang:1.25Linux container — logsprotocol counters present, both newrequireMetricassertions pass. Worth noting the assertion is inert on macOS, so this run is the one that actually covers itgo vet ./...clean;golangci-lintoninternal/topsrv/reports 3goconstinnetstat.go, all on pre-existingNewDesclines 37–39, none introduced herecount(topsrv_netstat_tcp_retransmits_total) - count(topsrv_netstat_tcp_out_segs_total)should reach0across the fleetRollout note for consumers
Any alert rule that divides by the new counter should not be enabled before fleet coverage is complete. A rule of the form
... and on (instance) (rate(out_segs) > N)yields an empty right-hand side on a host that lacks the counter, so the series disappears entirely and that host stops being evaluated — silently. Hosts pinned to an older agent version will not pick this up automatically and need their pin lifted first; the coverage query above is the gate.Thresholds are also not portable from any ratio calibrated against the tx-packets substitute: the denominator gets strictly smaller here, so the measured percentage rises and needs recalibrating against the 1–2% reference.
Out of scope
Also in this PR: CI off the Node 20 runtime
Unrelated to the metric, but folded in here rather than spawning another PR.
GitHub warned on every run that four actions still declare
runs.using=node20. Per the deprecation notice runners defaulted to Node 24 on 16 June 2026 and Node 20 is removed entirely in autumn 2026 — at which point those actions stop launching and take the requiredtestcheck down with them, blocking every merge tomaster. A scheduled breakage, not a cosmetic warning.actions/checkoutv4 → v7,actions/setup-gov5 → v7,goreleaser/goreleaser-actionv6 → v7. All three were node20; all three are node24 at v7.anchore/sbom-actionalready ships node24.dominikh/staticcheck-actionandludeeus/action-shellcheckare composite actions with no Node runtime of their own — nothing to bump.goreleaser-action@v7keeps its defaultversion: "~> v2"input, so the goreleaser major stays in step withversion: 2in.goreleaser.yml. It also runs in thetestjob (args: check), so this PR's CI exercises the bump rather than deferring all of it to the next tag.actions/cachev4.3.0, pinned by SHA insidedominikh/staticcheck-action— not ours to bump, andv1.4.1is already the newest. Upstream has a fix open (#23, mergeable) but untouched since 18 June, with no commits to that repo since March. So the action's own caching is switched off viause-cache: falseand the same directory is cached here instead withactions/cache@v6.The replacement is a faithful one, not a downgrade: staticcheck-action points
STATICCHECK_CACHEat$RUNNER_TEMP/staticcheck, which is exactly the path cached here, and the key strategy mirrors upstream's — unique key per commit for storing, prefix restore-key to pick up the newest earlier entry.Measured on the
testjob: 30s originally, 44s with caching off entirely, 34s with the replacement — the regression is paid back. Annotations went 4 → 0. The first run with the new step already got a cache hit: ourrestore-keys: staticcheck-Linux-also matches the entries the old action left behind understaticcheck-Linux--<sha>, so there was no cold-start cost and the existing cache was not orphaned.Follow-up
Once upstream #23 lands, both the
use-cache: falseline and the explicit cache step should go away in favour of the action's built-in caching. The comment inci.ymlsays so, but a code comment is a weak reminder — worth an issue if this matters.