Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Fix linter findings for Windows (part1) #13057

Merged
merged 11 commits into from
Apr 25, 2023
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions plugins/inputs/execd/execd_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package execd

import (
"errors"
"fmt"
"io"
"os"
Expand All @@ -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:
Expand Down
8 changes: 3 additions & 5 deletions plugins/inputs/execd/shim/goshim_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}()
}
12 changes: 6 additions & 6 deletions plugins/inputs/ping/ping_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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+`)
Expand Down
24 changes: 17 additions & 7 deletions plugins/inputs/ping/ping_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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{}{
Expand Down Expand Up @@ -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{}{
Expand Down Expand Up @@ -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())
}
4 changes: 3 additions & 1 deletion plugins/inputs/procstat/win_service_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package procstat

import (
"errors"
"unsafe"

"golang.org/x/sys/windows"
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 1 addition & 4 deletions plugins/inputs/win_eventlog/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"bytes"
"encoding/xml"
"fmt"
"io"
"strings"
"unicode/utf16"
"unicode/utf8"
Expand Down Expand Up @@ -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
}
powersj marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
9 changes: 5 additions & 4 deletions plugins/inputs/win_eventlog/win_eventlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"bytes"
_ "embed"
"encoding/xml"
"errors"
"fmt"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -333,7 +334,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
Expand Down Expand Up @@ -428,7 +429,7 @@ func (w *WinEventLog) renderLocalMessage(event Event, eventHandle EvtHandle) (Ev
if err != nil {
return event, nil
}
defer _EvtClose(publisherHandle)
defer _EvtClose(publisherHandle) //nolint:errcheck // Ignore error returned during Close

// Populating text values
keywords, err := formatEventString(EvtFormatMessageKeyword, eventHandle, publisherHandle)
Expand Down Expand Up @@ -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
}

Expand Down
10 changes: 6 additions & 4 deletions plugins/inputs/win_perf_counters/win_perf_counters.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,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
Expand Down Expand Up @@ -466,7 +467,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
Expand All @@ -482,7 +483,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
Expand Down Expand Up @@ -555,7 +556,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 ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down
Loading