diff --git a/.github/workflows/unit-test-on-pull-request.yml b/.github/workflows/unit-test-on-pull-request.yml index 91d29a530..e0564cfa4 100644 --- a/.github/workflows/unit-test-on-pull-request.yml +++ b/.github/workflows/unit-test-on-pull-request.yml @@ -37,18 +37,14 @@ jobs: - name: Get linter version id: linter-version run: (echo -n "version="; make linter-version) >> "$GITHUB_OUTPUT" - - name: golangci-lint - uses: golangci/golangci-lint-action@55c2c1448f86e01eaae002a5a3a9624417608d84 # v6.5.2 + - name: linter env: GOARCH: ${{ matrix.target-arch }} CGO_ENABLED: 1 - with: - version: ${{ steps.linter-version.outputs.version }} - - name: Lint eBPF code run: | sudo apt update sudo apt install -y clang-format-17 - make lint -C support/ebpf + make lint test: name: Test (${{ matrix.target_arch }}) diff --git a/.golangci.yml b/.golangci.yml index ec90a4a9e..b1eca4bcc 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,41 +1,27 @@ +version: "2" + +formatters: + enable: + - gofmt + - goimports + settings: + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + local-prefixes: + - github.com/open-telemetry/opentelemetry-ebpf-profiler + run: timeout: 10m build-tags: - integration - linux -issues: - exclude-dirs: - - artifacts - - build-targets - - design - - docker-images - - docs - - etc - - experiments - - infrastructure - - legal - - libpf-rs - - mocks - - pf-code-indexing-service/cibackend/gomock_* - - pf-debug-metadata-service/dmsbackend/gomock_* - - pf-host-agent/support/ci-kernels - - pf-storage-backend/storagebackend/gomock_* - - scratch - - systemtests/benchmarks/_outdata - - target - - virt-tests - - vm-images - - # Excluding configuration per-path, per-linter, per-text and per-source - exclude-rules: - # Don't complain about integer overflows - - text: "G115:" - linters: - - gosec - linters: - enable-all: true + default: all disable: # Disabled because of # - too many non-sensical warnings @@ -50,13 +36,14 @@ linters: - dupword - durationcheck # might be worth fixing - err113 + - errcheck - errorlint # might be worth fixing - exhaustive - exhaustruct - forbidigo - forcetypeassert # might be worth fixing - funlen - - gci # might be worth fixing + - funcorder - gochecknoglobals - gochecknoinits - gocognit @@ -64,8 +51,8 @@ linters: - gocyclo - godot - godox # complains about TODO etc - - gofumpt - gomoddirectives + - gosmopolitan - inamedparam - interfacebloat - ireturn @@ -81,6 +68,7 @@ linters: - paralleltest - protogetter - sqlclosecheck # might be worth fixing + - staticcheck - tagalign - tagliatelle - testableexamples # might be worth fixing @@ -94,45 +82,65 @@ linters: # we don't want to change code to Go 1.22+ yet - intrange - copyloopvar - - tenv -linters-settings: - goconst: - min-len: 2 - min-occurrences: 2 - gocritic: - enabled-tags: - - diagnostic - - experimental - - opinionated - - performance - - style - disabled-checks: - - dupImport # https://github.com/go-critic/go-critic/issues/845 - - ifElseChain - - whyNoLint - - sloppyReassign - - uncheckedInlineErr # Experimental rule with high false positive rate. - gocyclo: - min-complexity: 15 - govet: - enable-all: true - disable: - - fieldalignment - settings: - printf: # analyzer name, run `go tool vet help` to see all analyzers - funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer - - debug,debugf,debugln - - error,errorf,errorln - - fatal,fatalf,fataln - - info,infof,infoln - - log,logf,logln - - warn,warnf,warnln - - print,printf,println,sprint,sprintf,sprintln,fprint,fprintf,fprintln - lll: - line-length: 100 - tab-width: 4 - misspell: - locale: US - ignore-words: - - rela + exclusions: + paths: + - design-docs + - doc + - legal + - target + + settings: + goconst: + min-len: 2 + min-occurrences: 2 + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - ifElseChain + - whyNoLint + - sloppyReassign + - uncheckedInlineErr # experimental rule with high false positive rate. + - importShadow # shadow of imported package + gocyclo: + min-complexity: 15 + gosec: + excludes: + - G103 # unsafe calls should be audited + - G115 # integer overflow + - G204 # subprocess launched with variable + - G301 # directory permissions + - G302 # file permissions + - G304 # potential file inclusion via variable + govet: + enable-all: true + disable: + - fieldalignment + - unsafeptr + settings: + printf: # analyzer name, run `go tool vet help` to see all analyzers + funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer + - debug,debugf,debugln + - error,errorf,errorln + - fatal,fatalf,fataln + - info,infof,infoln + - log,logf,logln + - warn,warnf,warnln + - print,printf,println,sprint,sprintf,sprintln,fprint,fprintf,fprintln + lll: + line-length: 100 + tab-width: 4 + misspell: + locale: US + ignore-rules: + - rela + revive: + rules: + - name: unexported-naming + disabled: true diff --git a/Makefile b/Makefile index d35663594..4a8c5a7b1 100644 --- a/Makefile +++ b/Makefile @@ -83,11 +83,10 @@ rust-components: rust-targets rust-tests: rust-targets cargo test -GOLANGCI_LINT_VERSION = "v1.64.5" +GOLANGCI_LINT_VERSION = "v2.1.6" lint: generate vanity-import-check $(MAKE) lint -C support/ebpf - go run github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) version - go run github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) run + docker run --rm -t -v $$(pwd):/app -w /app golangci/golangci-lint:$(GOLANGCI_LINT_VERSION) sh -c "golangci-lint version && golangci-lint config verify && golangci-lint run --max-issues-per-linter -1 --max-same-issues -1" format-ebpf: $(MAKE) format -C support/ebpf diff --git a/interpreter/hotspot/method.go b/interpreter/hotspot/method.go index fb499f2a4..389943f62 100644 --- a/interpreter/hotspot/method.go +++ b/interpreter/hotspot/method.go @@ -13,8 +13,6 @@ import ( ) // Constants for the JVM internals that have never changed -// -//nolint:golint,stylecheck,revive const ConstMethod_has_linenumber_table = 0x0001 // hotspotMethod contains symbolization information for one Java method. It caches diff --git a/interpreter/perl/data.go b/interpreter/perl/data.go index 6a76081a5..41fb09dfc 100644 --- a/interpreter/perl/data.go +++ b/interpreter/perl/data.go @@ -24,7 +24,6 @@ type perlData struct { // vmStructs reflects the Perl internal class names and the offsets of named field // The struct names are based on the Perl C "struct name", the alternate typedef seen // mostly in code is in parenthesis. - //nolint:golint,stylecheck,revive vmStructs struct { // interpreter struct (PerlInterpreter) is defined in intrpvar.h via macro trickery // https://github.com/Perl/perl5/blob/v5.32.0/intrpvar.h diff --git a/interpreter/perl/perl.go b/interpreter/perl/perl.go index f3f7108ef..7228aa25b 100644 --- a/interpreter/perl/perl.go +++ b/interpreter/perl/perl.go @@ -52,7 +52,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/interpreter" ) -//nolint:golint,stylecheck,revive const ( // Scalar Value types (SVt) // https://github.com/Perl/perl5/blob/v5.32.0/sv.h#L132-L166 diff --git a/interpreter/php/instance.go b/interpreter/php/instance.go index fa619f314..57538a6e6 100644 --- a/interpreter/php/instance.go +++ b/interpreter/php/instance.go @@ -24,7 +24,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/util" ) -//nolint:golint,stylecheck,revive const ( // zend_function.type definitions from PHP sources ZEND_USER_FUNCTION = (1 << 1) diff --git a/interpreter/php/php.go b/interpreter/php/php.go index 5b6206adb..98a97d287 100644 --- a/interpreter/php/php.go +++ b/interpreter/php/php.go @@ -22,7 +22,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/support" ) -//nolint:golint,stylecheck,revive const ( // This is used to check if the VM mode is the default one // From https://github.com/php/php-src/blob/PHP-8.0/Zend/zend_vm_opcodes.h#L29 @@ -71,7 +70,6 @@ type phpData struct { rtAddr libpf.Address // vmStructs reflects the PHP internal class names and the offsets of named field - //nolint:golint,stylecheck,revive vmStructs struct { // https://github.com/php/php-src/blob/PHP-7.4/Zend/zend_globals.h#L135 zend_executor_globals struct { diff --git a/interpreter/ruby/ruby.go b/interpreter/ruby/ruby.go index 66f04b201..26410fa87 100644 --- a/interpreter/ruby/ruby.go +++ b/interpreter/ruby/ruby.go @@ -94,7 +94,6 @@ type rubyData struct { version uint32 // vmStructs reflects the Ruby internal names and offsets of named fields. - //nolint:golint,stylecheck,revive vmStructs struct { // rb_execution_context_struct // https://github.com/ruby/ruby/blob/5445e0435260b449decf2ac16f9d09bae3cafe72/vm_core.h#L843 diff --git a/libpf/pfelf/file.go b/libpf/pfelf/file.go index da55471a5..459a65dd1 100644 --- a/libpf/pfelf/file.go +++ b/libpf/pfelf/file.go @@ -174,7 +174,7 @@ func Open(name string) (*File, error) { ff, err := newFile(f, f, 0, false) if err != nil { - f.Close() + _ = f.Close() return nil, err } return ff, nil @@ -518,7 +518,7 @@ func (f *File) DebuglinkFileName(elfFilePath string, elfOpener ELFOpener) string } file, path := f.OpenDebugLink(elfFilePath, elfOpener) if file != nil { - file.Close() + _ = file.Close() } return path } @@ -648,7 +648,7 @@ func (f *File) OpenDebugLink(elfFilePath string, elfOpener ELFOpener) ( } fileCRC32, err := debugELF.CRC32() if err != nil || fileCRC32 != linkCRC32 { - debugELF.Close() + _ = debugELF.Close() continue } f.debuglinkPath = debugFile diff --git a/libpf/pfelf/pfelf_test.go b/libpf/pfelf/pfelf_test.go index 4d5b4231c..1e9addc57 100644 --- a/libpf/pfelf/pfelf_test.go +++ b/libpf/pfelf/pfelf_test.go @@ -69,7 +69,7 @@ func TestGetBuildIDError(t *testing.T) { buildID, err := pfelf.GetBuildID(elfFile) if assert.ErrorIs(t, pfelf.ErrNoBuildID, err) { - assert.Equal(t, "", buildID) + assert.Empty(t, buildID) } } @@ -83,7 +83,7 @@ func TestGetDebugLinkError(t *testing.T) { debugLink, _, err := pfelf.GetDebugLink(elfFile) if assert.ErrorIs(t, pfelf.ErrNoDebugLink, err) { - assert.Equal(t, "", debugLink) + assert.Empty(t, debugLink) } } diff --git a/libpf/pfelf/reference.go b/libpf/pfelf/reference.go index 5a40f2fbd..852aacd2a 100644 --- a/libpf/pfelf/reference.go +++ b/libpf/pfelf/reference.go @@ -41,7 +41,7 @@ func (ref *Reference) GetELF() (*File, error) { // Close closes the File if it has been opened earlier. func (ref *Reference) Close() { if ref.elfFile != nil { - ref.elfFile.Close() + _ = ref.elfFile.Close() ref.elfFile = nil } } diff --git a/nativeunwind/elfunwindinfo/elfehframe.go b/nativeunwind/elfunwindinfo/elfehframe.go index f0d90c1bc..a61b54597 100644 --- a/nativeunwind/elfunwindinfo/elfehframe.go +++ b/nativeunwind/elfunwindinfo/elfehframe.go @@ -96,7 +96,6 @@ const ( // The subset needed for normal .eh_frame handling type expressionOpcode uint8 -//nolint:deadcode,varcheck const ( opDeref expressionOpcode = 0x06 opConstU expressionOpcode = 0x10 @@ -123,7 +122,6 @@ type dwarfExpression struct { // https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/dwarfext.html type encoding uint8 -//nolint:deadcode,varcheck const ( encFormatNative encoding = 0x00 encFormatLeb128 encoding = 0x01 diff --git a/nativeunwind/elfunwindinfo/elfehframe_aarch64.go b/nativeunwind/elfunwindinfo/elfehframe_aarch64.go index 46768e8e9..2bc00ff4c 100644 --- a/nativeunwind/elfunwindinfo/elfehframe_aarch64.go +++ b/nativeunwind/elfunwindinfo/elfehframe_aarch64.go @@ -15,7 +15,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/support" ) -//nolint:deadcode,varcheck const ( // Aarch64 ABI armRegX0 uleb128 = 0 diff --git a/nativeunwind/elfunwindinfo/elfehframe_x86.go b/nativeunwind/elfunwindinfo/elfehframe_x86.go index 7d36c28cf..8a8c39512 100644 --- a/nativeunwind/elfunwindinfo/elfehframe_x86.go +++ b/nativeunwind/elfunwindinfo/elfehframe_x86.go @@ -15,7 +15,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/support" ) -//nolint:deadcode,varcheck const ( // x86_64 abi (https://refspecs.linuxbase.org/elf/x86_64-abi-0.99.pdf, page 57) x86RegRAX uleb128 = 0 diff --git a/nativeunwind/elfunwindinfo/elfgopclntab.go b/nativeunwind/elfunwindinfo/elfgopclntab.go index a37b627ab..eeaec34fb 100644 --- a/nativeunwind/elfunwindinfo/elfgopclntab.go +++ b/nativeunwind/elfunwindinfo/elfgopclntab.go @@ -46,8 +46,6 @@ const ( ) // pclntabHeader is the Golang pclntab header structure -// -//nolint:structcheck type pclntabHeader struct { // magic is one of the magicGo1_xx constants identifying the version magic uint32 @@ -63,8 +61,6 @@ type pclntabHeader struct { // pclntabHeader116 is the Golang pclntab header structure starting Go 1.16 // structural definition of this is found in go/src/runtime/symtab.go as pcHeader -// -//nolint:structcheck type pclntabHeader116 struct { pclntabHeader nfiles uint @@ -77,8 +73,6 @@ type pclntabHeader116 struct { // pclntabHeader118 is the Golang pclntab header structure starting Go 1.18 // structural definition of this is found in go/src/runtime/symtab.go as pcHeader -// -//nolint:structcheck type pclntabHeader118 struct { pclntabHeader nfiles uint @@ -91,16 +85,12 @@ type pclntabHeader118 struct { } // pclntabFuncMap is the Golang function symbol table map entry -// -//nolint:structcheck type pclntabFuncMap struct { pc uint64 funcOff uint64 } // pclntabFunc is the Golang function definition (struct _func in the spec) as before Go 1.18. -// -//nolint:structcheck type pclntabFunc struct { startPc uint64 nameOff, argsSize, frameSize int32 @@ -111,8 +101,6 @@ type pclntabFunc struct { // pclntabFunc118 is the Golang function definition (struct _func in the spec) // starting with Go 1.18. // see: go/src/runtime/runtime2.go (struct _func) -// -//nolint:structcheck type pclntabFunc118 struct { entryoff uint32 // start pc, as offset from pcHeader.textStart nameOff, argsSize, frameSize int32 diff --git a/nativeunwind/elfunwindinfo/stackdeltaextraction.go b/nativeunwind/elfunwindinfo/stackdeltaextraction.go index c1cf8758e..43cc73a92 100644 --- a/nativeunwind/elfunwindinfo/stackdeltaextraction.go +++ b/nativeunwind/elfunwindinfo/stackdeltaextraction.go @@ -94,7 +94,7 @@ func (ee *elfExtractor) extractDebugDeltas() error { debugELF, _ := ee.file.OpenDebugLink(ee.ref.FileName(), ee.ref) if debugELF != nil { err = ee.parseDebugFrame(debugELF) - debugELF.Close() + _ = debugELF.Close() } return err } diff --git a/process/coredump.go b/process/coredump.go index 8665b63e9..9da3f9c5f 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -86,7 +86,6 @@ type Note64 struct { Namesz, Descsz, Type uint32 } -//nolint:revive,stylecheck const ( NAMESPACE_CORE = "CORE\x00" NAMESPACE_LINUX = "LINUX\x00" diff --git a/processmanager/ebpf/ebpf.go b/processmanager/ebpf/ebpf.go index 71308f989..093761d8b 100644 --- a/processmanager/ebpf/ebpf.go +++ b/processmanager/ebpf/ebpf.go @@ -42,8 +42,6 @@ const ( ) // EbpfHandler provides the functionality to interact with eBPF maps. -// -//nolint:revive type EbpfHandler interface { // Embed interpreter.EbpfHandler as subset of this interface. interpreter.EbpfHandler @@ -793,8 +791,6 @@ func (impl *ebpfMapsImpl) DeletePidPageMappingInfoBatch(pid libpf.PID, prefixes // LookupPidPageInformation returns the fileID and bias for a given pid and page combination from // the eBPF map pid_page_to_mapping_info. // So far this function is used only in tests. -// -//nolint:deadcode func (impl *ebpfMapsImpl) LookupPidPageInformation(pid uint32, page uint64) (host.FileID, uint64, error) { // pid_page_to_mapping_info is a LPM trie and expects the pid and page diff --git a/processmanager/manager_test.go b/processmanager/manager_test.go index 9b8f789e7..932f97ca3 100644 --- a/processmanager/manager_test.go +++ b/processmanager/manager_test.go @@ -133,7 +133,7 @@ func generateDummyFiles(t *testing.T, num int) []string { content := []byte(tmpfile.Name()) _, err = tmpfile.Write(content) require.NoError(t, err) - tmpfile.Close() + _ = tmpfile.Close() require.NoError(t, err) files = append(files, tmpfile.Name()) } diff --git a/testsupport/testfiles.go b/testsupport/testfiles.go index c19c099d5..5c0373198 100644 --- a/testsupport/testfiles.go +++ b/testsupport/testfiles.go @@ -27,7 +27,7 @@ func writeExecutable(exeContents string) (string, error) { } else if err != nil { return "", fmt.Errorf("failed to write file: %v", err) } - exeFile.Close() + _ = exeFile.Close() return exeFile.Name(), nil } diff --git a/tools/coredump/new.go b/tools/coredump/new.go index 1849c4d0e..5ea18bd96 100644 --- a/tools/coredump/new.go +++ b/tools/coredump/new.go @@ -236,7 +236,6 @@ func dumpCore(pid uint64, noModuleBundling bool) (string, error) { } // `gcore` only accepts a path-prefix, not an exact path. - //nolint:gosec err := exec.Command("gcore", "-o", gcorePathPrefix, strconv.FormatUint(pid, 10)).Run() if err != nil { return "", fmt.Errorf("gcore failed: %w", err) diff --git a/tools/coredump/rebase.go b/tools/coredump/rebase.go index 04110bffc..8b024e2ef 100644 --- a/tools/coredump/rebase.go +++ b/tools/coredump/rebase.go @@ -62,7 +62,7 @@ func (cmd *rebaseCmd) exec(context.Context, []string) (err error) { } testCase.Threads, err = ExtractTraces(context.Background(), core, false, nil) - core.Close() + _ = core.Close() if err != nil { return fmt.Errorf("failed to extract traces: %w", err) } diff --git a/tools/coredump/storecoredump.go b/tools/coredump/storecoredump.go index 5363bf364..a8405d7ca 100644 --- a/tools/coredump/storecoredump.go +++ b/tools/coredump/storecoredump.go @@ -72,10 +72,10 @@ func (scd *StoreCoredump) ExtractAsFile(file string) (string, error) { return "", err } tmpFile := f.Name() - f.Close() + _ = f.Close() if err := scd.store.UnpackModuleToPath(info.Ref, tmpFile); err != nil { - os.Remove(tmpFile) + _ = os.Remove(tmpFile) return "", err } scd.tempFiles[file] = tmpFile @@ -84,7 +84,7 @@ func (scd *StoreCoredump) ExtractAsFile(file string) (string, error) { func (scd *StoreCoredump) Close() error { for _, tmpFile := range scd.tempFiles { - os.Remove(tmpFile) + _ = os.Remove(tmpFile) } return scd.CoredumpProcess.Close() }