Skip to content

Commit

Permalink
Merge pull request #256 from bobrik/ivan/golangci-lint-more
Browse files Browse the repository at this point in the history
Enable more warnings from golangci-lint
  • Loading branch information
bobrik authored Aug 31, 2023
2 parents 9c4571a + 8303ee1 commit c7eaf49
Show file tree
Hide file tree
Showing 20 changed files with 91 additions and 55 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ jobs:
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: v1.51.2
version: v1.54.2
env:
CGO_LDFLAGS: "-l bpf"

Expand Down
36 changes: 36 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
linters:
enable:
- dupl
- durationcheck
- exhaustive
- exportloopref
- forcetypeassert
- goimports
- gomoddirectives
- interfacebloat
- makezero
- mirror
- misspell
- musttag
- nestif
- nilnil
- nolintlint
- nosprintfhostport
- prealloc
- predeclared
- promlinter
- reassign
- revive
- tagalign
- unconvert
- unparam
- unused
- usestdlibvars
- whitespace
# nilerr: https://github.com/gostaticanalysis/nilerr/issues/8
# wastedassign: https://github.com/sanposhiho/wastedassign/issues/39
issues:
exclude-use-default: false
exclude:
- Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
- should have a package comment
26 changes: 13 additions & 13 deletions benchmark/getpid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,45 +28,45 @@ func BenchmarkGetpidWithoutAnyProbes(b *testing.B) {
}

func BenchmarkGetpidTracepointWithNoMap(b *testing.B) {
benchmarkWithProbe(b, "tracepoint", "probes/tracepoint-empty.bpf.o", false)
benchmarkWithProbe(b, "probes/tracepoint-empty.bpf.o", false)
}

func BenchmarkGetpidTracepointWithSimpleMap(b *testing.B) {
benchmarkWithProbe(b, "tracepoint", "probes/tracepoint-simple.bpf.o", true)
benchmarkWithProbe(b, "probes/tracepoint-simple.bpf.o", true)
}

func BenchmarkGetpidTracepointWithComplexMap(b *testing.B) {
benchmarkWithProbe(b, "tracepoint", "probes/tracepoint-complex.bpf.o", true)
benchmarkWithProbe(b, "probes/tracepoint-complex.bpf.o", true)
}

func BenchmarkGetpidFentryWithNoMap(b *testing.B) {
benchmarkWithProbe(b, "fentry", "probes/fentry-empty.bpf.o", false)
benchmarkWithProbe(b, "probes/fentry-empty.bpf.o", false)
}

func BenchmarkGetpidFentryWithSimpleMap(b *testing.B) {
benchmarkWithProbe(b, "fentry", "probes/fentry-simple.bpf.o", true)
benchmarkWithProbe(b, "probes/fentry-simple.bpf.o", true)
}

func BenchmarkGetpidFentryWithComplexMap(b *testing.B) {
benchmarkWithProbe(b, "fentry", "probes/fentry-complex.bpf.o", true)
benchmarkWithProbe(b, "probes/fentry-complex.bpf.o", true)
}

func BenchmarkGetpidKprobeWithNoMap(b *testing.B) {
benchmarkWithProbe(b, "kprobe", "probes/kprobe-empty.bpf.o", false)
benchmarkWithProbe(b, "probes/kprobe-empty.bpf.o", false)
}

func BenchmarkGetpidKprobeWithSimpleMap(b *testing.B) {
benchmarkWithProbe(b, "kprobe", "probes/kprobe-simple.bpf.o", true)
benchmarkWithProbe(b, "probes/kprobe-simple.bpf.o", true)
}

func BenchmarkGetpidKprobeWithComplexMap(b *testing.B) {
benchmarkWithProbe(b, "kprobe", "probes/kprobe-complex.bpf.o", true)
benchmarkWithProbe(b, "probes/kprobe-complex.bpf.o", true)
}

