Skip to content

Commit 02f7b49

Browse files
synaretesynarete
committed
metrics: code cleanup for json-only mode
Improve code readability: with json-only mode function and variable names may be shorten and have simpler names. In addition, removed dead code. Signed-off-by: Shachar Sharon <[email protected]>
1 parent 9e751f0 commit 02f7b49

File tree

3 files changed

+42
-82
lines changed

3 files changed

+42
-82
lines changed

‎internal/metrics/collectors.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ type smbLocksCollector struct {
119119
}
120120

121121
func (col *smbLocksCollector) Collect(ch chan<- prometheus.Metric) {
122-
locks, _ := RunSmbStatusLockedFiles()
122+
locks, _ := RunSmbStatusLocks()
123123
ch <- prometheus.MustNewConstMetric(col.dsc[0],
124124
prometheus.GaugeValue, float64(len(locks)))
125125
}

‎internal/metrics/smbstatus.go

Lines changed: 27 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"os"
99
"os/exec"
1010
"strings"
11-
"time"
1211
)
1312

1413
// SmbStatusServerID represents a server_id output field
@@ -77,8 +76,8 @@ type SmbStatusLockedFile struct {
7776
NumPendingDeletes int `json:"num_pending_deletes"`
7877
}
7978

80-
// SmbStatusJSON represents output of 'smbstatus --json'
81-
type SmbStatusJSON struct {
79+
// SmbStatus represents output of 'smbstatus --json'
80+
type SmbStatus struct {
8281
Timestamp string `json:"timestamp"`
8382
Version string `json:"version"`
8483
SmbConf string `json:"smb_conf"`
@@ -87,39 +86,15 @@ type SmbStatusJSON struct {
8786
LockedFiles map[string]SmbStatusLockedFile `json:"locked_files"`
8887
}
8988

90-
// SmbStatusProc represents a single entry from the output of 'smbstatus -p'
91-
type SmbStatusProc struct {
92-
PID string
93-
Username string
94-
Group string
95-
Machine string
96-
ProtocolVersion string
97-
Encryption string
98-
Signing string
99-
}
100-
101-
// SmbStatusLock represents a single entry from the output of 'smbstatus -L'
102-
type SmbStatusLock struct {
103-
PID string
104-
UserID string
105-
DenyMode string
106-
Access string
107-
RW string
108-
Oplock string
109-
SharePath string
110-
Name string
111-
Time string
112-
}
113-
114-
// SmbStatusJSON represents output of 'smbstatus -L --json'
115-
type SmbStatusLocksJSON struct {
89+
// SmbStatusLocks represents output of 'smbstatus -L --json'
90+
type SmbStatusLocks struct {
11691
Timestamp string `json:"timestamp"`
11792
Version string `json:"version"`
11893
SmbConf string `json:"smb_conf"`
11994
OpenFiles map[string]SmbStatusLockedFile `json:"open_files"`
12095
}
12196

122-
// LocateSmbStatus finds the local executable of 'smbstatus' on host container.
97+
// LocateSmbStatus finds the local executable of 'smbstatus' on host.
12398
func LocateSmbStatus() (string, error) {
12499
knowns := []string{
125100
"/usr/bin/smbstatus",
@@ -149,18 +124,18 @@ func RunSmbStatusVersion() (string, error) {
149124
return ver, nil
150125
}
151126

152-
// RunSmbStatusShares executes 'smbstatus -S --json' on host container
127+
// RunSmbStatusShares executes 'smbstatus --shares --json' on host
153128
func RunSmbStatusShares() ([]SmbStatusTreeCon, error) {
154-
dat, err := executeSmbStatusCommand("-S --json")
129+
dat, err := executeSmbStatusCommand("--shares --json")
155130
if err != nil {
156131
return []SmbStatusTreeCon{}, err
157132
}
158-
return parseSmbStatusSharesAsJSON(dat)
133+
return parseSmbStatusTreeCons(dat)
159134
}
160135

161-
func parseSmbStatusSharesAsJSON(dat string) ([]SmbStatusTreeCon, error) {
136+
func parseSmbStatusTreeCons(dat string) ([]SmbStatusTreeCon, error) {
162137
tcons := []SmbStatusTreeCon{}
163-
res, err := parseSmbStatusJSON(dat)
138+
res, err := parseSmbStatus(dat)
164139
if err != nil {
165140
return tcons, err
166141
}
@@ -170,18 +145,18 @@ func parseSmbStatusSharesAsJSON(dat string) ([]SmbStatusTreeCon, error) {
170145
return tcons, nil
171146
}
172147

173-
// RunSmbStatusLocks executes 'smbstatus -L --json' on host container
174-
func RunSmbStatusLockedFiles() ([]SmbStatusLockedFile, error) {
175-
dat, err := executeSmbStatusCommand("-L --json")
148+
// RunSmbStatusLocks executes 'smbstatus --locks --json' on host
149+
func RunSmbStatusLocks() ([]SmbStatusLockedFile, error) {
150+
dat, err := executeSmbStatusCommand("--locks --json")
176151
if err != nil {
177152
return []SmbStatusLockedFile{}, err
178153
}
179-
return parseSmbStatusLocksAsJSON(dat)
154+
return parseSmbStatusLockedFiles(dat)
180155
}
181156

182-
func parseSmbStatusLocksAsJSON(dat string) ([]SmbStatusLockedFile, error) {
157+
func parseSmbStatusLockedFiles(dat string) ([]SmbStatusLockedFile, error) {
183158
lockedFiles := []SmbStatusLockedFile{}
184-
res, err := parseSmbStatusLocksJSON(dat)
159+
res, err := parseSmbStatusLocks(dat)
185160
if err != nil {
186161
return lockedFiles, err
187162
}
@@ -194,16 +169,16 @@ func parseSmbStatusLocksAsJSON(dat string) ([]SmbStatusLockedFile, error) {
194169
// SmbStatusSharesByMachine converts the output of RunSmbStatusShares into map
195170
// indexed by machine's host
196171
func SmbStatusSharesByMachine() (map[string][]SmbStatusTreeCon, error) {
197-
shares, err := RunSmbStatusShares()
172+
tcons, err := RunSmbStatusShares()
198173
if err != nil {
199174
return map[string][]SmbStatusTreeCon{}, err
200175
}
201-
return makeSmbSharesMap(shares), nil
176+
return makeSmbSharesMap(tcons), nil
202177
}
203178

204-
func makeSmbSharesMap(shares []SmbStatusTreeCon) map[string][]SmbStatusTreeCon {
179+
func makeSmbSharesMap(tcons []SmbStatusTreeCon) map[string][]SmbStatusTreeCon {
205180
ret := map[string][]SmbStatusTreeCon{}
206-
for _, share := range shares {
181+
for _, share := range tcons {
207182
ret[share.Machine] = append(ret[share.Machine], share)
208183
}
209184
return ret
@@ -227,33 +202,18 @@ func executeCommand(command string, arg ...string) (string, error) {
227202
return res, nil
228203
}
229204

230-
func ParseTime(s string) (time.Time, error) {
231-
layouts := []string{
232-
time.ANSIC,
233-
time.UnixDate,
234-
time.RFC3339,
235-
}
236-
for _, layout := range layouts {
237-
if t, err := time.Parse(layout, s); err == nil {
238-
return t, nil
239-
}
240-
}
241-
// samba's lib/util/time.c uses non standad layout...
242-
return time.Time{}, errors.New("unknow time format " + s)
243-
}
244-
245-
// parseSmbStatusJSON parses to output of 'smbstatus --json' into internal
205+
// parseSmbStatus parses to output of 'smbstatus --json' into internal
246206
// representation.
247-
func parseSmbStatusJSON(data string) (*SmbStatusJSON, error) {
248-
res := SmbStatusJSON{}
207+
func parseSmbStatus(data string) (*SmbStatus, error) {
208+
res := SmbStatus{}
249209
err := json.Unmarshal([]byte(data), &res)
250210
return &res, err
251211
}
252212

253-
// parseSmbStatusJSON parses to output of 'smbstatus -L --json' into internal
254-
// representation.
255-
func parseSmbStatusLocksJSON(data string) (*SmbStatusLocksJSON, error) {
256-
res := SmbStatusLocksJSON{}
213+
// parseSmbStatusLocks parses to output of 'smbstatus --locks --json' into
214+
// internal representation.
215+
func parseSmbStatusLocks(data string) (*SmbStatusLocks, error) {
216+
res := SmbStatusLocks{}
257217
err := json.Unmarshal([]byte(data), &res)
258218
return &res, err
259219
}

‎internal/metrics/smbstatus_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
//revive:disable line-length-limit
1212
//nolint:revive,lll
1313
var (
14-
outputSmbStatusJSON = `
14+
smbstatusOutput1 = `
1515
{
1616
"timestamp": "2022-07-19T16:26:34.652845+0530",
1717
"version": "4.17.0pre1-GIT-130283cbae0",
@@ -63,7 +63,7 @@ var (
6363
}
6464
`
6565

66-
outputSmbStatusJSON2 = `
66+
smbstatusOutput2 = `
6767
{
6868
"timestamp": "2023-06-07T11:49:05.528375+0000",
6969
"version": "4.17.8",
@@ -93,7 +93,7 @@ var (
9393
}
9494
}`
9595

96-
outputSmbStatusAllJSON = `
96+
smbstatusOutput3 = `
9797
{
9898
"timestamp": "2022-04-15T18:25:15.364891+0200",
9999
"version": "4.17.0pre1-GIT-a0f12b9c80b",
@@ -217,7 +217,7 @@ var (
217217
218218
`
219219

220-
outputSmbStatusAllJSON2 = `
220+
smbstatusOutput4 = `
221221
{
222222
"timestamp": "2022-07-20T12:07:36.225955+0000",
223223
"version": "4.17.0pre1-UNKNOWN",
@@ -414,7 +414,7 @@ var (
414414
}
415415
`
416416

417-
outputSmbStatusLocksJSON = `
417+
smbstatusLocksOutput = `
418418
{
419419
"timestamp": "2024-04-14T14:53:34.901974+0300",
420420
"version": "4.21.0pre1-GIT-58a018fb7ad",
@@ -553,16 +553,16 @@ var (
553553

554554
//revive:enable line-length-limit
555555

556-
func TestParseSmbStatusSharesJSON(t *testing.T) {
557-
dat, err := parseSmbStatusJSON(outputSmbStatusJSON)
556+
func TestParseSmbStatusTCons(t *testing.T) {
557+
dat, err := parseSmbStatus(smbstatusOutput1)
558558
assert.NoError(t, err)
559559
assert.Equal(t, len(dat.TCons), 2)
560560

561-
dat, err = parseSmbStatusJSON(outputSmbStatusJSON2)
561+
dat, err = parseSmbStatus(smbstatusOutput2)
562562
assert.NoError(t, err)
563563
assert.Equal(t, len(dat.TCons), 1)
564564

565-
shares, err := parseSmbStatusSharesAsJSON(outputSmbStatusJSON2)
565+
shares, err := parseSmbStatusTreeCons(smbstatusOutput2)
566566
assert.NoError(t, err)
567567
assert.Equal(t, len(shares), 1)
568568
share1 := shares[0]
@@ -579,20 +579,20 @@ func TestParseSmbStatusSharesJSON(t *testing.T) {
579579
}
580580
}
581581

582-
func TestParseSmbStatusAllJSON(t *testing.T) {
583-
dat, err := parseSmbStatusJSON(outputSmbStatusAllJSON)
582+
func TestParseSmbStatusAll(t *testing.T) {
583+
dat, err := parseSmbStatus(smbstatusOutput3)
584584
assert.NoError(t, err)
585585
assert.Equal(t, len(dat.Sessions), 1)
586586
assert.Equal(t, len(dat.TCons), 1)
587587
assert.Equal(t, len(dat.LockedFiles), 1)
588588

589-
dat2, err := parseSmbStatusJSON(outputSmbStatusAllJSON2)
589+
dat2, err := parseSmbStatus(smbstatusOutput4)
590590
assert.NoError(t, err)
591591
assert.Equal(t, len(dat2.LockedFiles), 2)
592592
}
593593

594-
func TestParseSmbStatusLocksJSON(t *testing.T) {
595-
locks, err := parseSmbStatusLocksAsJSON(outputSmbStatusLocksJSON)
594+
func TestParseSmbStatusLocks(t *testing.T) {
595+
locks, err := parseSmbStatusLockedFiles(smbstatusLocksOutput)
596596
assert.NoError(t, err)
597597
assert.Equal(t, len(locks), 2)
598598
lock1 := locks[0]

0 commit comments

Comments
 (0)