Skip to content

spancheck.extra-start-span-signatures configuration overwrites defaults #5323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
6 of 7 tasks
magnusbaeck opened this issue Jan 15, 2025 · 1 comment · Fixed by #5324
Closed
6 of 7 tasks

spancheck.extra-start-span-signatures configuration overwrites defaults #5323

magnusbaeck opened this issue Jan 15, 2025 · 1 comment · Fixed by #5324
Assignees
Labels
bug Something isn't working

Comments

@magnusbaeck
Copy link

magnusbaeck commented Jan 15, 2025

Welcome

  • Yes, I'm using a binary release within 2 latest releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've read the typecheck section of the FAQ.
  • Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.).
  • I agree to follow this project's Code of Conduct

Description of the problem

Setting the extra-start-span-signatures configuration of the spancheck linter will overwrite a value in the struct that's being passed to spancheck.NewAnalyzerWithConfig. This wipes out the default value set in spancheck.NewDefaultConfig. This isn't how the spancheck CLI works; it appends to the same slice.

This is a problem in itself, but diagnosing it gets even more confusing because of a spancheck conditional that makes it ignore the first len(defaultStartSpanSignatures) items in the slice when creating a regexp for matching function signatures (since only extra signatures belong in that regexp). Thus,

linters-settings:
  spancheck:
    extra-start-span-signatures:
      - validsignature1
      - validsignature2

won't result in any custom function signatures, and golangci-lint's behavior deviates from the equivalent spancheck invocation (spancheck -extra-start-span-signatures validsignature1,validsignature2 ./...). However, if you pad the array with len(defaultStartSpanSignatures) dummy values,

linters-settings:
  spancheck:
    extra-start-span-signatures:
      - dummy1
      - dummy2
      - dummy3
      - validsignature1
      - validsignature2

golangci-lint behaves like spancheck again. At least with my code; I'd expect the absence of default signatures to cause problems in some cases.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.63.4 built with go1.23.4 from c1149695 on 2025-01-03T19:49:42Z

Configuration

---
linters:
  disable-all: true
  enable:
    - bidichk
    - bodyclose
    - canonicalheader
    - contextcheck
    - copyloopvar
    - dupword
    - durationcheck
    - errcheck
    - errchkjson
    - exhaustive
    - exptostd
    - fatcontext
    - forcetypeassert
    - gocyclo
    - godot
    - gofmt
    - gosec
    - gosimple
    - govet
    - inamedparam
    - ineffassign
    - makezero
    - mirror
    - misspell
    - nilnesserr
    - noctx
    - nosprintfhostport
    - predeclared
    - reassign
    - spancheck
    - staticcheck
    - tagalign
    - tenv
    - testableexamples
    - testifylint
    - typecheck
    - unconvert
    - unparam
    - unused
    - usestdlibvars
    - usetesting
    - whitespace
linters-settings:
  misspell:
    locale: US
  spancheck:
    extra-start-span-signatures:
      - 'example.com/reponame/internal/eventpipeline.Pipeline..processMessage:opentelemetry'
      - 'example.com/packagename.StartEventSpan:opentelemetry'
  usestdlibvars:
    crypto-hash: true
    http-method: false
    sql-isolation-level: true
    time-layout: true
    tls-signature-scheme: true
  usetesting:
    os-setenv: true
    os-temp-dir: true

Go environment

