amazon-cloudwatch-agent icon indicating copy to clipboard operation
amazon-cloudwatch-agent copied to clipboard

Bump linter for go1.25 compatibility. apply linting fixes

Open dricross opened this issue 5 months ago • 1 comments

Description of the issue

Check lint is failing on PRs:

Failed executing command with error: can't load config: the Go language version (go1.24) used to build golangci-lint is lower than the targeted Go version (1.25.0)

We recently bumped to go1.25 but apparently that PR skipped the lint check so it didn't catch this incompatibility :melting_face: We have to upgrade from v1.64.2 all the way to v2.4.0 since that's when go1.25 support was added, which incurred some new linting rules.

The upgrade from v1.x to v2.x comes with a new format for the .golangci-lint.yml file. Migrated to v2 format using the following command:

% golangci-lint migrate --skip-validation
WARN The configuration comments are not migrated. 
WARN Details about the migration: https://golangci-lint.run/docs/product/migration-guide/ 
WARN The configuration `run.timeout` is ignored. By default, in v2, the timeout is disabled. 

Output of make lint after upgrading and migrating:

% make lint
#Install from source for golangci-lint is not recommended based on https://golangci-lint.run/usage/install/#install-from-source so using binary
#installation
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b /local/home/dricross/workplace/amazon-cloudwatch-agent/build/tools v2.4.0
golangci/golangci-lint info checking GitHub for tag 'v2.4.0'
golangci/golangci-lint info found version: 2.4.0 for v2.4.0/linux/amd64
golangci/golangci-lint info installed /local/home/dricross/workplace/amazon-cloudwatch-agent/build/tools/golangci-lint
# Using 04bfe4e to get SPDX template changes that are not present in the most recent tag v1.0.0
# This is required to be able to easily omit the year in our license header.
GOBIN=/local/home/dricross/workplace/amazon-cloudwatch-agent/build/tools go install github.com/google/addlicense@04bfe4e
Check License finished successfully
GOBIN=/local/home/dricross/workplace/amazon-cloudwatch-agent/build/tools go install github.com/pavius/impi/cmd/[email protected]
go: finding module for package github.com/kisielk/gotool
go: found github.com/kisielk/gotool in github.com/kisielk/gotool v1.0.0
# Skip plugins/plugins.go
Check import order/grouping finished
/local/home/dricross/workplace/amazon-cloudwatch-agent/build/tools/golangci-lint run ./...
internal/k8sCommon/k8sclient/kubernetes_utils.go:86:9: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck)
        if pod.ObjectMeta.OwnerReferences != nil {
               ^
internal/k8sCommon/k8sclient/kubernetes_utils.go:87:32: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck)
                for _, ownerRef := range pod.ObjectMeta.OwnerReferences {
                                             ^
internal/k8sCommon/k8sclient/kubernetes_utils.go:92:4: QF1003: could use tagged switch on ownerRef.Kind (staticcheck)
                        if ownerRef.Kind == "ReplicaSet" {
                        ^
internal/k8sCommon/k8sclient/kubernetes_utils_test.go:207:6: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck)
        pod.ObjectMeta.OwnerReferences = nil
            ^
internal/state/range.go:385:3: QF1003: could use tagged switch on count (staticcheck)
                if count == 0 {
                ^
plugins/inputs/logfile/tail/tail.go:426:22: ST1005: error strings should not be capitalized (staticcheck)
                                lineObject.Err = errors.New(msg)
                                                 ^
plugins/inputs/prometheus/start.go:39:2: ST1019(related information): other import of "github.com/prometheus/prometheus/config" (staticcheck)
        promconfig "github.com/prometheus/prometheus/config"
        ^
plugins/outputs/cloudwatch/cloudwatch.go:486:42: QF1008: could remove embedded field "MetricDatum" from selector (staticcheck)
                        if index == 0 && c.IsDropping(*metric.MetricDatum.MetricName) {
                                                              ^
plugins/outputs/cloudwatch/cloudwatch.go:558:42: QF1008: could remove embedded field "MetricDatum" from selector (staticcheck)
                        if index == 0 && c.IsDropping(*metric.MetricDatum.MetricName) {
                                                              ^
plugins/processors/ec2tagger/ec2tagger.go:357:9: QF1008: could remove embedded field "Config" from selector (staticcheck)
                        if t.Config.MiddlewareID != nil {
                             ^
plugins/processors/ec2tagger/ec2tagger.go:358:51: QF1008: could remove embedded field "Config" from selector (staticcheck)
                                awsmiddleware.TryConfigure(t.logger, host, *t.Config.MiddlewareID, awsmiddleware.SDKv1(&client.Handlers))
                                                                              ^
plugins/processors/ec2tagger/ec2tagger.go:387:17: QF1001: could apply De Morgan's law (staticcheck)
                needRefresh = !(len(t.EC2InstanceTagKeys) == 1 && t.EC2InstanceTagKeys[0] == "*")
                              ^
plugins/processors/ec2tagger/ec2tagger.go:412:17: QF1001: could apply De Morgan's law (staticcheck)
                needRefresh = !(len(t.EBSDeviceKeys) == 1 && t.EBSDeviceKeys[0] == "*")
                              ^
receiver/awsnvmereceiver/scraper.go:188:28: QF1008: could remove embedded field "TelemetrySettings" from selector (staticcheck)
                logger:         settings.TelemetrySettings.Logger,
                                         ^
receiver/awsnvmereceiver/scraper.go:201:24: QF1008: could remove embedded field "MetricsBuilderConfig" from selector (staticcheck)
                if ts.IsEnabled(&cfg.MetricsBuilderConfig.Metrics) {
                                     ^
receiver/awsnvmereceiver/scraper_test.go:64:11: QF1008: could remove embedded field "TelemetrySettings" from selector (staticcheck)
        settings.TelemetrySettings.Logger = logger
                 ^
receiver/awsnvmereceiver/scraper_test.go:86:11: QF1008: could remove embedded field "TelemetrySettings" from selector (staticcheck)
        settings.TelemetrySettings.Logger = logger
                 ^
receiver/awsnvmereceiver/scraper_test.go:375:3: QF1003: could use tagged switch on devicePath (staticcheck)
                if devicePath == "/dev/nvme0n1" {
                ^
receiver/awsnvmereceiver/scraper_test.go:612:6: QF1008: could remove embedded field "MetricsBuilderConfig" from selector (staticcheck)
        cfg.MetricsBuilderConfig.Metrics.DiskioInstanceStoreTotalReadOps.Enabled = false
            ^
receiver/awsnvmereceiver/scraper_test.go:613:6: QF1008: could remove embedded field "MetricsBuilderConfig" from selector (staticcheck)
        cfg.MetricsBuilderConfig.Metrics.DiskioInstanceStoreTotalWriteOps.Enabled = false
            ^
translator/translate/otel/receiver/prometheus/translator.go:99:35: QF1008: could remove embedded field "Config" from selector (staticcheck)
                        cfg.TargetAllocator.TLSSetting.Config.CAFile = defaultTLSCaPath
                                                       ^
21 issues:
* staticcheck: 21
make: *** [lint] Error 1

Another lint error showed up after the PR check ran:

/home/runner/work/amazon-cloudwatch-agent/amazon-cloudwatch-agent/build/tools/golangci-lint run ./...
Error: translator/translate/logs/logs_collected/windows_events/ruleMaxPersistState.go:4:9: var-naming: don't use an underscore in package name (revive)
package windows_events
        ^
1 issues:
* revive: 1

More lint issues showed up after adjusting the windows_events package name:

/local/home/dricross/workplace/amazon-cloudwatch-agent/build/tools/golangci-lint run ./...
translator/translate/logs/logs_collected/windowsevents/collect_list/collectlist.go:51:1: named return "returnKey" with type "string" found (nonamedreturns)
func (c *CollectList) ApplyRule(input interface{}) (returnKey string, returnVal interface{}) {
^
translator/translate/logs/logs_collected/windowsevents/collect_list/ruleEventFormat.go:22:1: named return "returnKey" with type "string" found (nonamedreturns)
func (r *EventFormat) ApplyRule(input interface{}) (returnKey string, returnVal interface{}) {
^
translator/translate/logs/logs_collected/windowsevents/windows_event.go:31:1: named return "returnKey" with type "string" found (nonamedreturns)
func (w *WindowsEvent) ApplyRule(input interface{}) (returnKey string, returnVal interface{}) {
^
translator/translate/logs/logs_collected/windowsevents/collect_list/collectlist.go:94:6: var-naming: func addFixedJsonConfig should be addFixedJSONConfig (revive)
func addFixedJsonConfig(result map[string]interface{}) {
     ^
translator/translate/logs/logs_collected/windowsevents/ruleFileStateFolder.go:12:37: unused-parameter: parameter 'input' seems to be unused, consider removing or renaming it as _ (revive)
func (f *FileStateFolder) ApplyRule(input interface{}) (returnKey string, returnVal interface{}) {
                                    ^
translator/translate/logs/logs_collected/windowsevents/windows_event.go:48:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
        } else {
                translator.AddInfoMessages("", "No windows event log configuration found.")
                return "", '"'
        }
translator/translate/logs/logs_collected/windowsevents/windows_event_test.go:18:6: var-naming: var rawJsonString should be rawJSONString (revive)
        var rawJsonString = `
            ^
translator/translate/logs/logs_collected/windowsevents/collect_list/collectlist.go:38:5: var eventLevelMapping is unused (unused)
var eventLevelMapping = map[string]string{
    ^
8 issues:
* nonamedreturns: 3
* revive: 4
* unused: 1

Description of changes

  • Bump linter to be compatible with go1.25
  • Apply linting fixes
  • Add go.mod and Makefile to linting file filters. If we bump the Go version or the linter version, this will make the lint check step run

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

  • make lint

Requirements

Before commiting your code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

Integration Tests

To run integration tests against this PR, add the ready for testing label.

dricross avatar Sep 05 '25 14:09 dricross

Ran into too many issues with various linting issues and flaky unit tests to solve reasonably quickly. Ended up reverting the Go bump: https://github.com/aws/amazon-cloudwatch-agent/pull/1851. When we re-do the Go bump we'll look into these.

dricross avatar Sep 05 '25 20:09 dricross

integ tests are passing: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/20240547778

gonna update the branch, but shouldn't change any test results

the-mann avatar Dec 19 '25 16:12 the-mann