Fix the linting action to work appropriately in github actions. (#2976)

* Work on the linting action

* Update golangci config file for renamed skip-dirs

* move timeout change into golangci config file

* Start linting

* Disable linting of const in tests

* Remove errgroup as recommended by depguard

* Fix missed refactor of deprecated type

* Fix test of refactored code

* More linting

* additional linting

* Appease linter

* linting linting

* Fix tests and more linting

* ensure that remotecfg ticket stops even if registerCollector fails

* remove errGroup/run and just use wg. Add nolints, one last refactor

* Fix nolint

* Syntax linting

* syntax linting

* ensure the remotecfg ticker stops and doesn't get stopped twice on a re-evaluation

* Use concurrent safe multierr append

* Re-add alloylint make target

* Fix accidental propagation of context canceled error

* Revert "Use concurrent safe multierr append"

This reverts commit 7ce27b5.

* Add errors together concurrent-safe

* move experimental lint -> lint

* Refactor ticker handling in remotecfg

* Remove unnecessary mutex locks

Author: dehaansa

.github/workflows/test_pr.yml

@@ -5,27 +5,24 @@ jobs:
   lint:
     name: Lint
     runs-on: ubuntu-latest
-    container:
-      # The build image contains golangci-lint.
-      image: grafana/alloy-build-image:v0.1.8
     steps:
     - name: Checkout code
       uses: actions/checkout@v4
-
-    - name: Set ownership
-      # https://github.com/actions/runner/issues/2033#issuecomment-1204205989
-      run: |
-          # this is to fix GIT not liking owner of the checkout dir
-          chown -R $(id -u):$(id -g) $PWD
-
     - name: Set up Go 1.23
       uses: actions/setup-go@v5
       with:
         go-version-file: go.mod
         cache: false
-
-    - run: apt-get update -y && apt-get install -y libsystemd-dev
-    - run: make lint
+    - name: golangci-lint
+      uses: golangci/golangci-lint-action@v6
+      with:
+        version: v1.64.7
+    - name: golangci-lint-syntax
+      uses: golangci/golangci-lint-action@v6
+      with:
+        version: v1.64.7
+        working-directory: ./syntax/
+    - run: make run-alloylint
 
   test_linux:
     name: Test Linux

.golangci.yml

@@ -1,10 +1,7 @@
 # Full list of configuration options: https://golangci-lint.run/usage/configuration/
 
 run:
-  timeout: 5m
-  skip-dirs:
-    - component/pyroscope/scrape/internal/fastdelta
-    - component/pyroscope/scrape/internal/pproflite
+  timeout: 10m
 
 output:
   sort-results: true
@@ -26,13 +23,20 @@ linters:
     - typecheck   # Ensure code typechecks
     - depguard    # Allow/denylist specific imports
     - makezero    # Detect misuse of make with non-zero length and append
-    - tenv        # Use testing.(*T).Setenv instead of os.Setenv
+    - usetesting  # Use testing.(*T).Setenv instead of os.Setenv
     - whitespace  # Report unnecessary blank lines
 
 issues:
+  max-same-issues: 0
+  max-issues-per-linter: 0
+  
   # We want to use our own exclusion rules and ignore all the defaults.
   exclude-use-default: false
 
+  exclude-dirs:
+    - component/pyroscope/scrape/internal/fastdelta
+    - component/pyroscope/scrape/internal/pproflite
+
   exclude-rules:
     # It's fine if tests ignore errors.
     - path: _test.go
@@ -83,6 +87,8 @@ linters-settings:
     # at a glance.
     multi-if: true
     multi-func: true
+  goconst:
+    ignore-tests: true
 
   revive:
     rules:

Makefile

@@ -136,6 +136,10 @@ lint: alloylint
 	find . -name go.mod -execdir golangci-lint run -v --timeout=10m \;
 	$(ALLOYLINT_BINARY) ./...
 
+.PHONY: run-alloylint
+run-alloylint: alloylint
+	$(ALLOYLINT_BINARY) ./...
+
 .PHONY: test
 # We have to run test twice: once for all packages with -race and then once
 # more without -race for packages that have known race detection issues. The

internal/alloycli/automemlimit_cgroups_unsupported.go

@@ -13,13 +13,16 @@ import (
 	"github.com/grafana/alloy/internal/runtime/logging"
 )
 
-func applyAutoMemLimit(l *logging.Logger) {
+func applyAutoMemLimit(l *logging.Logger) error {
 	// For non-linux builds without cgroups, memlimit will always report an error.
 	// However, if the system experiment is requested, we can use the system memory limit provider.
 	// This logic is similar to https://github.com/KimMachineGun/automemlimit/blob/main/memlimit/experiment.go
 	if v, ok := os.LookupEnv("AUTOMEMLIMIT_EXPERIMENT"); ok {
 		if slices.Contains(strings.Split(v, ","), "system") {
-			memlimit.SetGoMemLimitWithOpts(memlimit.WithProvider(memlimit.FromSystem), memlimit.WithLogger(slog.New(l.Handler())))
+			_, err := memlimit.SetGoMemLimitWithOpts(memlimit.WithProvider(memlimit.FromSystem), memlimit.WithLogger(slog.New(l.Handler())))
+			return err
 		}
 	}
+
+	return nil
 }

internal/alloycli/automemlimit_linux.go

@@ -10,6 +10,7 @@ import (
 	"github.com/grafana/alloy/internal/runtime/logging"
 )
 
-func applyAutoMemLimit(l *logging.Logger) {
-	memlimit.SetGoMemLimitWithOpts(memlimit.WithLogger(slog.New(l.Handler())))
+func applyAutoMemLimit(l *logging.Logger) error {
+	_, err := memlimit.SetGoMemLimitWithOpts(memlimit.WithLogger(slog.New(l.Handler())))
+	return err
 }

internal/alloycli/cmd_run.go

@@ -235,7 +235,10 @@ func (fr *alloyRun) Run(cmd *cobra.Command, configPath string) error {
 
 	// Set the memory limit, this will honor GOMEMLIMIT if set
 	// If there is a cgroup on linux it will use that
-	applyAutoMemLimit(l)
+	err = applyAutoMemLimit(l)
+	if err != nil {
+		level.Error(l).Log("msg", "failed to apply memory limit", "err", err)
+	}
 
 	// Enable the profiling.
 	setMutexBlockProfiling(l)

internal/cmd/listcomponents/main.go

@@ -29,6 +29,6 @@ func main() {
 		}
 
 		stability, _ := strconv.Unquote(reg.Stability.String())
-		fmt.Fprintf(tw, "%s \t%s\n", reg.Name, stability)
+		_, _ = fmt.Fprintf(tw, "%s \t%s\n", reg.Name, stability)
 	}
 }

internal/component/beyla/ebpf/beyla_linux_test.go

@@ -438,7 +438,6 @@ func TestConvert_Filters(t *testing.T) {
 	config := args.Convert()
 
 	require.Equal(t, expectedConfig, config)
-
 }
 
 func TestArguments_Validate(t *testing.T) {

internal/component/common/kubernetes/event.go

@@ -22,10 +22,10 @@ const (
 
 type queuedEventHandler struct {
 	log   log.Logger
-	queue workqueue.RateLimitingInterface
+	queue workqueue.TypedRateLimitingInterface[Event]
 }
 
-func NewQueuedEventHandler(log log.Logger, queue workqueue.RateLimitingInterface) *queuedEventHandler {
+func NewQueuedEventHandler(log log.Logger, queue workqueue.TypedRateLimitingInterface[Event]) *queuedEventHandler {
 	return &queuedEventHandler{
 		log:   log,
 		queue: queue,

internal/component/common/kubernetes/rules_test.go

@@ -8,6 +8,6 @@ import (
 
 func TestEventTypeIsHashable(t *testing.T) {
 	// This test is here to ensure that the EventType type is hashable according to the workqueue implementation
-	queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
+	queue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[Event]())
 	queue.AddRateLimited(Event{})
 }

internal/component/common/loki/client/logger.go

@@ -74,12 +74,12 @@ func (l *logger) Chan() chan<- loki.Entry {
 
 func (l *logger) run() {
 	for e := range l.entries {
-		fmt.Fprint(l.Writer, blue.Sprint(e.Timestamp.Format("2006-01-02T15:04:05.999999999-0700")))
-		fmt.Fprint(l.Writer, "\t")
-		fmt.Fprint(l.Writer, yellow.Sprint(e.Labels.String()))
-		fmt.Fprint(l.Writer, "\t")
-		fmt.Fprint(l.Writer, e.Line)
-		fmt.Fprint(l.Writer, "\n")
+		_, _ = fmt.Fprint(l.Writer, blue.Sprint(e.Timestamp.Format("2006-01-02T15:04:05.999999999-0700")))
+		_, _ = fmt.Fprint(l.Writer, "\t")
+		_, _ = fmt.Fprint(l.Writer, yellow.Sprint(e.Labels.String()))
+		_, _ = fmt.Fprint(l.Writer, "\t")
+		_, _ = fmt.Fprint(l.Writer, e.Line)
+		_, _ = fmt.Fprint(l.Writer, "\n")
 		_ = l.Flush()
 	}
 }

internal/component/common/loki/positions/positions_test.go

@@ -21,7 +21,7 @@ import (
 func tempFilename(t *testing.T) string {
 	t.Helper()
 
-	temp, err := os.CreateTemp("", "positions")
+	temp, err := os.CreateTemp(t.TempDir(), "positions")
 	if err != nil {
 		t.Fatal("tempFilename:", err)
 	}

internal/component/common/loki/utils/fs_errors.go

@@ -13,7 +13,7 @@ import (
 // (e.g. due to antivirus scans) or if the file appears as closed when being removed or rotated.
 func IsEphemeralOrFileClosed(err error) bool {
 	return robustio.IsEphemeralError(err) ||
-		errors.Is(os.ErrClosed, err) ||
+		errors.Is(err, os.ErrClosed) ||
 		// The above errors.Is(os.ErrClosedm, err) condition doesn't always capture the 'file already closed' error on
 		// Windows. Check the error message as well.
 		// Inspired by https://github.com/grafana/loki/blob/987e551f9e21b9a612dd0b6a3e60503ce6fe13a8/clients/cmd/docker-driver/driver.go#L145

internal/component/common/relabel/relabel.go

@@ -230,7 +230,7 @@ func doRelabel(cfg *Config, lb LabelBuilder) (keep bool) {
 		values = make([]string, 0, len(cfg.SourceLabels))
 	}
 	for _, ln := range cfg.SourceLabels {
-		values = append(values, lb.Get(string(ln)))
+		values = append(values, lb.Get(ln))
 	}
 	val := strings.Join(values, cfg.Separator)
 

internal/component/database_observability/mysql/collector/parser/parser.go

@@ -0,0 +1,23 @@
+package parser
+
+import "github.com/go-kit/log"
+
+type Parser interface {
+	Parse(sql string) (any, error)
+	Redact(sql string) (string, error)
+	StmtType(stmt any) StatementType
+	ParseTableName(t any) string
+	ExtractTableNames(logger log.Logger, digest string, stmt any) []string
+}
+
+type StatementType string
+
+var (
+	StatementTypeSelect StatementType = "select"
+	StatementTypeInsert StatementType = "insert"
+	StatementTypeUpdate StatementType = "update"
+	StatementTypeDelete StatementType = "delete"
+
+	_ Parser = (*XwbSqlParser)(nil)
+	_ Parser = (*TiDBSqlParser)(nil)
+)

internal/component/database_observability/mysql/collector/parser/parser_tidb.go

@@ -38,17 +38,17 @@ func (p *TiDBSqlParser) Redact(sql string) (string, error) {
 	return res, nil
 }
 
-func (p *TiDBSqlParser) StmtType(stmt any) string {
+func (p *TiDBSqlParser) StmtType(stmt any) StatementType {
 	s := stmt.(*ast.StmtNode)
-	switch ast.GetStmtLabel(*s) {
-	case "Select":
-		return "select"
-	case "Insert":
-		return "insert"
-	case "Update":
-		return "update"
-	case "Delete":
-		return "delete"
+	switch (*s).(type) {
+	case *ast.SelectStmt:
+		return StatementTypeSelect
+	case *ast.InsertStmt:
+		return StatementTypeInsert
+	case *ast.UpdateStmt:
+		return StatementTypeUpdate
+	case *ast.DeleteStmt:
+		return StatementTypeDelete
 	default:
 		return ""
 	}

internal/component/database_observability/mysql/collector/parser/parser_xwb.go

@@ -20,18 +20,18 @@ func (p *XwbSqlParser) Redact(sql string) (string, error) {
 	return sqlparser.RedactSQLQuery(sql)
 }
 
-func (p *XwbSqlParser) StmtType(stmt any) string {
+func (p *XwbSqlParser) StmtType(stmt any) StatementType {
 	switch stmt.(type) {
 	case *sqlparser.Select:
-		return "select"
+		return StatementTypeSelect
 	case *sqlparser.Insert:
-		return "insert"
+		return StatementTypeInsert
 	case *sqlparser.Update:
-		return "update"
+		return StatementTypeUpdate
 	case *sqlparser.Delete:
-		return "delete"
+		return StatementTypeDelete
 	case *sqlparser.Union:
-		return "select" // label union as a select
+		return StatementTypeSelect // label union as a select
 	default:
 		return ""
 	}

internal/component/database_observability/mysql/collector/query_sample.go

@@ -47,27 +47,14 @@ type QuerySample struct {
 	instanceKey     string
 	collectInterval time.Duration
 	entryHandler    loki.EntryHandler
-	sqlParser       Parser
+	sqlParser       parser.Parser
 
 	logger  log.Logger
 	running *atomic.Bool
 	ctx     context.Context
 	cancel  context.CancelFunc
 }
 
-type Parser interface {
-	Parse(sql string) (any, error)
-	Redact(sql string) (string, error)
-	StmtType(stmt any) string
-	ParseTableName(t any) string
-	ExtractTableNames(logger log.Logger, digest string, stmt any) []string
-}
-
-var (
-	_ Parser = (*parser.XwbSqlParser)(nil)
-	_ Parser = (*parser.TiDBSqlParser)(nil)
-)
-
 func NewQuerySample(args QuerySampleArguments) (*QuerySample, error) {
 	c := &QuerySample{
 		dbConnection:    args.DB,

internal/component/database_observability/mysql/component_test.go

@@ -126,5 +126,4 @@ func Test_enableOrDisableCollectors(t *testing.T) {
 			collector.SchemaTableName: true,
 		}, actualCollectors)
 	})
-
 }

internal/component/discovery/kubelet/kubelet.go

@@ -161,10 +161,10 @@ func (d *Discovery) Refresh(ctx context.Context) ([]*targetgroup.Group, error) {
 	if err := json.Unmarshal(body, &podList); err != nil {
 		return nil, fmt.Errorf("error unmarshaling response body: %v", err)
 	}
-	return d.refresh(podList)
+	return d.refresh(podList), nil
 }
 
-func (d *Discovery) refresh(podList v1.PodList) ([]*targetgroup.Group, error) {
+func (d *Discovery) refresh(podList v1.PodList) []*targetgroup.Group {
 	discovered := make(map[string]bool)
 	// Create a list of targets from the pods
 	var targetGroups []*targetgroup.Group
@@ -191,7 +191,7 @@ func (d *Discovery) refresh(podList v1.PodList) ([]*targetgroup.Group, error) {
 	// update the list of discovered pods
 	d.discoveredPodSources = discovered
 
-	return targetGroups, nil
+	return targetGroups
 }
 
 func (d *Discovery) buildPodTargetGroup(pod v1.Pod) *targetgroup.Group {

internal/component/discovery/kubelet/kubelet_test.go

@@ -56,12 +56,10 @@ func TestPodDeletion(t *testing.T) {
 	kubeletDiscovery, err := NewKubeletDiscovery(args)
 	require.NoError(t, err)
 
-	_, err = kubeletDiscovery.refresh(podList1)
-	require.NoError(t, err)
+	kubeletDiscovery.refresh(podList1)
 	require.Len(t, kubeletDiscovery.discoveredPodSources, 2)
 
-	tg2, err := kubeletDiscovery.refresh(podList2)
-	require.NoError(t, err)
+	tg2 := kubeletDiscovery.refresh(podList2)
 	require.Len(t, kubeletDiscovery.discoveredPodSources, 1)
 	// should contain a target group for pod 1 with an empty target list as it has
 	// been deleted
@@ -107,8 +105,7 @@ func TestDiscoveryPodWithoutPod(t *testing.T) {
 	kubeletDiscovery, err := NewKubeletDiscovery(args)
 	require.NoError(t, err)
 
-	_, err = kubeletDiscovery.refresh(podList1)
-	require.NoError(t, err)
+	kubeletDiscovery.refresh(podList1)
 	require.Len(t, kubeletDiscovery.discoveredPodSources, 2)
 }
 

internal/component/discovery/relabel/relabel.go

@@ -79,6 +79,9 @@ func (c *Component) Run(ctx context.Context) error {
 
 // Update implements component.Component.
 func (c *Component) Update(args component.Arguments) error {
+	c.mut.Lock()
+	defer c.mut.Unlock()
+
 	newArgs := args.(Arguments)
 
 	targets := make([]discovery.Target, 0, len(newArgs.Targets))