chore: Review of //nolint comments (influxdata#12088)

.github/workflows/golangci-lint.yml

@@ -34,6 +34,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/setup-go@v3
+        with:
+          go-version: 1.19
       - uses: actions/checkout@v3
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3

.golangci.yml

@@ -4,6 +4,7 @@ linters:
     # - telegraflinter
     - bodyclose
     - dogsled
+    - errcheck
     - goprintffuncname
     - gosimple
     - govet
@@ -44,7 +45,7 @@ linters-settings:
       - name: error-return
       - name: error-strings
       - name: errorf
-#      - name: flag-parameter #disable for now
+      #      - name: flag-parameter #disable for now
       - name: function-result-limit
         arguments: [ 3 ]
       - name: identical-branches
@@ -69,7 +70,7 @@ linters-settings:
       - name: unconditional-recursion
       - name: unexported-naming
       - name: unhandled-error
-        arguments: ["outputBuffer.Write", "fmt.Printf", "fmt.Println", "fmt.Print", "fmt.Fprintf", "fmt.Fprint", "fmt.Fprintln"]
+        arguments: [ "outputBuffer.Write", "fmt.Printf", "fmt.Println", "fmt.Print", "fmt.Fprintf", "fmt.Fprint", "fmt.Fprintln" ]
       - name: unnecessary-stmt
       - name: unreachable-code
       # - name: unused-parameter
@@ -129,5 +130,8 @@ issues:
       linters:
         - revive
 
+    - path: cmd/telegraf/(main|printer).go
+      text: "Error return value of `outputBuffer.Write` is not checked"
+
 output:
   format: tab

agent/agent.go

@@ -423,7 +423,9 @@ func (a *Agent) testRunInputs(
 	}
 	wg.Wait()
 
-	internal.SleepContext(ctx, wait)
+	if err := internal.SleepContext(ctx, wait); err != nil {
+		log.Printf("E! [agent] SleepContext finished with: %v", err)
+	}
 
 	log.Printf("D! [agent] Stopping service inputs")
 	stopServiceInputs(unit.inputs)

cmd/telegraf/main.go

