From 77ee21f8e55432ba22c8638f433476e9dda1af5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20=C5=BBak?= Date: Tue, 25 Apr 2023 15:15:55 +0200 Subject: [PATCH] chore: Fix linter findings for Windows (part1) (#13057) --- .golangci.yml | 2 +- plugins/inputs/execd/execd_windows.go | 9 +++- plugins/inputs/execd/shim/goshim_windows.go | 8 ++-- plugins/inputs/ping/ping_windows.go | 12 ++--- plugins/inputs/ping/ping_windows_test.go | 24 +++++++--- .../inputs/procstat/win_service_windows.go | 4 +- plugins/inputs/win_eventlog/util.go | 5 +- plugins/inputs/win_eventlog/win_eventlog.go | 9 ++-- .../win_perf_counters/win_perf_counters.go | 10 ++-- .../win_perf_counters_integration_test.go | 7 ++- .../win_perf_counters_test.go | 46 +++++++++---------- plugins/inputs/win_services/win_services.go | 22 +++++---- .../win_services_integration_test.go | 19 ++++++-- .../inputs/win_services/win_services_test.go | 19 +++++--- plugins/inputs/win_wmi/win_wmi.go | 5 +- plugins/inputs/win_wmi/win_wmi_test.go | 2 +- 16 files changed, 120 insertions(+), 83 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a56b1368767b6..8cf464fd1d39b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -244,7 +244,7 @@ issues: # revive:var-naming - don't use an underscore in package name # EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok - - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked + - Error return value of .((os\.)?std(out|err)\..*|.*Close.*|.*Flush|.*Disconnect|.*Clear|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked # EXC0013 revive: Annoying issue about not having a comment. The rare codebase has such comments - package comment should be of the form "(.+)... # EXC0015 revive: Annoying issue about not having a comment. The rare codebase has such comments diff --git a/plugins/inputs/execd/execd_windows.go b/plugins/inputs/execd/execd_windows.go index 3396a6a3f5e9f..fc8d402f2ceb2 100644 --- a/plugins/inputs/execd/execd_windows.go +++ b/plugins/inputs/execd/execd_windows.go @@ -3,6 +3,7 @@ package execd import ( + "errors" "fmt" "io" "os" @@ -19,10 +20,14 @@ func (e *Execd) Gather(acc telegraf.Accumulator) error { switch e.Signal { case "STDIN": if osStdin, ok := e.process.Stdin.(*os.File); ok { - osStdin.SetWriteDeadline(time.Now().Add(1 * time.Second)) + if err := osStdin.SetWriteDeadline(time.Now().Add(1 * time.Second)); err != nil { + if !errors.Is(err, os.ErrNoDeadline) { + return fmt.Errorf("setting write deadline failed: %w", err) + } + } } if _, err := io.WriteString(e.process.Stdin, "\n"); err != nil { - return fmt.Errorf("Error writing to stdin: %s", err) + return fmt.Errorf("error writing to stdin: %w", err) } case "none": default: diff --git a/plugins/inputs/execd/shim/goshim_windows.go b/plugins/inputs/execd/shim/goshim_windows.go index b2d03d41bc9cc..ae237891fcf0b 100644 --- a/plugins/inputs/execd/shim/goshim_windows.go +++ b/plugins/inputs/execd/shim/goshim_windows.go @@ -13,10 +13,8 @@ func listenForCollectMetricsSignals(ctx context.Context, collectMetricsPrompt ch signal.Notify(collectMetricsPrompt, syscall.SIGHUP) go func() { - select { - case <-ctx.Done(): - // context done. stop to signals to avoid pushing messages to a closed channel - signal.Stop(collectMetricsPrompt) - } + <-ctx.Done() + // context done. stop to signals to avoid pushing messages to a closed channel + signal.Stop(collectMetricsPrompt) }() } diff --git a/plugins/inputs/ping/ping_windows.go b/plugins/inputs/ping/ping_windows.go index 7f411ba97cdd2..ed400c33129e2 100644 --- a/plugins/inputs/ping/ping_windows.go +++ b/plugins/inputs/ping/ping_windows.go @@ -12,11 +12,11 @@ import ( "github.com/influxdata/telegraf" ) -func (p *Ping) pingToURL(u string, acc telegraf.Accumulator) { - tags := map[string]string{"url": u} +func (p *Ping) pingToURL(host string, acc telegraf.Accumulator) { + tags := map[string]string{"url": host} fields := map[string]interface{}{"result_code": 0} - args := p.args(u) + args := p.args(host) totalTimeout := 60.0 if len(p.Arguments) == 0 { totalTimeout = p.timeout() * float64(p.Count) @@ -34,9 +34,9 @@ func (p *Ping) pingToURL(u string, acc telegraf.Accumulator) { if err != nil { // fatal error if pendingError != nil { - acc.AddError(fmt.Errorf("%s: %s", pendingError, u)) + acc.AddError(fmt.Errorf("%s: %w", host, pendingError)) } else { - acc.AddError(fmt.Errorf("%s: %s", err, u)) + acc.AddError(fmt.Errorf("%s: %w", host, err)) } fields["result_code"] = 2 @@ -88,7 +88,7 @@ func (p *Ping) args(url string) []string { func processPingOutput(out string) (int, int, int, int, int, int, error) { // So find a line contain 3 numbers except reply lines var stats, aproxs []string = nil, nil - err := errors.New("Fatal error processing ping output") + err := errors.New("fatal error processing ping output") stat := regexp.MustCompile(`=\W*(\d+)\D*=\W*(\d+)\D*=\W*(\d+)`) aprox := regexp.MustCompile(`=\W*(\d+)\D*ms\D*=\W*(\d+)\D*ms\D*=\W*(\d+)\D*ms`) tttLine := regexp.MustCompile(`TTL=\d+`) diff --git a/plugins/inputs/ping/ping_windows_test.go b/plugins/inputs/ping/ping_windows_test.go index 301580395ae35..408321e3b3530 100644 --- a/plugins/inputs/ping/ping_windows_test.go +++ b/plugins/inputs/ping/ping_windows_test.go @@ -73,7 +73,7 @@ func TestPingGather(t *testing.T) { pingHost: mockHostPinger, } - acc.GatherError(p.Gather) + require.NoError(t, acc.GatherError(p.Gather)) tags := map[string]string{"url": "www.google.com"} fields := map[string]interface{}{ "packets_transmitted": 4, @@ -118,7 +118,9 @@ func TestBadPingGather(t *testing.T) { pingHost: mockErrorHostPinger, } - acc.GatherError(p.Gather) + err := acc.GatherError(p.Gather) + require.NoError(t, err) + tags := map[string]string{"url": "www.amazon.com"} fields := map[string]interface{}{ "packets_transmitted": 4, @@ -176,7 +178,9 @@ func TestLossyPingGather(t *testing.T) { pingHost: mockLossyHostPinger, } - acc.GatherError(p.Gather) + err := acc.GatherError(p.Gather) + require.NoError(t, err) + tags := map[string]string{"url": "www.google.com"} fields := map[string]interface{}{ "packets_transmitted": 9, @@ -237,7 +241,9 @@ func TestFatalPingGather(t *testing.T) { pingHost: mockFatalHostPinger, } - acc.GatherError(p.Gather) + err := acc.GatherError(p.Gather) + require.Error(t, err) + require.True(t, acc.HasFloatField("ping", "errors"), "Fatal ping should have packet measurements") require.False(t, acc.HasInt64Field("ping", "packets_transmitted"), @@ -283,7 +289,8 @@ func TestUnreachablePingGather(t *testing.T) { pingHost: mockUnreachableHostPinger, } - acc.GatherError(p.Gather) + err := acc.GatherError(p.Gather) + require.NoError(t, err) tags := map[string]string{"url": "www.google.com"} fields := map[string]interface{}{ @@ -331,7 +338,8 @@ func TestTTLExpiredPingGather(t *testing.T) { pingHost: mockTTLExpiredPinger, } - acc.GatherError(p.Gather) + err := acc.GatherError(p.Gather) + require.NoError(t, err) tags := map[string]string{"url": "www.google.com"} fields := map[string]interface{}{ @@ -365,5 +373,7 @@ func TestPingBinary(t *testing.T) { return "", nil }, } - acc.GatherError(p.Gather) + err := acc.GatherError(p.Gather) + require.Error(t, err) + require.EqualValues(t, "www.google.com: fatal error processing ping output", err.Error()) } diff --git a/plugins/inputs/procstat/win_service_windows.go b/plugins/inputs/procstat/win_service_windows.go index 0ce833de9edf0..a0426541df240 100644 --- a/plugins/inputs/procstat/win_service_windows.go +++ b/plugins/inputs/procstat/win_service_windows.go @@ -3,6 +3,7 @@ package procstat import ( + "errors" "unsafe" "golang.org/x/sys/windows" @@ -34,7 +35,8 @@ func queryPidWithWinServiceName(winServiceName string) (uint32, error) { var bytesNeeded uint32 var buf []byte - if err := windows.QueryServiceStatusEx(srv.Handle, windows.SC_STATUS_PROCESS_INFO, nil, 0, &bytesNeeded); err != windows.ERROR_INSUFFICIENT_BUFFER { + err = windows.QueryServiceStatusEx(srv.Handle, windows.SC_STATUS_PROCESS_INFO, nil, 0, &bytesNeeded) + if !errors.Is(err, windows.ERROR_INSUFFICIENT_BUFFER) { return 0, err } diff --git a/plugins/inputs/win_eventlog/util.go b/plugins/inputs/win_eventlog/util.go index 8be153e8c810a..4f6477575b7b5 100644 --- a/plugins/inputs/win_eventlog/util.go +++ b/plugins/inputs/win_eventlog/util.go @@ -9,7 +9,6 @@ import ( "bytes" "encoding/xml" "fmt" - "io" "strings" "unicode/utf16" "unicode/utf8" @@ -97,12 +96,10 @@ func UnrollXMLFields(data []byte, fieldsUsage map[string]int, separator string) for { var node xmlnode err := dec.Decode(&node) - if err == io.EOF { - break - } if err != nil { break } + var parents []string walkXML([]xmlnode{node}, parents, separator, func(node xmlnode, parents []string, separator string) bool { innerText := strings.TrimSpace(node.Text) diff --git a/plugins/inputs/win_eventlog/win_eventlog.go b/plugins/inputs/win_eventlog/win_eventlog.go index 895fb85a3359b..e6bed85089821 100644 --- a/plugins/inputs/win_eventlog/win_eventlog.go +++ b/plugins/inputs/win_eventlog/win_eventlog.go @@ -11,6 +11,7 @@ import ( "bytes" _ "embed" "encoding/xml" + "errors" "fmt" "path/filepath" "reflect" @@ -120,7 +121,7 @@ func (w *WinEventLog) Gather(acc telegraf.Accumulator) error { for { events, err := w.fetchEvents(w.subscription) if err != nil { - if err == ERROR_NO_MORE_ITEMS { + if errors.Is(err, ERROR_NO_MORE_ITEMS) { break } w.Log.Errorf("Error getting events: %v", err) @@ -334,7 +335,7 @@ func (w *WinEventLog) fetchEventHandles(subsHandle EvtHandle) ([]EvtHandle, erro err := _EvtNext(subsHandle, eventsNumber, &eventHandles[0], 0, 0, &evtReturned) if err != nil { - if err == ERROR_INVALID_OPERATION && evtReturned == 0 { + if errors.Is(err, ERROR_INVALID_OPERATION) && evtReturned == 0 { return nil, ERROR_NO_MORE_ITEMS } return nil, err @@ -428,7 +429,7 @@ func (w *WinEventLog) renderLocalMessage(event Event, eventHandle EvtHandle) (Ev if err != nil { return event, nil //nolint:nilerr // We can return event without most values } - defer _EvtClose(publisherHandle) + defer _EvtClose(publisherHandle) //nolint:errcheck // Ignore error returned during Close // Populating text values keywords, err := formatEventString(EvtFormatMessageKeyword, eventHandle, publisherHandle) @@ -493,7 +494,7 @@ func formatEventString( var bufferUsed uint32 err := _EvtFormatMessage(publisherHandle, eventHandle, 0, 0, 0, messageFlag, 0, nil, &bufferUsed) - if err != nil && err != ERROR_INSUFFICIENT_BUFFER { + if err != nil && !errors.Is(err, ERROR_INSUFFICIENT_BUFFER) { return "", err } diff --git a/plugins/inputs/win_perf_counters/win_perf_counters.go b/plugins/inputs/win_perf_counters/win_perf_counters.go index 258033a4a5e6d..1e1fb536e7ba7 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters.go @@ -393,7 +393,8 @@ func (m *WinPerfCounters) ParseConfig() error { } func (m *WinPerfCounters) checkError(err error) error { - if pdhErr, ok := err.(*PdhError); ok { + var pdhErr *PdhError + if errors.As(err, &pdhErr) { for _, ignoredErrors := range m.IgnoredErrors { if PDHErrors[pdhErr.ErrorCode] == ignoredErrors { return nil @@ -477,7 +478,7 @@ func (m *WinPerfCounters) gatherComputerCounters(hostCounterInfo *hostCountersIn if err != nil { //ignore invalid data as some counters from process instances returns this sometimes if !isKnownCounterDataError(err) { - return fmt.Errorf("error while getting value for counter %s: %v", metric.counterPath, err) + return fmt.Errorf("error while getting value for counter %q: %w", metric.counterPath, err) } m.Log.Warnf("error while getting value for counter %q, instance: %s, will skip metric: %v", metric.counterPath, metric.instance, err) continue @@ -493,7 +494,7 @@ func (m *WinPerfCounters) gatherComputerCounters(hostCounterInfo *hostCountersIn if err != nil { //ignore invalid data as some counters from process instances returns this sometimes if !isKnownCounterDataError(err) { - return fmt.Errorf("error while getting value for counter %s: %v", metric.counterPath, err) + return fmt.Errorf("error while getting value for counter %q: %w", metric.counterPath, err) } m.Log.Warnf("error while getting value for counter %q, instance: %s, will skip metric: %v", metric.counterPath, metric.instance, err) continue @@ -566,7 +567,8 @@ func addCounterMeasurement(metric *counter, instanceName string, value interface } func isKnownCounterDataError(err error) bool { - if pdhErr, ok := err.(*PdhError); ok && (pdhErr.ErrorCode == PDH_INVALID_DATA || + var pdhErr *PdhError + if errors.As(err, &pdhErr) && (pdhErr.ErrorCode == PDH_INVALID_DATA || pdhErr.ErrorCode == PDH_CALC_NEGATIVE_DENOMINATOR || pdhErr.ErrorCode == PDH_CALC_NEGATIVE_VALUE || pdhErr.ErrorCode == PDH_CSTATUS_INVALID_DATA || diff --git a/plugins/inputs/win_perf_counters/win_perf_counters_integration_test.go b/plugins/inputs/win_perf_counters/win_perf_counters_integration_test.go index f2adcb475ae39..610cbce8337f3 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters_integration_test.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters_integration_test.go @@ -3,13 +3,15 @@ package win_perf_counters import ( + "errors" "fmt" "strings" "testing" "time" - "github.com/influxdata/telegraf/testutil" "github.com/stretchr/testify/require" + + "github.com/influxdata/telegraf/testutil" ) func TestWinPerformanceQueryImplIntegration(t *testing.T) { @@ -107,7 +109,8 @@ func TestWinPerformanceQueryImplIntegration(t *testing.T) { require.NoError(t, err) farr, err := query.GetFormattedCounterArrayDouble(hCounter) - if phderr, ok := err.(*PdhError); ok && phderr.ErrorCode != PDH_INVALID_DATA && phderr.ErrorCode != PDH_CALC_NEGATIVE_VALUE { + var phdErr *PdhError + if errors.As(err, &phdErr) && phdErr.ErrorCode != PDH_INVALID_DATA && phdErr.ErrorCode != PDH_CALC_NEGATIVE_VALUE { time.Sleep(time.Second) farr, err = query.GetFormattedCounterArrayDouble(hCounter) } diff --git a/plugins/inputs/win_perf_counters/win_perf_counters_test.go b/plugins/inputs/win_perf_counters/win_perf_counters_test.go index 4cbf2428a00e9..fc1143340de1f 100644 --- a/plugins/inputs/win_perf_counters/win_perf_counters_test.go +++ b/plugins/inputs/win_perf_counters/win_perf_counters_test.go @@ -31,7 +31,7 @@ type FakePerformanceQuery struct { var MetricTime = time.Date(2018, 5, 28, 12, 0, 0, 0, time.UTC) func (m *testCounter) ToCounterValue(raw bool) *CounterValue { - _, _, inst, _, _ := extractCounterInfoFromCounterPath(m.path) + _, _, inst, _, _ := extractCounterInfoFromCounterPath(m.path) //nolint:dogsled // only instance is needed for this helper function in tests if inst == "" { inst = "--" } @@ -58,7 +58,7 @@ func (m *FakePerformanceQuery) Open() error { func (m *FakePerformanceQuery) Close() error { if !m.openCalled { - return errors.New("CloSe: uninitialized query") + return errors.New("in Close: uninitialized query") } m.openCalled = false return nil @@ -66,23 +66,23 @@ func (m *FakePerformanceQuery) Close() error { func (m *FakePerformanceQuery) AddCounterToQuery(counterPath string) (PDH_HCOUNTER, error) { if !m.openCalled { - return 0, errors.New("AddCounterToQuery: uninitialized query") + return 0, errors.New("in AddCounterToQuery: uninitialized query") } if c, ok := m.counters[counterPath]; ok { return c.handle, nil } else { - return 0, errors.New(fmt.Sprintf("AddCounterToQuery: invalid counter path: %s", counterPath)) + return 0, fmt.Errorf("in AddCounterToQuery: invalid counter path: %q", counterPath) } } func (m *FakePerformanceQuery) AddEnglishCounterToQuery(counterPath string) (PDH_HCOUNTER, error) { if !m.openCalled { - return 0, errors.New("AddEnglishCounterToQuery: uninitialized query") + return 0, errors.New("in AddEnglishCounterToQuery: uninitialized query") } if c, ok := m.counters[counterPath]; ok { return c.handle, nil } else { - return 0, fmt.Errorf("AddEnglishCounterToQuery: invalid counter path: %s", counterPath) + return 0, fmt.Errorf("in AddEnglishCounterToQuery: invalid counter path: %q", counterPath) } } @@ -92,20 +92,20 @@ func (m *FakePerformanceQuery) GetCounterPath(counterHandle PDH_HCOUNTER) (strin return counter.path, nil } } - return "", fmt.Errorf("GetCounterPath: invalid handle: %d", counterHandle) + return "", fmt.Errorf("in GetCounterPath: invalid handle: %q", counterHandle) } func (m *FakePerformanceQuery) ExpandWildCardPath(counterPath string) ([]string, error) { if e, ok := m.expandPaths[counterPath]; ok { return e, nil } else { - return []string{}, fmt.Errorf("ExpandWildCardPath: invalid counter path: %s", counterPath) + return []string{}, fmt.Errorf("in ExpandWildCardPath: invalid counter path: %q", counterPath) } } func (m *FakePerformanceQuery) GetFormattedCounterValueDouble(counterHandle PDH_HCOUNTER) (float64, error) { if !m.openCalled { - return 0, errors.New("GetFormattedCounterValueDouble: uninitialized query") + return 0, errors.New("in GetFormattedCounterValueDouble: uninitialized query") } for _, counter := range m.counters { if counter.handle == counterHandle { @@ -115,12 +115,12 @@ func (m *FakePerformanceQuery) GetFormattedCounterValueDouble(counterHandle PDH_ return counter.value, nil } } - return 0, fmt.Errorf("GetFormattedCounterValueDouble: invalid handle: %d", counterHandle) + return 0, fmt.Errorf("in GetFormattedCounterValueDouble: invalid handle: %q", counterHandle) } func (m *FakePerformanceQuery) GetRawCounterValue(counterHandle PDH_HCOUNTER) (int64, error) { if !m.openCalled { - return 0, errors.New("GetRawCounterValue: uninitialised query") + return 0, errors.New("in GetRawCounterValue: uninitialised query") } for _, counter := range m.counters { if counter.handle == counterHandle { @@ -130,7 +130,7 @@ func (m *FakePerformanceQuery) GetRawCounterValue(counterHandle PDH_HCOUNTER) (i return int64(counter.value), nil } } - return 0, fmt.Errorf("GetRawCounterValue: invalid handle: %d", counterHandle) + return 0, fmt.Errorf("in GetRawCounterValue: invalid handle: %q", counterHandle) } func (m *FakePerformanceQuery) findCounterByPath(counterPath string) *testCounter { @@ -153,7 +153,7 @@ func (m *FakePerformanceQuery) findCounterByHandle(counterHandle PDH_HCOUNTER) * func (m *FakePerformanceQuery) GetFormattedCounterArrayDouble(hCounter PDH_HCOUNTER) ([]CounterValue, error) { if !m.openCalled { - return nil, errors.New("GetFormattedCounterArrayDouble: uninitialized query") + return nil, errors.New("in GetFormattedCounterArrayDouble: uninitialized query") } for _, c := range m.counters { if c.handle == hCounter { @@ -167,21 +167,21 @@ func (m *FakePerformanceQuery) GetFormattedCounterArrayDouble(hCounter PDH_HCOUN } counters = append(counters, *counter.ToCounterValue(false)) } else { - return nil, fmt.Errorf("GetFormattedCounterArrayDouble: invalid counter : %s", p) + return nil, fmt.Errorf("in GetFormattedCounterArrayDouble: invalid counter: %q", p) } } return counters, nil } else { - return nil, fmt.Errorf("GetFormattedCounterArrayDouble: invalid counter : %d", hCounter) + return nil, fmt.Errorf("in GetFormattedCounterArrayDouble: invalid counter: %q", hCounter) } } } - return nil, fmt.Errorf("GetFormattedCounterArrayDouble: invalid counter : %d, no paths found", hCounter) + return nil, fmt.Errorf("in GetFormattedCounterArrayDouble: invalid counter: %q, no paths found", hCounter) } func (m *FakePerformanceQuery) GetRawCounterArray(hCounter PDH_HCOUNTER) ([]CounterValue, error) { if !m.openCalled { - return nil, errors.New("GetRawCounterArray: uninitialised query") + return nil, errors.New("in GetRawCounterArray: uninitialised query") } for _, c := range m.counters { if c.handle == hCounter { @@ -195,28 +195,28 @@ func (m *FakePerformanceQuery) GetRawCounterArray(hCounter PDH_HCOUNTER) ([]Coun } counters = append(counters, *counter.ToCounterValue(true)) } else { - return nil, fmt.Errorf("GetRawCounterArray: invalid counter : %s", p) + return nil, fmt.Errorf("in GetRawCounterArray: invalid counter: %q", p) } } return counters, nil } else { - return nil, fmt.Errorf("GetRawCounterArray: invalid counter : %d", hCounter) + return nil, fmt.Errorf("in GetRawCounterArray: invalid counter: %q", hCounter) } } } - return nil, fmt.Errorf("GetRawCounterArray: invalid counter : %d, no paths found", hCounter) + return nil, fmt.Errorf("in GetRawCounterArray: invalid counter: %q, no paths found", hCounter) } func (m *FakePerformanceQuery) CollectData() error { if !m.openCalled { - return errors.New("CollectData: uninitialized query") + return errors.New("in CollectData: uninitialized query") } return nil } func (m *FakePerformanceQuery) CollectDataWithTime() (time.Time, error) { if !m.openCalled { - return time.Now(), errors.New("CollectData: uninitialized query") + return time.Now(), errors.New("in CollectDataWithTime: uninitialized query") } return MetricTime, nil } @@ -233,7 +233,7 @@ func (m FakePerformanceQueryCreator) NewPerformanceQuery(computer string) Perfor var ret PerformanceQuery var ok bool if ret, ok = m.fakeQueries[computer]; !ok { - panic(fmt.Errorf("query for %s not found", computer)) + panic(fmt.Errorf("query for %q not found", computer)) } return ret } diff --git a/plugins/inputs/win_services/win_services.go b/plugins/inputs/win_services/win_services.go index e2060d10c9495..6fa2313516c41 100644 --- a/plugins/inputs/win_services/win_services.go +++ b/plugins/inputs/win_services/win_services.go @@ -5,8 +5,9 @@ package win_services import ( _ "embed" + "errors" "fmt" - "os" + "io/fs" "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/mgr" @@ -19,19 +20,20 @@ import ( //go:embed sample.conf var sampleConfig string -type ServiceErr struct { +type ServiceError struct { Message string Service string Err error } -func (e *ServiceErr) Error() string { +func (e *ServiceError) Error() string { return fmt.Sprintf("%s: %q: %v", e.Message, e.Service, e.Err) } func IsPermission(err error) bool { - if err, ok := err.(*ServiceErr); ok { - return os.IsPermission(err.Err) + var serviceErr *ServiceError + if errors.As(err, &serviceErr) { + return errors.Is(serviceErr, fs.ErrPermission) } return false } @@ -119,7 +121,7 @@ func (m *WinServices) Init() error { func (m *WinServices) Gather(acc telegraf.Accumulator) error { scmgr, err := m.mgrProvider.Connect() if err != nil { - return fmt.Errorf("Could not open service manager: %s", err) + return fmt.Errorf("could not open service manager: %w", err) } defer scmgr.Disconnect() @@ -161,7 +163,7 @@ func (m *WinServices) Gather(acc telegraf.Accumulator) error { func (m *WinServices) listServices(scmgr WinServiceManager) ([]string, error) { names, err := scmgr.ListServices() if err != nil { - return nil, fmt.Errorf("Could not list services: %s", err) + return nil, fmt.Errorf("could not list services: %w", err) } var services []string @@ -178,7 +180,7 @@ func (m *WinServices) listServices(scmgr WinServiceManager) ([]string, error) { func collectServiceInfo(scmgr WinServiceManager, serviceName string) (*ServiceInfo, error) { srv, err := scmgr.OpenService(serviceName) if err != nil { - return nil, &ServiceErr{ + return nil, &ServiceError{ Message: "could not open service", Service: serviceName, Err: err, @@ -188,7 +190,7 @@ func collectServiceInfo(scmgr WinServiceManager, serviceName string) (*ServiceIn srvStatus, err := srv.Query() if err != nil { - return nil, &ServiceErr{ + return nil, &ServiceError{ Message: "could not query service", Service: serviceName, Err: err, @@ -197,7 +199,7 @@ func collectServiceInfo(scmgr WinServiceManager, serviceName string) (*ServiceIn srvCfg, err := srv.Config() if err != nil { - return nil, &ServiceErr{ + return nil, &ServiceError{ Message: "could not get config of service", Service: serviceName, Err: err, diff --git a/plugins/inputs/win_services/win_services_integration_test.go b/plugins/inputs/win_services/win_services_integration_test.go index 4a002ce5ac04c..871ae66dd8244 100644 --- a/plugins/inputs/win_services/win_services_integration_test.go +++ b/plugins/inputs/win_services/win_services_integration_test.go @@ -20,12 +20,16 @@ func TestListIntegration(t *testing.T) { provider := &MgProvider{} scmgr, err := provider.Connect() require.NoError(t, err) - defer scmgr.Disconnect() + defer func() { + err := scmgr.Disconnect() + require.NoError(t, err) + }() winServices := &WinServices{ ServiceNames: KnownServices, } - winServices.Init() + + require.NoError(t, winServices.Init()) services, err := winServices.listServices(scmgr) require.NoError(t, err) require.Len(t, services, 2, "Different number of services") @@ -40,12 +44,16 @@ func TestEmptyListIntegration(t *testing.T) { provider := &MgProvider{} scmgr, err := provider.Connect() require.NoError(t, err) - defer scmgr.Disconnect() + defer func() { + err := scmgr.Disconnect() + require.NoError(t, err) + }() winServices := &WinServices{ ServiceNames: []string{}, } - winServices.Init() + + require.NoError(t, winServices.Init()) services, err := winServices.listServices(scmgr) require.NoError(t, err) require.Condition(t, func() bool { return len(services) > 20 }, "Too few service") @@ -60,7 +68,8 @@ func TestGatherErrorsIntegration(t *testing.T) { ServiceNames: InvalidServices, mgrProvider: &MgProvider{}, } - ws.Init() + + require.NoError(t, ws.Init()) require.Len(t, ws.ServiceNames, 3, "Different number of services") var acc testutil.Accumulator require.NoError(t, ws.Gather(&acc)) diff --git a/plugins/inputs/win_services/win_services_test.go b/plugins/inputs/win_services/win_services_test.go index ebe485d4bacfe..3c8d1e33fce93 100644 --- a/plugins/inputs/win_services/win_services_test.go +++ b/plugins/inputs/win_services/win_services_test.go @@ -155,9 +155,10 @@ func TestMgrErrors(t *testing.T) { ServiceNames: []string{"Fake service 1"}, mgrProvider: &FakeMgProvider{testErrors[3]}, } - winServices.Init() - var acc3 testutil.Accumulator + err = winServices.Init() + require.NoError(t, err) + var acc3 testutil.Accumulator buf := &bytes.Buffer{} log.SetOutput(buf) require.NoError(t, winServices.Gather(&acc3)) @@ -170,9 +171,10 @@ func TestServiceErrors(t *testing.T) { Log: testutil.Logger{}, mgrProvider: &FakeMgProvider{testErrors[2]}, } - winServices.Init() - var acc1 testutil.Accumulator + err := winServices.Init() + require.NoError(t, err) + var acc1 testutil.Accumulator buf := &bytes.Buffer{} log.SetOutput(buf) require.NoError(t, winServices.Gather(&acc1)) @@ -198,7 +200,10 @@ func TestGatherContainsTag(t *testing.T) { ServiceNames: []string{"Service*"}, mgrProvider: &FakeMgProvider{testSimpleData[0]}, } - winServices.Init() + + err := winServices.Init() + require.NoError(t, err) + var acc1 testutil.Accumulator require.NoError(t, winServices.Gather(&acc1)) require.Len(t, acc1.Errors, 0, "There should be no errors after gather") @@ -220,7 +225,9 @@ func TestExcludingNamesTag(t *testing.T) { ServiceNamesExcluded: []string{"Service*"}, mgrProvider: &FakeMgProvider{testSimpleData[0]}, } - winServices.Init() + err := winServices.Init() + require.NoError(t, err) + var acc1 testutil.Accumulator require.NoError(t, winServices.Gather(&acc1)) diff --git a/plugins/inputs/win_wmi/win_wmi.go b/plugins/inputs/win_wmi/win_wmi.go index 1fe5d42c219c1..821beccf0b93f 100644 --- a/plugins/inputs/win_wmi/win_wmi.go +++ b/plugins/inputs/win_wmi/win_wmi.go @@ -5,6 +5,7 @@ package win_wmi import ( _ "embed" + "errors" "fmt" "runtime" "strings" @@ -110,8 +111,8 @@ func (q *Query) doQuery(acc telegraf.Accumulator) error { // init COM if err := ole.CoInitializeEx(0, ole.COINIT_MULTITHREADED); err != nil { - oleCode := err.(*ole.OleError).Code() - if oleCode != ole.S_OK && oleCode != sFalse { + var oleCode *ole.OleError + if errors.As(err, &oleCode) && oleCode.Code() != ole.S_OK && oleCode.Code() != sFalse { return err } } diff --git a/plugins/inputs/win_wmi/win_wmi_test.go b/plugins/inputs/win_wmi/win_wmi_test.go index 2eebb4db6e60d..5ebf29954e441 100644 --- a/plugins/inputs/win_wmi/win_wmi_test.go +++ b/plugins/inputs/win_wmi/win_wmi_test.go @@ -68,7 +68,7 @@ func TestWmi_Gather(t *testing.T) { var logger = new(testutil.Logger) var acc = new(testutil.Accumulator) plugin := Wmi{Queries: []Query{testQuery}, Log: logger} - plugin.Init() + require.NoError(t, plugin.Init()) require.NoError(t, plugin.Gather(acc)) // no errors in accumulator require.Empty(t, acc.Errors)