$ go version && go env
go version go1.23.3 linux/amd64
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/username/.cache/go-build'
GOENV='/home/username/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/username/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/username/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go-1.23.3'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go-1.23.3/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.3'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/username/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/username/src/reponame/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build213867971=/tmp/go-build -gno-record-gcc-switches'

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO golangci-lint has version 1.63.4 built with go1.23.4 from c1149695 on 2025-01-03T19:49:42Z 
INFO [config_reader] Config search paths: [./ /home/username/src/reponame /home/username/src /home/username /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 41 linters: [bidichk bodyclose canonicalheader contextcheck copyloopvar dupword durationcheck errcheck errchkjson exhaustive exptostd fatcontext forcetypeassert gocyclo godot gofmt gosec gosimple govet inamedparam ineffassign makezero mirror misspell nilnesserr noctx nosprintfhostport predeclared reassign spancheck staticcheck tagalign tenv testableexamples testifylint unconvert unparam unused usestdlibvars usetesting whitespace] 
INFO [loader] Go packages loading at mode 8767 (imports|name|exports_file|files|types_sizes|compiled_files|deps) took 270.170891ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 7.211595ms 
INFO [linters_context/goanalysis] analyzers took 1m40.52208184s with top 10 stages: buildir: 34.968207546s, buildssa: 9.508123401s, exhaustive: 4.142349359s, inspect: 3.778784143s, fact_deprecated: 3.764710622s, ctrlflow: 2.756909261s, printf: 2.363026932s, fact_purity: 1.609404469s, nilness: 1.400566945s, SA5012: 1.240096491s 
INFO [runner] Issues before processing: 103, after processing: 2 
INFO [runner] Processors filtering stat (in/out): filename_unadjuster: 103/103, invalid_issue: 103/103, path_prettifier: 103/103, skip_dirs: 103/103, autogenerated_exclude: 103/98, max_from_linter: 2/2, exclude-rules: 98/28, nolint: 28/2, max_same_issues: 2/2, source_code: 2/2, fixer: 2/2, path_prefixer: 2/2, sort_results: 2/2, max_per_file_from_linter: 2/2, path_shortener: 2/2, cgo: 103/103, skip_files: 103/103, identifier_marker: 98/98, exclude: 98/98, uniq_by_line: 2/2, diff: 2/2, severity-rules: 2/2 
INFO [runner] processing took 7.051091ms with stages: nolint: 4.657221ms, exclude-rules: 804.882µs, identifier_marker: 762.44µs, path_prettifier: 405.67µs, autogenerated_exclude: 267.75µs, skip_dirs: 89.11µs, source_code: 19.594µs, filename_unadjuster: 18.474µs, cgo: 9.644µs, invalid_issue: 8.845µs, uniq_by_line: 2.366µs, max_same_issues: 2.201µs, path_shortener: 759ns, max_from_linter: 467ns, max_per_file_from_linter: 467ns, fixer: 248ns, sort_results: 219ns, exclude: 196ns, diff: 190ns, skip_files: 185ns, severity-rules: 84ns, path_prefixer: 79ns 
INFO [runner] linters took 8.114369936s with stages: goanalysis_metalinter: 8.10725533s 
internal/eventpipeline/pipeline.go:79:105: span is unassigned, probable memory leak (spancheck)
	err := logretryamqp.Start(ctx, ep.logger, ep.consumerCfg, messageproc.QuarantiningProcessor(ep.logger, ep.processMessage, ep.quarantin))
	                                                                                                       ^
internal/eventpipeline/pipeline.go:199:7: span is unassigned, probable memory leak (spancheck)
	ctx, _ = packagename.StartEventSpan(ctx, fooEvent, packagename.Receive)
	     ^
INFO File cache stats: 100 entries of total size 522.7KiB 
INFO Memory: 84 samples, avg is 610.4MB, max is 914.3MB 
INFO Execution took 8.39465925s  ```

A minimal reproducible example or link to a public repository

Prerequisites:

$ go mod init example.com
go: creating new go.mod: module example.com
$ go get go.opentelemetry.io/otel/trace
go: downloading go.opentelemetry.io/otel v1.33.0
go: downloading go.opentelemetry.io/otel/trace v1.33.0
go: added go.opentelemetry.io/otel v1.33.0
go: added go.opentelemetry.io/otel/trace v1.33.0
$ go get go.opentelemetry.io/otel 
go: downloading go.opentelemetry.io/otel/metric v1.33.0
go: downloading go.opentelemetry.io/auto/sdk v1.1.0

main.go:

package main

import (
	"context"

	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/trace"
)

func StartTrace() (context.Context, trace.Span) {
	return otel.Tracer("example.com/main").Start(context.Background(), "span name")
}

func main() {
	_, _ = StartTrace()
}

Running this with the default configuration correctly yields an error about a span leak using either golangci-lint or spancheck:

$ cat .golangci.yml && golangci-lint run -v
---
linters:
  disable-all: true
  enable:
    - spancheck
INFO golangci-lint has version 1.63.4 built with go1.23.4 from c1149695 on 2025-01-03T19:49:42Z 
INFO [config_reader] Config search paths: [./ /tmp/trash.ZpLO /tmp / /home/username] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 1 linters: [spancheck]    
INFO [loader] Go packages loading at mode 8767 (types_sizes|compiled_files|files|imports|name|deps|exports_file) took 86.619274ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 109.877µs 
INFO [linters_context/goanalysis] analyzers took 453.207µs with top 10 stages: ctrlflow: 271.492µs, inspect: 80.767µs, spancheck: 50.788µs, typecheck: 50.16µs 
INFO [runner] Processors filtering stat (in/out): max_from_linter: 1/1, source_code: 1/1, sort_results: 1/1, skip_dirs: 1/1, exclude: 1/1, identifier_marker: 1/1, max_per_file_from_linter: 1/1, max_same_issues: 1/1, path_prefixer: 1/1, invalid_issue: 1/1, path_prettifier: 1/1, autogenerated_exclude: 1/1, path_shortener: 1/1, fixer: 1/1, filename_unadjuster: 1/1, skip_files: 1/1, nolint: 1/1, uniq_by_line: 1/1, diff: 1/1, severity-rules: 1/1, cgo: 1/1, exclude-rules: 1/1 
INFO [runner] processing took 93.5µs with stages: nolint: 24.084µs, identifier_marker: 18.484µs, exclude-rules: 13.137µs, autogenerated_exclude: 10.765µs, path_prettifier: 8.932µs, source_code: 8.214µs, skip_dirs: 3.318µs, uniq_by_line: 1.031µs, max_same_issues: 961ns, invalid_issue: 773ns, cgo: 571ns, max_from_linter: 543ns, path_shortener: 539ns, filename_unadjuster: 528ns, max_per_file_from_linter: 265ns, severity-rules: 240ns, exclude: 224ns, skip_files: 222ns, fixer: 211ns, sort_results: 207ns, diff: 167ns, path_prefixer: 84ns 
INFO [runner] linters took 53.487217ms with stages: goanalysis_metalinter: 53.354342ms 
main.go:11:9: span is unassigned, probable memory leak (spancheck)
	return otel.Tracer("example.com/main").Start(context.Background(), "span name")
	       ^
INFO File cache stats: 1 entries of total size 271B 
INFO Memory: 3 samples, avg is 37.2MB, max is 47.1MB 
INFO Execution took 144.138808ms                  
$ spancheck ./...    
/tmp/trash.ZpLO/main.go:11:9: span is unassigned, probable memory leak

Adding a extra span start signature that doesn't match any function or method should clearly yield the same result, but doesn't since the default signatures have been overwritten:

$ cat .golangci.yml && golangci-lint run -v
---
linters:
  disable-all: true
  enable:
    - spancheck
linters-settings:
  spancheck:
    extra-start-span-signatures:
      - dummy:opentelemetry
INFO golangci-lint has version 1.63.4 built with go1.23.4 from c1149695 on 2025-01-03T19:49:42Z 
INFO [config_reader] Config search paths: [./ /tmp/trash.ZpLO /tmp / /home/username] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 1 linters: [spancheck]    
INFO [loader] Go packages loading at mode 8767 (compiled_files|deps|exports_file|types_sizes|files|imports|name) took 83.879089ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 240.067µs 
INFO [linters_context/goanalysis] analyzers took 349.608916ms with top 10 stages: inspect: 189.516986ms, ctrlflow: 160.030358ms, spancheck: 57.055µs, typecheck: 4.517µs 
INFO [runner] processing took 2.306µs with stages: max_same_issues: 274ns, skip_dirs: 242ns, severity-rules: 170ns, cgo: 155ns, path_prettifier: 142ns, invalid_issue: 138ns, filename_unadjuster: 137ns, source_code: 126ns, nolint: 122ns, skip_files: 119ns, max_from_linter: 118ns, identifier_marker: 116ns, exclude: 115ns, fixer: 41ns, exclude-rules: 40ns, diff: 39ns, path_shortener: 37ns, sort_results: 36ns, path_prefixer: 36ns, autogenerated_exclude: 35ns, uniq_by_line: 34ns, max_per_file_from_linter: 34ns 
INFO [runner] linters took 846.680675ms with stages: goanalysis_metalinter: 846.652606ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 11 samples, avg is 126.8MB, max is 197.3MB 
INFO Execution took 933.6993ms                    
$ spancheck -extra-start-span-signatures dummy:opentelemetry ./...
/tmp/trash.ZpLO/main.go:11:9: span is unassigned, probable memory leak

Validation

  • Yes, I've included all information above (version, config, etc.).

Supporter

@magnusbaeck magnusbaeck added the bug Something isn't working label Jan 15, 2025
Copy link

boring-cyborg bot commented Jan 15, 2025

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants