Skip to content

Commit 097f9ea

Browse files
committed
process_collector_netstat_darwin: address review feedback
- Trim unused trailing fields from nstatCounts, nstatMsgSrcAdded, and nstatMsgErr. binary.Read only reads sizeof(struct) bytes and leaves the rest of the datagram unread, so the Go structs only need to cover the fields we actually use. - Set SO_RCVTIMEO once in openNstatSocket instead of on every read. Did not adopt the suggestion to also query the TCP_KERNEL/UDP_KERNEL providers: verified via GET_SRC_DESC that NSTAT_FILTER_SPECIFIC_USER_BY_PID is not honored by those providers on this machine (they return every socket on the system, not just this process's), which would attribute other processes' traffic to this one. Tested with and without a local VPN active to rule that out as a factor; behavior was identical. Signed-off-by: Dmitry Golovin <sanlm@mail.ru>
1 parent 0a23ef1 commit 097f9ea

1 file changed

Lines changed: 31 additions & 44 deletions

File tree

prometheus/process_collector_netstat_darwin.go

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -71,39 +71,28 @@ type nstatMsgAddAllSrcs struct {
7171
TargetUUID [16]byte
7272
}
7373

74+
// nstatMsgSrcAdded mirrors the leading fields of struct nstat_msg_src_added;
75+
// trailing Provider/Reserved fields are present on the wire but unused here,
76+
// so binary.Read simply leaves them unread.
7477
type nstatMsgSrcAdded struct {
75-
Hdr nstatMsgHdr
76-
SrcRef uint64
77-
Provider uint32
78-
Reserved [4]byte
78+
Hdr nstatMsgHdr
79+
SrcRef uint64
7980
}
8081

8182
type nstatMsgQuerySrcReq struct {
8283
Hdr nstatMsgHdr
8384
SrcRef uint64
8485
}
8586

86-
// nstatCounts mirrors struct nstat_counts. Only the first four fields are used
87-
// here; the rest are read (to consume the full wire message) but not exposed.
87+
// nstatCounts mirrors the leading fields of struct nstat_counts. The real
88+
// struct has more trailing fields (retransmits, RTT estimates, etc.); we only
89+
// need the byte counters, and binary.Read leaves the rest of the message
90+
// unread, so trimming here avoids coupling to the full XNU layout.
8891
type nstatCounts struct {
89-
RxPackets uint64
90-
RxBytes uint64
91-
TxPackets uint64
92-
TxBytes uint64
93-
CellRxBytes uint64
94-
CellTxBytes uint64
95-
WifiRxBytes uint64
96-
WifiTxBytes uint64
97-
WiredRxBytes uint64
98-
WiredTxBytes uint64
99-
RxDuplicateBytes uint32
100-
RxOutOfOrderBytes uint32
101-
TxRetransmit uint32
102-
ConnectAttempts uint32
103-
ConnectSuccesses uint32
104-
MinRtt uint32
105-
AvgRtt uint32
106-
VarRtt uint32
92+
RxPackets uint64
93+
RxBytes uint64
94+
TxPackets uint64
95+
TxBytes uint64
10796
}
10897

10998
type nstatMsgSrcCounts struct {
@@ -113,10 +102,11 @@ type nstatMsgSrcCounts struct {
113102
Counts nstatCounts
114103
}
115104

105+
// nstatMsgErr mirrors the leading fields of struct nstat_msg_error; the
106+
// trailing Reserved field is unused here.
116107
type nstatMsgErr struct {
117-
Hdr nstatMsgHdr
118-
Error uint32
119-
Reserved [4]byte
108+
Hdr nstatMsgHdr
109+
Error uint32
120110
}
121111

122112
// getNetworkBytes returns the total bytes received and sent over the network
@@ -168,6 +158,12 @@ func openNstatSocket() (int, error) {
168158
return -1, fmt.Errorf("nstat: connect: %w", err)
169159
}
170160

161+
tv := unix.NsecToTimeval(nstatReadTimeout.Nanoseconds())
162+
if err := unix.SetsockoptTimeval(fd, unix.SOL_SOCKET, unix.SO_RCVTIMEO, &tv); err != nil {
163+
unix.Close(fd)
164+
return -1, fmt.Errorf("nstat: SetsockoptTimeval: %w", err)
165+
}
166+
171167
return fd, nil
172168
}
173169

@@ -189,11 +185,10 @@ func nstatCollectSrcRefs(fd int, provider uint32, pid int32) ([]uint64, error) {
189185
return nil, err
190186
}
191187

192-
deadline := time.Now().Add(nstatReadTimeout)
193188
var refs []uint64
194189
buf := make([]byte, 4096)
195190
for {
196-
n, err := nstatRead(fd, buf, deadline)
191+
n, err := nstatRead(fd, buf)
197192
if err != nil {
198193
return nil, err
199194
}
@@ -236,10 +231,9 @@ func nstatQueryCounts(fd int, srcref uint64) (rxBytes, txBytes uint64, err error
236231
return 0, 0, err
237232
}
238233

239-
deadline := time.Now().Add(nstatReadTimeout)
240234
buf := make([]byte, 4096)
241235
for {
242-
n, err := nstatRead(fd, buf, deadline)
236+
n, err := nstatRead(fd, buf)
243237
if err != nil {
244238
return 0, 0, err
245239
}
@@ -277,28 +271,21 @@ func nstatSend(fd int, msg any) error {
277271
return nil
278272
}
279273

280-
// nstatRead reads one datagram from the control socket, applying deadline as a
281-
// per-call receive timeout.
282-
func nstatRead(fd int, buf []byte, deadline time.Time) (int, error) {
283-
d := time.Until(deadline)
284-
if d < 0 {
285-
d = 0
286-
}
287-
tv := unix.NsecToTimeval(d.Nanoseconds())
288-
if err := unix.SetsockoptTimeval(fd, unix.SOL_SOCKET, unix.SO_RCVTIMEO, &tv); err != nil {
289-
return 0, fmt.Errorf("nstat: SetsockoptTimeval: %w", err)
290-
}
274+
// nstatRead reads one datagram from the control socket. SO_RCVTIMEO is set
275+
// once on the socket in openNstatSocket, so a stalled kernel response can't
276+
// hang collection forever.
277+
func nstatRead(fd int, buf []byte) (int, error) {
291278
n, err := unix.Read(fd, buf)
292279
if err != nil {
293280
return 0, fmt.Errorf("nstat: read: %w", err)
294281
}
295-
if n < int(unsafeSizeofNstatMsgHdr) {
282+
if n < nstatMsgHdrSize {
296283
return 0, fmt.Errorf("nstat: short read (%d bytes)", n)
297284
}
298285
return n, nil
299286
}
300287

301-
const unsafeSizeofNstatMsgHdr = 16
288+
const nstatMsgHdrSize = 16
302289

303290
func nstatReadHdr(buf []byte) (nstatMsgHdr, error) {
304291
var hdr nstatMsgHdr

0 commit comments

Comments
 (0)