Skip to content

Commit 071fef7

Browse files
sspainksspaink
authored
inputs.nfsclient: use uint64, also update error handling (#9067)
* Use uint64 Fix error handling * update comment * More detail to error
1 parent 66c6396 commit 071fef7

2 files changed

Lines changed: 132 additions & 90 deletions

File tree

‎plugins/inputs/nfsclient/nfsclient.go‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package nfsclient
22

33
import (
44
"bufio"
5-
"log"
5+
"fmt"
66
"os"
77
"regexp"
88
"strconv"
@@ -61,45 +61,48 @@ func (n *NFSClient) Description() string {
6161
return "Read per-mount NFS client metrics from /proc/self/mountstats"
6262
}
6363

64-
func convertToInt64(line []string) []int64 {
64+
func convertToUint64(line []string) ([]uint64, error) {
6565
/* A "line" of input data (a pre-split array of strings) is
6666
processed one field at a time. Each field is converted to
67-
an int64 value, and appened to an array of return values.
68-
On an error, check for ErrRange, and throw a fatal error
67+
an uint64 value, and appened to an array of return values.
68+
On an error, check for ErrRange, and returns an error
6969
if found. This situation indicates a pretty major issue in
7070
the /proc/self/mountstats file, and returning faulty data
7171
is worse than no data. Other errors are ignored, and append
7272
whatever we got in the first place (probably 0).
7373
Yes, this is ugly. */
7474

75-
var nline []int64
75+
var nline []uint64
7676

7777
if len(line) < 2 {
78-
return nline
78+
return nline, nil
7979
}
8080

8181
// Skip the first field; it's handled specially as the "first" variable
8282
for _, l := range line[1:] {
83-
val, err := strconv.ParseInt(l, 10, 64)
83+
val, err := strconv.ParseUint(l, 10, 64)
8484
if err != nil {
8585
if numError, ok := err.(*strconv.NumError); ok {
8686
if numError.Err == strconv.ErrRange {
87-
log.Fatalf("ErrRange: line:[%v] raw:[%v] -> parsed:[%v]\n", line, l, val)
87+
return nil, fmt.Errorf("errrange: line:[%v] raw:[%v] -> parsed:[%v]", line, l, val)
8888
}
8989
}
9090
}
9191
nline = append(nline, val)
9292
}
93-
return nline
93+
return nline, nil
9494
}
9595

96-
func (n *NFSClient) parseStat(mountpoint string, export string, version string, line []string, acc telegraf.Accumulator) {
96+
func (n *NFSClient) parseStat(mountpoint string, export string, version string, line []string, acc telegraf.Accumulator) error {
9797
tags := map[string]string{"mountpoint": mountpoint, "serverexport": export}
98-
nline := convertToInt64(line)
98+
nline, err := convertToUint64(line)
99+
if err != nil {
100+
return err
101+
}
99102

100103
if len(nline) == 0 {
101104
n.Log.Warnf("Parsing Stat line with one field: %s\n", line)
102-
return
105+
return nil
103106
}
104107

105108
first := strings.Replace(line[0], ":", "", 1)
@@ -240,9 +243,11 @@ func (n *NFSClient) parseStat(mountpoint string, export string, version string,
240243
}
241244
}
242245
}
246+
247+
return nil
243248
}
244249

245-
func (n *NFSClient) processText(scanner *bufio.Scanner, acc telegraf.Accumulator) {
250+
func (n *NFSClient) processText(scanner *bufio.Scanner, acc telegraf.Accumulator) error {
246251
var mount string
247252
var version string
248253
var export string
@@ -293,9 +298,14 @@ func (n *NFSClient) processText(scanner *bufio.Scanner, acc telegraf.Accumulator
293298
}
294299

295300
if !skip {
296-
n.parseStat(mount, export, version, line, acc)
301+
err := n.parseStat(mount, export, version, line, acc)
302+
if err != nil {
303+
return fmt.Errorf("could not parseStat: %w", err)
304+
}
297305
}
298306
}
307+
308+
return nil
299309
}
300310

301311
func (n *NFSClient) getMountStatsPath() string {
@@ -316,7 +326,10 @@ func (n *NFSClient) Gather(acc telegraf.Accumulator) error {
316326
defer file.Close()
317327

318328
scanner := bufio.NewScanner(file)
319-
n.processText(scanner, acc)
329+
err = n.processText(scanner, acc)
330+
if err != nil {
331+
return err
332+
}
320333

321334
if err := scanner.Err(); err != nil {
322335
n.Log.Errorf("%s", err)

‎plugins/inputs/nfsclient/nfsclient_test.go‎

Lines changed: 104 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package nfsclient
22

33
import (
44
"bufio"
5-
"github.com/influxdata/telegraf/testutil"
65
"os"
76
"strings"
87
"testing"
8+
9+
"github.com/influxdata/telegraf/testutil"
10+
"github.com/stretchr/testify/require"
911
)
1012

1113
func getMountStatsPath() string {
@@ -24,17 +26,18 @@ func TestNFSClientParsev3(t *testing.T) {
2426
nfsclient.nfs3Ops = map[string]bool{"READLINK": true, "GETATTR": false}
2527
nfsclient.nfs4Ops = map[string]bool{"READLINK": true, "GETATTR": false}
2628
data := strings.Fields(" READLINK: 500 501 502 503 504 505 506 507")
27-
nfsclient.parseStat("1.2.3.4:/storage/NFS", "/A", "3", data, &acc)
29+
err := nfsclient.parseStat("1.2.3.4:/storage/NFS", "/A", "3", data, &acc)
30+
require.NoError(t, err)
2831

2932
fieldsOps := map[string]interface{}{
30-
"ops": int64(500),
31-
"trans": int64(501),
32-
"timeouts": int64(502),
33-
"bytes_sent": int64(503),
34-
"bytes_recv": int64(504),
35-
"queue_time": int64(505),
36-
"response_time": int64(506),
37-
"total_time": int64(507),
33+
"ops": uint64(500),
34+
"trans": uint64(501),
35+
"timeouts": uint64(502),
36+
"bytes_sent": uint64(503),
37+
"bytes_recv": uint64(504),
38+
"queue_time": uint64(505),
39+
"response_time": uint64(506),
40+
"total_time": uint64(507),
3841
}
3942
acc.AssertContainsFields(t, "nfs_ops", fieldsOps)
4043
}
@@ -46,17 +49,41 @@ func TestNFSClientParsev4(t *testing.T) {
4649
nfsclient.nfs3Ops = map[string]bool{"DESTROY_SESSION": true, "GETATTR": false}
4750
nfsclient.nfs4Ops = map[string]bool{"DESTROY_SESSION": true, "GETATTR": false}
4851
data := strings.Fields(" DESTROY_SESSION: 500 501 502 503 504 505 506 507")
49-
nfsclient.parseStat("2.2.2.2:/nfsdata/", "/B", "4", data, &acc)
52+
err := nfsclient.parseStat("2.2.2.2:/nfsdata/", "/B", "4", data, &acc)
53+
require.NoError(t, err)
54+
55+
fieldsOps := map[string]interface{}{
56+
"ops": uint64(500),
57+
"trans": uint64(501),
58+
"timeouts": uint64(502),
59+
"bytes_sent": uint64(503),
60+
"bytes_recv": uint64(504),
61+
"queue_time": uint64(505),
62+
"response_time": uint64(506),
63+
"total_time": uint64(507),
64+
}
65+
acc.AssertContainsFields(t, "nfs_ops", fieldsOps)
66+
}
67+
68+
func TestNFSClientParseLargeValue(t *testing.T) {
69+
var acc testutil.Accumulator
70+
71+
nfsclient := NFSClient{Fullstat: true}
72+
nfsclient.nfs3Ops = map[string]bool{"SETCLIENTID": true, "GETATTR": false}
73+
nfsclient.nfs4Ops = map[string]bool{"SETCLIENTID": true, "GETATTR": false}
74+
data := strings.Fields(" SETCLIENTID: 218 216 0 53568 12960 18446744073709531008 134 197")
75+
err := nfsclient.parseStat("2.2.2.2:/nfsdata/", "/B", "4", data, &acc)
76+
require.NoError(t, err)
5077

5178
fieldsOps := map[string]interface{}{
52-
"ops": int64(500),
53-
"trans": int64(501),
54-
"timeouts": int64(502),
55-
"bytes_sent": int64(503),
56-
"bytes_recv": int64(504),
57-
"queue_time": int64(505),
58-
"response_time": int64(506),
59-
"total_time": int64(507),
79+
"ops": uint64(218),
80+
"trans": uint64(216),
81+
"timeouts": uint64(0),
82+
"bytes_sent": uint64(53568),
83+
"bytes_recv": uint64(12960),
84+
"queue_time": uint64(18446744073709531008),
85+
"response_time": uint64(134),
86+
"total_time": uint64(197),
6087
}
6188
acc.AssertContainsFields(t, "nfs_ops", fieldsOps)
6289
}
@@ -72,14 +99,15 @@ func TestNFSClientProcessStat(t *testing.T) {
7299

73100
scanner := bufio.NewScanner(file)
74101

75-
nfsclient.processText(scanner, &acc)
102+
err := nfsclient.processText(scanner, &acc)
103+
require.NoError(t, err)
76104

77105
fieldsReadstat := map[string]interface{}{
78-
"ops": int64(600),
79-
"retrans": int64(1),
80-
"bytes": int64(1207),
81-
"rtt": int64(606),
82-
"exe": int64(607),
106+
"ops": uint64(600),
107+
"retrans": uint64(1),
108+
"bytes": uint64(1207),
109+
"rtt": uint64(606),
110+
"exe": uint64(607),
83111
}
84112

85113
readTags := map[string]string{
@@ -91,11 +119,11 @@ func TestNFSClientProcessStat(t *testing.T) {
91119
acc.AssertContainsTaggedFields(t, "nfsstat", fieldsReadstat, readTags)
92120

93121
fieldsWritestat := map[string]interface{}{
94-
"ops": int64(700),
95-
"retrans": int64(1),
96-
"bytes": int64(1407),
97-
"rtt": int64(706),
98-
"exe": int64(707),
122+
"ops": uint64(700),
123+
"retrans": uint64(1),
124+
"bytes": uint64(1407),
125+
"rtt": uint64(706),
126+
"exe": uint64(707),
99127
}
100128

101129
writeTags := map[string]string{
@@ -117,57 +145,58 @@ func TestNFSClientProcessFull(t *testing.T) {
117145

118146
scanner := bufio.NewScanner(file)
119147

120-
nfsclient.processText(scanner, &acc)
148+
err := nfsclient.processText(scanner, &acc)
149+
require.NoError(t, err)
121150

122151
fieldsEvents := map[string]interface{}{
123-
"inoderevalidates": int64(301736),
124-
"dentryrevalidates": int64(22838),
125-
"datainvalidates": int64(410979),
126-
"attrinvalidates": int64(26188427),
127-
"vfsopen": int64(27525),
128-
"vfslookup": int64(9140),
129-
"vfsaccess": int64(114420),
130-
"vfsupdatepage": int64(30785253),
131-
"vfsreadpage": int64(5308856),
132-
"vfsreadpages": int64(5364858),
133-
"vfswritepage": int64(30784819),
134-
"vfswritepages": int64(79832668),
135-
"vfsgetdents": int64(170),
136-
"vfssetattr": int64(64),
137-
"vfsflush": int64(18194),
138-
"vfsfsync": int64(29294718),
139-
"vfslock": int64(0),
140-
"vfsrelease": int64(18279),
141-
"congestionwait": int64(0),
142-
"setattrtrunc": int64(2),
143-
"extendwrite": int64(785551),
144-
"sillyrenames": int64(0),
145-
"shortreads": int64(0),
146-
"shortwrites": int64(0),
147-
"delay": int64(0),
148-
"pnfsreads": int64(0),
149-
"pnfswrites": int64(0),
152+
"inoderevalidates": uint64(301736),
153+
"dentryrevalidates": uint64(22838),
154+
"datainvalidates": uint64(410979),
155+
"attrinvalidates": uint64(26188427),
156+
"vfsopen": uint64(27525),
157+
"vfslookup": uint64(9140),
158+
"vfsaccess": uint64(114420),
159+
"vfsupdatepage": uint64(30785253),
160+
"vfsreadpage": uint64(5308856),
161+
"vfsreadpages": uint64(5364858),
162+
"vfswritepage": uint64(30784819),
163+
"vfswritepages": uint64(79832668),
164+
"vfsgetdents": uint64(170),
165+
"vfssetattr": uint64(64),
166+
"vfsflush": uint64(18194),
167+
"vfsfsync": uint64(29294718),
168+
"vfslock": uint64(0),
169+
"vfsrelease": uint64(18279),
170+
"congestionwait": uint64(0),
171+
"setattrtrunc": uint64(2),
172+
"extendwrite": uint64(785551),
173+
"sillyrenames": uint64(0),
174+
"shortreads": uint64(0),
175+
"shortwrites": uint64(0),
176+
"delay": uint64(0),
177+
"pnfsreads": uint64(0),
178+
"pnfswrites": uint64(0),
150179
}
151180
fieldsBytes := map[string]interface{}{
152-
"normalreadbytes": int64(204440464584),
153-
"normalwritebytes": int64(110857586443),
154-
"directreadbytes": int64(783170354688),
155-
"directwritebytes": int64(296174954496),
156-
"serverreadbytes": int64(1134399088816),
157-
"serverwritebytes": int64(407107155723),
158-
"readpages": int64(85749323),
159-
"writepages": int64(30784819),
181+
"normalreadbytes": uint64(204440464584),
182+
"normalwritebytes": uint64(110857586443),
183+
"directreadbytes": uint64(783170354688),
184+
"directwritebytes": uint64(296174954496),
185+
"serverreadbytes": uint64(1134399088816),
186+
"serverwritebytes": uint64(407107155723),
187+
"readpages": uint64(85749323),
188+
"writepages": uint64(30784819),
160189
}
161190
fieldsXprtTCP := map[string]interface{}{
162-
"bind_count": int64(1),
163-
"connect_count": int64(1),
164-
"connect_time": int64(0),
165-
"idle_time": int64(0),
166-
"rpcsends": int64(96172963),
167-
"rpcreceives": int64(96172963),
168-
"badxids": int64(0),
169-
"inflightsends": int64(620878754),
170-
"backlogutil": int64(0),
191+
"bind_count": uint64(1),
192+
"connect_count": uint64(1),
193+
"connect_time": uint64(0),
194+
"idle_time": uint64(0),
195+
"rpcsends": uint64(96172963),
196+
"rpcreceives": uint64(96172963),
197+
"badxids": uint64(0),
198+
"inflightsends": uint64(620878754),
199+
"backlogutil": uint64(0),
171200
}
172201

173202
acc.AssertContainsFields(t, "nfs_events", fieldsEvents)

0 commit comments

Comments
 (0)