@@ -3,7 +3,7 @@ package main
 import (
 	"fmt"
 	"io"
-	"log" //nolint:revive
+	"log"
 	"os"
 	"sort"
 	"strings"
@@ -118,6 +118,7 @@ func runApp(args []string, outputBuffer io.Writer, pprof Server, c TelegrafConfi
 	extraFlags := append(pluginFilterFlags, cliFlags()...)
 
 	// This function is used when Telegraf is run with only flags
+
 	action := func(cCtx *cli.Context) error {
 		err := logger.SetupLogging(logger.LogConfig{})
 		if err != nil {

cmd/telegraf/pprof.go

@@ -2,7 +2,7 @@ package main
 
 import (
 	"fmt"
-	"log" //nolint: revive
+	"log"
 	"net/http"
 	"strings"
 )

cmd/telegraf/telegraf.go

@@ -4,7 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log" //nolint:revive
+	"log"
 	"os"
 	"os/signal"
 	"strings"

config/config_test.go

@@ -953,8 +953,7 @@ type MockupProcessorPluginParser struct {
 func (m *MockupProcessorPluginParser) Start(_ telegraf.Accumulator) error {
 	return nil
 }
-func (m *MockupProcessorPluginParser) Stop() error {
-	return nil
+func (m *MockupProcessorPluginParser) Stop() {
 }
 func (m *MockupProcessorPluginParser) SampleConfig() string {
 	return "Mockup test processor plugin with parser"
@@ -978,8 +977,7 @@ type MockupProcessorPlugin struct{}
 func (m *MockupProcessorPlugin) Start(_ telegraf.Accumulator) error {
 	return nil
 }
-func (m *MockupProcessorPlugin) Stop() error {
-	return nil
+func (m *MockupProcessorPlugin) Stop() {
 }
 func (m *MockupProcessorPlugin) SampleConfig() string {
 	return "Mockup test processor plugin with parser"
@@ -999,8 +997,7 @@ type MockupProcessorPluginParserOnly struct {
 func (m *MockupProcessorPluginParserOnly) Start(_ telegraf.Accumulator) error {
 	return nil
 }
-func (m *MockupProcessorPluginParserOnly) Stop() error {
-	return nil
+func (m *MockupProcessorPluginParserOnly) Stop() {
 }
 func (m *MockupProcessorPluginParserOnly) SampleConfig() string {
 	return "Mockup test processor plugin with parser"
@@ -1023,8 +1020,7 @@ type MockupProcessorPluginParserFunc struct {
 func (m *MockupProcessorPluginParserFunc) Start(_ telegraf.Accumulator) error {
 	return nil
 }
-func (m *MockupProcessorPluginParserFunc) Stop() error {
-	return nil
+func (m *MockupProcessorPluginParserFunc) Stop() {
 }
 func (m *MockupProcessorPluginParserFunc) SampleConfig() string {
 	return "Mockup test processor plugin with parser"

config/deprecation.go

@@ -2,7 +2,7 @@ package config
 
 import (
 	"fmt"
-	"log" //nolint:revive // log is ok here as the logging facility is not set-up yet
+	"log"
 	"reflect"
 	"sort"
 	"strings"
@@ -258,7 +258,7 @@ func (c *Config) PrintDeprecationList(plugins []PluginDeprecationInfo) {
 	for _, plugin := range plugins {
 		switch plugin.LogLevel {
 		case telegraf.Warn, telegraf.Error:
-			_, _ = fmt.Printf(
+			fmt.Printf(
 				"  %-40s %-5s since %-5s removal in %-5s %s\n",
 				plugin.Name, plugin.LogLevel, plugin.info.Since, plugin.info.RemovalIn, plugin.info.Notice,
 			)
@@ -269,7 +269,7 @@ func (c *Config) PrintDeprecationList(plugins []PluginDeprecationInfo) {
 		}
 		sort.Slice(plugin.Options, func(i, j int) bool { return plugin.Options[i].Name < plugin.Options[j].Name })
 		for _, option := range plugin.Options {
-			_, _ = fmt.Printf(
+			fmt.Printf(
 				"  %-40s %-5s since %-5s removal in %-5s %s\n",
 				plugin.Name+"/"+option.Name, option.LogLevel, option.info.Since, option.info.RemovalIn, option.info.Notice,
 			)

internal/process/process_test.go

@@ -74,7 +74,7 @@ func TestMain(m *testing.M) {
 // cleanly.
 func externalProcess() {
 	wait := make(chan int)
-	_, _ = fmt.Fprintln(os.Stdout, "started")
+	fmt.Fprintln(os.Stdout, "started")
 	<-wait
-	os.Exit(2)
+	os.Exit(2) //nolint:revive // os.Exit called intentionally
 }

internal/snmp/translate.go

@@ -7,9 +7,10 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/influxdata/telegraf"
 	"github.com/sleepinggenius2/gosmi"
 	"github.com/sleepinggenius2/gosmi/types"
+
+	"github.com/influxdata/telegraf"
 )
 
 // must init, append path for each directory, load module for every file
@@ -44,10 +45,6 @@ func (*GosmiMibLoader) loadModule(path string) error {
 	return err
 }
 
-func ClearCache() {
-	cache = make(map[string]bool)
-}
-
 // will give all found folders to gosmi and load in all modules found in the folders
 func LoadMibsFromPath(paths []string, log telegraf.Logger, loader MibLoader) error {
 	folders, err := walkPaths(paths, log)
@@ -138,7 +135,7 @@ func walkPaths(paths []string, log telegraf.Logger) ([]string, error) {
 			return nil
 		})
 		if err != nil {
-			return folders, fmt.Errorf("Couldn't walk path %q: %v", mibPath, err)
+			return folders, fmt.Errorf("couldn't walk path %q: %v", mibPath, err)
 		}
 	}
 	return folders, nil
@@ -179,7 +176,7 @@ func TrapLookup(oid string) (e MibEntry, err error) {
 
 // The following is for snmp
 
-func GetIndex(oidNum string, mibPrefix string, node gosmi.SmiNode) (col []string, tagOids map[string]struct{}, err error) {
+func GetIndex(mibPrefix string, node gosmi.SmiNode) (col []string, tagOids map[string]struct{}, err error) {
 	// first attempt to get the table's tags
 	tagOids = map[string]struct{}{}
 
@@ -211,7 +208,7 @@ func SnmpTranslateCall(oid string) (mibName string, oidNum string, oidText strin
 			return oid, oid, oid, oid, gosmi.SmiNode{}, err
 		}
 		if s[1] == "" {
-			return "", oid, oid, oid, gosmi.SmiNode{}, fmt.Errorf("cannot parse %v\n", oid)
+			return "", oid, oid, oid, gosmi.SmiNode{}, fmt.Errorf("cannot parse %v", oid)
 		}
 		// node becomes sysUpTime.0
 		node := s[1]

metric/series_grouper.go

@@ -88,20 +88,20 @@ func groupID(seed maphash.Seed, measurement string, taglist []*telegraf.Tag, tm
 	var mh maphash.Hash
 	mh.SetSeed(seed)
 
-	mh.WriteString(measurement) //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
-	mh.WriteByte(0)             //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
+	mh.WriteString(measurement) //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
+	mh.WriteByte(0)             //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
 
 	for _, tag := range taglist {
-		mh.WriteString(tag.Key)   //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
-		mh.WriteByte(0)           //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
-		mh.WriteString(tag.Value) //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
-		mh.WriteByte(0)           //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
+		mh.WriteString(tag.Key)   //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
+		mh.WriteByte(0)           //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
+		mh.WriteString(tag.Value) //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
+		mh.WriteByte(0)           //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
 	}
-	mh.WriteByte(0) //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
+	mh.WriteByte(0) //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
 
 	var tsBuf [8]byte
 	binary.BigEndian.PutUint64(tsBuf[:], uint64(tm.UnixNano()))
-	mh.Write(tsBuf[:]) //nolint:revive // all Write***() methods for hash in maphash.go returns nil err
+	mh.Write(tsBuf[:]) //nolint:errcheck,revive // all Write***() methods for hash in maphash.go returns nil err
 
 	return mh.Sum64()
 }

models/running_output_test.go

@@ -40,7 +40,7 @@ func BenchmarkRunningOutputAddWrite(b *testing.B) {
 
 	for n := 0; n < b.N; n++ {
 		ro.AddMetric(testutil.TestMetric(101, "metric1"))
-		ro.Write() //nolint: revive // skip checking err for benchmark tests
+		ro.Write() //nolint: errcheck,revive // skip checking err for benchmark tests
 	}
 }
 
@@ -56,7 +56,7 @@ func BenchmarkRunningOutputAddWriteEvery100(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		ro.AddMetric(testutil.TestMetric(101, "metric1"))
 		if n%100 == 0 {
-			ro.Write() //nolint: revive // skip checking err for benchmark tests
+			ro.Write() //nolint: errcheck,revive // skip checking err for benchmark tests
 		}
 	}
 }

models/running_processor_test.go

@@ -5,11 +5,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/models"
 	"github.com/influxdata/telegraf/plugins/processors"
 	"github.com/influxdata/telegraf/testutil"
-	"github.com/stretchr/testify/require"
 )
 
 // MockProcessor is a Processor with an overridable Apply implementation.
@@ -56,7 +57,8 @@ func TestRunningProcessor_Init(t *testing.T) {
 	rp := &models.RunningProcessor{
 		Processor: processors.NewStreamingProcessorFromProcessor(&mock),
 	}
-	rp.Init()
+	err := rp.Init()
+	require.NoError(t, err)
 	require.True(t, mock.HasBeenInit)
 }
 
@@ -188,13 +190,15 @@ func TestRunningProcessor_Apply(t *testing.T) {
 				Processor: tt.args.Processor,
 				Config:    tt.args.Config,
 			}
-			rp.Config.Filter.Compile()
+			err := rp.Config.Filter.Compile()
+			require.NoError(t, err)
 
 			acc := testutil.Accumulator{}
-			err := rp.Start(&acc)
+			err = rp.Start(&acc)
 			require.NoError(t, err)
 			for _, m := range tt.input {
-				rp.Add(m, &acc)
+				err = rp.Add(m, &acc)
+				require.NoError(t, err)
 			}
 			rp.Stop()
 

plugins/common/shim/processor.go

@@ -63,7 +63,9 @@ func (s *Shim) RunProcessor() error {
 			continue
 		}
 
-		s.Processor.Add(m, acc)
+		if err = s.Processor.Add(m, acc); err != nil {
+			fmt.Fprintf(s.stderr, "Failure during processing metric by processor: %v\b", err)
+		}
 	}
 
 	close(s.metricCh)

plugins/common/starlark/field_dict.go

@@ -239,7 +239,9 @@ func asStarlarkValue(value interface{}) (starlark.Value, error) {
 			if err != nil {
 				return starlark.None, err
 			}
-			dict.SetKey(sKey, sValue)
+			if err = dict.SetKey(sKey, sValue); err != nil {
+				return starlark.None, err
+			}
 		}
 		return dict, nil
 	case reflect.Float32, reflect.Float64:

plugins/inputs/dovecot/dovecot_test.go

@@ -13,9 +13,10 @@ import (
 	"time"
 
 	"github.com/docker/go-connections/nat"
-	"github.com/influxdata/telegraf/testutil"
 	"github.com/stretchr/testify/require"
 	"github.com/testcontainers/testcontainers-go/wait"
+
+	"github.com/influxdata/telegraf/testutil"
 )
 
 func TestDovecotIntegration(t *testing.T) {
@@ -53,9 +54,9 @@ func TestDovecotIntegration(t *testing.T) {
 
 	// Test type=global server=unix
 	addr := "/tmp/socket"
-	wait := make(chan int)
+	waitCh := make(chan int)
 	go func() {
-		defer close(wait)
+		defer close(waitCh)
 
 		la, err := net.ResolveUnixAddr("unix", addr)
 		require.NoError(t, err)
@@ -65,7 +66,7 @@ func TestDovecotIntegration(t *testing.T) {
 		defer l.Close()
 		defer os.Remove(addr)
 
-		wait <- 0
+		waitCh <- 0
 		conn, err := l.Accept()
 		require.NoError(t, err)
 		defer conn.Close()
@@ -80,7 +81,7 @@ func TestDovecotIntegration(t *testing.T) {
 	}()
 
 	// Wait for server to start
-	<-wait
+	<-waitCh
 
 	d := &Dovecot{Servers: []string{addr}, Type: "global"}
 	err := d.Gather(&acc)

plugins/inputs/fail2ban/fail2ban_test.go

@@ -126,7 +126,7 @@ func TestHelperProcess(_ *testing.T) {
 			os.Exit(0)
 		}
 	}
-	//nolint:errcheck,revive // Test will fail anyway
+
 	fmt.Fprint(os.Stdout, "invalid argument")
 	//nolint:revive // os.Exit called intentionally
 	os.Exit(1)