func benchmarkWithProbe(b *testing.B, kind string, file string, checkMap bool) {
func benchmarkWithProbe(b *testing.B, file string, checkMap bool) {
byteOrder := util.GetHostByteOrder()

m, link, err := setupGetpidProbe(kind, file)
m, link, err := setupGetpidProbe(file)
if err != nil {
b.Fatalf("Error setting up getpid probe: %v", err)
}
Expand Down Expand Up @@ -100,7 +100,7 @@ func benchmarkWithProbe(b *testing.B, kind string, file string, checkMap bool) {

iter := counts.Iterator()
for iter.Next() {
keys += 1
keys++
valueBytes, err := counts.GetValue(unsafe.Pointer(&iter.Key()[0]))
if err != nil {
b.Fatalf("Error reading key from bpf map: %v", err)
Expand All @@ -120,7 +120,7 @@ func benchmarkWithProbe(b *testing.B, kind string, file string, checkMap bool) {
b.Logf("keys = %d, value = %d", keys, value)
}

func setupGetpidProbe(kind string, name string) (*libbpfgo.Module, *libbpfgo.BPFLink, error) {
func setupGetpidProbe(name string) (*libbpfgo.Module, *libbpfgo.BPFLink, error) {
module, err := libbpfgo.NewModuleFromFile(name)
if err != nil {
return nil, nil, fmt.Errorf("error creating module from file %q: %v", name, err)
Expand Down
1 change: 0 additions & 1 deletion cmd/ebpf_exporter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ func listen(addr string) error {
}

return http.ListenAndServe(addr, nil)

}

func ensureCapabilities(keep string) error {
Expand Down
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ const (
HistogramBucketFixed HistogramBucketType = "fixed"
)

// ParseConfigs parses the named configs from the provided configs directory
func ParseConfigs(dir string, names []string) ([]Config, error) {
configs := make([]Config, len(names))

Expand Down
4 changes: 2 additions & 2 deletions decoder/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestDecodeLabels(t *testing.T) {
err bool
}{
{
in: append([]byte{0x8, 0x0, 0x0, 0x0}, zeroPaddedString("bananas", 32)...),
in: append([]byte{0x8, 0x0, 0x0, 0x0}, zeroPaddedString("potatoes", 16)...),
labels: []config.Label{
{
Name: "number",
Expand Down Expand Up @@ -116,7 +116,7 @@ func TestDecodeLabels(t *testing.T) {
},
},
out: []string{"8", "bananas"},
err: true, // this label should be skipped, only tomatos allowed
err: true, // this label should be skipped, only tomatoes allowed
},
}

Expand Down
2 changes: 1 addition & 1 deletion decoder/dname.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
type Dname struct{}

// Decode transforms wire format into string
func (d *Dname) Decode(in []byte, conf config.Decoder) ([]byte, error) {
func (d *Dname) Decode(in []byte, _ config.Decoder) ([]byte, error) {
if len(in) == 0 {
return []byte("."), nil
}
Expand Down
2 changes: 1 addition & 1 deletion decoder/inet_ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
type InetIP struct{}

// Decode transforms an ip byte representation into a string
func (i *InetIP) Decode(in []byte, conf config.Decoder) ([]byte, error) {
func (i *InetIP) Decode(in []byte, _ config.Decoder) ([]byte, error) {
ip := net.IP(in)
return []byte(ip.String()), nil
}
2 changes: 1 addition & 1 deletion decoder/ksym.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type KSym struct {
}

// Decode transforms kernel address to a function name
func (k *KSym) Decode(in []byte, conf config.Decoder) ([]byte, error) {
func (k *KSym) Decode(in []byte, _ config.Decoder) ([]byte, error) {
if k.cache == nil {
k.cache = map[string][]byte{}
}
Expand Down
2 changes: 1 addition & 1 deletion decoder/majorminor.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type MajorMinor struct {
}

// Decode transforms minormajor device id into device name like sda2
func (m *MajorMinor) Decode(in []byte, conf config.Decoder) ([]byte, error) {
func (m *MajorMinor) Decode(in []byte, _ config.Decoder) ([]byte, error) {
if m.cache == nil {
m.cache = map[uint64][]byte{}
}
Expand Down
2 changes: 1 addition & 1 deletion decoder/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
type String struct{}

// Decode transforms byte slice from the kernel into string
func (s *String) Decode(in []byte, conf config.Decoder) ([]byte, error) {
func (s *String) Decode(in []byte, _ config.Decoder) ([]byte, error) {
return in[0:clen(in)], nil
}

Expand Down
6 changes: 4 additions & 2 deletions decoder/syscall.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"github.com/cloudflare/ebpf_exporter/v2/config"
)

// Syscall is a decoder that decodes syscall numbers into their names
type Syscall struct{}

// Decode transforms a syscall number into a syscall name
func (s *Syscall) Decode(in []byte, _ config.Decoder) ([]byte, error) {
number, err := strconv.Atoi(string(in))
if err != nil {
Expand All @@ -21,7 +23,7 @@ func (s *Syscall) Decode(in []byte, _ config.Decoder) ([]byte, error) {
func resolveSyscall(number uint64) string {
if name, ok := syscalls[number]; ok {
return name
} else {
return fmt.Sprintf("unknown_syscall:%d", number)
}

return fmt.Sprintf("unknown_syscall:%d", number)
}
2 changes: 1 addition & 1 deletion decoder/uint.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
type UInt struct{}

// Decode transforms unsigned integers into their string values
func (u *UInt) Decode(in []byte, conf config.Decoder) ([]byte, error) {
func (u *UInt) Decode(in []byte, _ config.Decoder) ([]byte, error) {
byteOrder := util.GetHostByteOrder()

result := uint64(0)
Expand Down
4 changes: 2 additions & 2 deletions exporter/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/cloudflare/ebpf_exporter/v2/config"
)

func attachModule(module *libbpfgo.Module, cfg config.Config) (map[*libbpfgo.BPFProg]bool, error) {
func attachModule(module *libbpfgo.Module, cfg config.Config) map[*libbpfgo.BPFProg]bool {
attached := map[*libbpfgo.BPFProg]bool{}

iter := module.Iterator()
Expand All @@ -26,5 +26,5 @@ func attachModule(module *libbpfgo.Module, cfg config.Config) (map[*libbpfgo.BPF
}
}

return attached, nil
return attached
}
22 changes: 10 additions & 12 deletions exporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var percpuMapTypes = map[libbpfgo.MapType]struct{}{
type Exporter struct {
configs []config.Config
modules map[string]*libbpfgo.Module
perfEventArrayCollectors []*PerfEventArraySink
perfEventArrayCollectors []*perfEventArraySink
kaddrs map[string]uint64
enabledConfigsDesc *prometheus.Desc
programInfoDesc *prometheus.Desc
Expand Down Expand Up @@ -132,10 +132,7 @@ func (e *Exporter) Attach() error {
return fmt.Errorf("error loading bpf object from %q for config %q: %v", cfg.BPFPath, cfg.Name, err)
}

attachments, err := attachModule(module, cfg)
if err != nil {
return fmt.Errorf("failed to attach to config %q: %s", cfg.Name, err)
}
attachments := attachModule(module, cfg)

err = validateMaps(module, cfg)
if err != nil {
Expand All @@ -159,13 +156,14 @@ func (e *Exporter) passKaddrs(module *libbpfgo.Module, cfg config.Config) error
}

for _, kaddr := range cfg.Kaddrs {
if addr, ok := e.kaddrs[kaddr]; !ok {
addr, ok := e.kaddrs[kaddr]
if !ok {
return fmt.Errorf("error finding kaddr for %q", kaddr)
} else {
name := fmt.Sprintf("kaddr_%s", kaddr)
if err := module.InitGlobalVariable(name, uint64(addr)); err != nil {
return fmt.Errorf("error setting kaddr value for %q (const volatile %q) to 0x%x: %v", kaddr, name, addr, err)
}
}

name := fmt.Sprintf("kaddr_%s", kaddr)
if err := module.InitGlobalVariable(name, addr); err != nil {
return fmt.Errorf("error setting kaddr value for %q (const volatile %q) to 0x%x: %v", kaddr, name, addr, err)
}
}

Expand Down Expand Up @@ -227,7 +225,7 @@ func (e *Exporter) Describe(ch chan<- *prometheus.Desc) {

for _, counter := range cfg.Metrics.Counters {
if counter.PerfEventArray {
perfSink := NewPerfEventArraySink(e.decoders, e.modules[cfg.Name], counter)
perfSink := newPerfEventArraySink(e.decoders, e.modules[cfg.Name], counter)
e.perfEventArrayCollectors = append(e.perfEventArrayCollectors, perfSink)
}

Expand Down
19 changes: 9 additions & 10 deletions exporter/perf_event_array.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@ import (
"github.com/prometheus/client_golang/prometheus"
)

type PerfEventArraySink struct {
type perfEventArraySink struct {
counterConfig config.Counter
counterVec *prometheus.CounterVec
dropCounter prometheus.Counter
}

func NewPerfEventArraySink(decoders *decoder.Set, module *libbpfgo.Module, counterConfig config.Counter) *PerfEventArraySink {
func newPerfEventArraySink(decoders *decoder.Set, module *libbpfgo.Module, counterConfig config.Counter) *perfEventArraySink {
var (
receiveCh = make(chan []byte)
lostCh = make(chan uint64)
)

sink := &PerfEventArraySink{
sink := &perfEventArraySink{
counterConfig: counterConfig,
dropCounter: createDropCounterForPerfMap(counterConfig),
}
Expand All @@ -34,7 +34,7 @@ func NewPerfEventArraySink(decoders *decoder.Set, module *libbpfgo.Module, count
log.Fatalf("Can't init PerfBuf: %s", err)
}

go func(sink *PerfEventArraySink, receiveCh <-chan []byte) {
go func(sink *perfEventArraySink, receiveCh <-chan []byte) {
for rawBytes := range receiveCh {
// https://github.com/cilium/ebpf/pull/94#discussion_r425823371
// https://lore.kernel.org/patchwork/patch/1244339/
Expand All @@ -53,17 +53,16 @@ func NewPerfEventArraySink(decoders *decoder.Set, module *libbpfgo.Module, count
}

sink.counterVec.WithLabelValues(labelValues...).Inc()

}
}(sink, receiveCh)

go func(sink *PerfEventArraySink, lostCh <-chan uint64) {
go func(sink *perfEventArraySink, lostCh <-chan uint64) {
for droppedEvents := range lostCh {
sink.dropCounter.Add(float64(droppedEvents))
}
}(sink, lostCh)

go func(sink *PerfEventArraySink) {
go func(sink *perfEventArraySink) {
flushDuration := time.Hour
if sink.counterConfig.FlushInterval > 0 {
flushDuration = sink.counterConfig.FlushInterval
Expand All @@ -82,15 +81,15 @@ func NewPerfEventArraySink(decoders *decoder.Set, module *libbpfgo.Module, count
return sink
}

func (s *PerfEventArraySink) Collect(ch chan<- prometheus.Metric) {
func (s *perfEventArraySink) Collect(ch chan<- prometheus.Metric) {
s.counterVec.Collect(ch)
}

func (s *PerfEventArraySink) Describe(ch chan<- *prometheus.Desc) {
func (s *perfEventArraySink) Describe(ch chan<- *prometheus.Desc) {
s.counterVec.Describe(ch)
}

func (s *PerfEventArraySink) resetCounterVec() {
func (s *perfEventArraySink) resetCounterVec() {
s.counterVec = createCounterVecForPerfMap(s.counterConfig, labelNamesFromCounterConfig(s.counterConfig))
}

Expand Down
2 changes: 1 addition & 1 deletion exporter/perf_event_cb.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// These callbacks need to be in a separate file to avoid multiple definitions error

//export attachPerfEventCallback
func attachPerfEventCallback(prog unsafe.Pointer, cookie C.long, link *unsafe.Pointer) C.int {
func attachPerfEventCallback(prog unsafe.Pointer, _ C.long, link *unsafe.Pointer) C.int {
program := (*C.struct_bpf_program)(prog)

links, err := attachPerfEvent(program)
Expand Down
2 changes: 1 addition & 1 deletion exporter/xdp.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func attachXDP(prog *C.struct_bpf_program) ([]*C.struct_bpf_link, error) {

link, err := C.bpf_program__attach_xdp(prog, C.int(iface.Index))
if link == nil {
return nil, fmt.Errorf("failed to attach xdp on device %q for program %s: %v\n", device, name, err)
return nil, fmt.Errorf("failed to attach xdp on device %q for program %s: %v", device, name, err)
}

return []*C.struct_bpf_link{link}, nil
Expand Down
2 changes: 1 addition & 1 deletion exporter/xdp_cb.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// These callbacks need to be in a separate file to avoid multiple definitions error

//export attachXDPCallback
func attachXDPCallback(prog unsafe.Pointer, cookie C.long, link *unsafe.Pointer) C.int {
func attachXDPCallback(prog unsafe.Pointer, _ C.long, link *unsafe.Pointer) C.int {
program := (*C.struct_bpf_program)(prog)

links, err := attachXDP(program)
Expand Down
Loading

0 comments on commit c7eaf49

Please sign in to comment.