Skip to content

Commit

Permalink
chore: Fix linter findings for Windows (part1) (influxdata#13057)
Browse files Browse the repository at this point in the history
  • Loading branch information
zak-pawel committed Apr 25, 2023
1 parent c44c5ed commit 77ee21f
Show file tree
Hide file tree
Showing 16 changed files with 120 additions and 83 deletions.
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
}
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 @@ -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
Expand Down Expand Up @@ -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)
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 @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 ||
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

0 comments on commit 77ee21f

Please sign in to comment.