[chore] Fix lint in pkg/instrumentors (#116)

* Fix lint in instrumentors * Apply suggestions from code review Co-authored-by: Robert Pająk <[email protected]> * Update pkg/instrumentors/allocator/allocator_linux.go --------- Co-authored-by: Robert Pająk <[email protected]>
open-telemetry · Apr 27, 2023 · 4101e5d · 4101e5d
1 parent fac24ae
commit 4101e5d
Show file tree

Hide file tree

Showing 12 changed files with 63 additions and 36 deletions.
diff --git a/pkg/instrumentors/allocator/allocator_linux.go b/pkg/instrumentors/allocator/allocator_linux.go
@@ -17,18 +17,22 @@ package allocator
 import (
 	"os"
 
+	"golang.org/x/sys/unix"
+
 	"go.opentelemetry.io/auto/pkg/instrumentors/bpffs"
 	"go.opentelemetry.io/auto/pkg/instrumentors/context"
 	"go.opentelemetry.io/auto/pkg/log"
-	"golang.org/x/sys/unix"
 )
 
+// Allocator handles the allocation of the BPF file-system.
 type Allocator struct{}
 
+// New returns a new [Allocator].
 func New() *Allocator {
 	return &Allocator{}
 }
 
+// Load loads the BPF file-system.
 func (a *Allocator) Load(ctx *context.InstrumentorContext) error {
 	logger := log.Logger.WithName("allocator")
 	if ctx.TargetDetails.AllocationDetails != nil {
@@ -38,25 +42,24 @@ func (a *Allocator) Load(ctx *context.InstrumentorContext) error {
 	}
 	logger.V(0).Info("Loading allocator")
 
-	err := a.mountBpfFS()
+	err := a.mountBPFFs()
 	if err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (a *Allocator) mountBpfFS() error {
-	_, err := os.Stat(bpffs.BpfFsPath)
+func (a *Allocator) mountBPFFs() error {
+	_, err := os.Stat(bpffs.BPFFsPath)
 	if err != nil {
-		if os.IsNotExist(err) {
-			if err := os.MkdirAll(bpffs.BpfFsPath, 0755); err != nil {
-				return err
-			}
-		} else {
+		if !os.IsNotExist(err) {
+			return err
+		}
+		if err := os.MkdirAll(bpffs.BPFFsPath, 0755); err != nil {
 			return err
 		}
 	}
 
-	return unix.Mount(bpffs.BpfFsPath, bpffs.BpfFsPath, "bpf", 0, "")
+	return unix.Mount(bpffs.BPFFsPath, bpffs.BPFFsPath, "bpf", 0, "")
 }
diff --git a/pkg/instrumentors/api.go b/pkg/instrumentors/api.go
@@ -19,10 +19,21 @@ import (
 	"go.opentelemetry.io/auto/pkg/instrumentors/events"
 )
 
+// Instrumentor provides instrumentation for a Go package.
 type Instrumentor interface {
+	// LibraryName returns the package name being instrumented.
 	LibraryName() string
+
+	// FuncNames returns the fully-qualified function names that are
+	// instrumented.
 	FuncNames() []string
+
+	// Load loads all instrumentation offsets.
 	Load(ctx *context.InstrumentorContext) error
+
+	// Run runs the events processing loop.
 	Run(eventsChan chan<- *events.Event)
+
+	// Close stops the Instrumentor.
 	Close()
 }
diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go
@@ -42,7 +42,7 @@ type HttpEvent struct {
 	EndTime     uint64
 	Method      [100]byte
 	Path        [100]byte
-	SpanContext context.EbpfSpanContext
+	SpanContext context.EBPFSpanContext
 }
 
 type gorillaMuxInstrumentor struct {
@@ -95,7 +95,7 @@ func (g *gorillaMuxInstrumentor) Load(ctx *context.InstrumentorContext) error {
 	g.bpfObjects = &bpfObjects{}
 	err = spec.LoadAndAssign(g.bpfObjects, &ebpf.CollectionOptions{
 		Maps: ebpf.MapOptions{
-			PinPath: bpffs.BpfFsPath,
+			PinPath: bpffs.BPFFsPath,
 		},
 	})
 	if err != nil {

diff --git a/pkg/instrumentors/bpf/google/golang/org/grpc/probe.go b/pkg/instrumentors/bpf/google/golang/org/grpc/probe.go
@@ -43,8 +43,8 @@ type GrpcEvent struct {
 	EndTime           uint64
 	Method            [50]byte
 	Target            [50]byte
-	SpanContext       context.EbpfSpanContext
-	ParentSpanContext context.EbpfSpanContext
+	SpanContext       context.EBPFSpanContext
+	ParentSpanContext context.EBPFSpanContext
 }
 
 type grpcInstrumentor struct {
@@ -88,7 +88,7 @@ func (g *grpcInstrumentor) Load(ctx *context.InstrumentorContext) error {
 	g.bpfObjects = &bpfObjects{}
 	err = spec.LoadAndAssign(g.bpfObjects, &ebpf.CollectionOptions{
 		Maps: ebpf.MapOptions{
-			PinPath: bpffs.BpfFsPath,
+			PinPath: bpffs.BPFFsPath,
 		},
 	})
 	if err != nil {

diff --git a/pkg/instrumentors/bpf/google/golang/org/grpc/server/probe.go b/pkg/instrumentors/bpf/google/golang/org/grpc/server/probe.go
@@ -41,8 +41,8 @@ type GrpcEvent struct {
 	StartTime         uint64
 	EndTime           uint64
 	Method            [100]byte
-	SpanContext       context.EbpfSpanContext
-	ParentSpanContext context.EbpfSpanContext
+	SpanContext       context.EBPFSpanContext
+	ParentSpanContext context.EBPFSpanContext
 }
 
 type grpcServerInstrumentor struct {
@@ -107,7 +107,7 @@ func (g *grpcServerInstrumentor) Load(ctx *context.InstrumentorContext) error {
 	g.bpfObjects = &bpfObjects{}
 	err = spec.LoadAndAssign(g.bpfObjects, &ebpf.CollectionOptions{
 		Maps: ebpf.MapOptions{
-			PinPath: bpffs.BpfFsPath,
+			PinPath: bpffs.BPFFsPath,
 		},
 	})
 	if err != nil {

diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go
@@ -42,7 +42,7 @@ type HttpEvent struct {
 	EndTime     uint64
 	Method      [6]byte
 	Path        [100]byte
-	SpanContext context.EbpfSpanContext
+	SpanContext context.EBPFSpanContext
 }
 
 type httpServerInstrumentor struct {
@@ -90,7 +90,7 @@ func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error {
 	h.bpfObjects = &bpfObjects{}
 	err = spec.LoadAndAssign(h.bpfObjects, &ebpf.CollectionOptions{
 		Maps: ebpf.MapOptions{
-			PinPath: bpffs.BpfFsPath,
+			PinPath: bpffs.BPFFsPath,
 		},
 	})
 	if err != nil {

diff --git a/pkg/instrumentors/bpffs/bpfsfs.go b/pkg/instrumentors/bpffs/bpfsfs.go
@@ -14,6 +14,5 @@
 
 package bpffs
 
-const (
-	BpfFsPath = "/sys/fs/bpf"
-)
+// BPFFsPath is the system path to the BPF file-system.
+const BPFFsPath = "/sys/fs/bpf"
diff --git a/pkg/instrumentors/context/inst_context.go b/pkg/instrumentors/context/inst_context.go
@@ -16,10 +16,12 @@ package context
 
 import (
 	"github.com/cilium/ebpf/link"
+
 	"go.opentelemetry.io/auto/pkg/inject"
 	"go.opentelemetry.io/auto/pkg/process"
 )
 
+// InstrumentorContext holds the state of the auto-instrumentation system.
 type InstrumentorContext struct {
 	TargetDetails *process.TargetDetails
 	Executable    *link.Executable

diff --git a/pkg/instrumentors/context/span_context.go b/pkg/instrumentors/context/span_context.go
@@ -16,7 +16,9 @@ package context
 
 import "go.opentelemetry.io/otel/trace"
 
-type EbpfSpanContext struct {
+// EBPFSpanContext is the the span context representation within the eBPF
+// instrumentation system.
+type EBPFSpanContext struct {
 	TraceID trace.TraceID
 	SpanID  trace.SpanID
 }
diff --git a/pkg/instrumentors/events/event.go b/pkg/instrumentors/events/event.go
@@ -19,6 +19,7 @@ import (
 	"go.opentelemetry.io/otel/trace"
 )
 
+// Event is a telemetry event that happens within an instrumented package.
 type Event struct {
 	Library           string
 	Name              string

diff --git a/pkg/instrumentors/manager.go b/pkg/instrumentors/manager.go
@@ -28,16 +28,18 @@ import (
 	"go.opentelemetry.io/auto/pkg/process"
 )
 
-type instrumentorsManager struct {
+// Manager handles the management of [Instrumentor] instances.
+type Manager struct {
 	instrumentors  map[string]Instrumentor
 	done           chan bool
 	incomingEvents chan *events.Event
 	otelController *opentelemetry.Controller
 	allocator      *allocator.Allocator
 }
 
-func NewManager(otelController *opentelemetry.Controller) (*instrumentorsManager, error) {
-	m := &instrumentorsManager{
+// NewManager returns a new [Manager].
+func NewManager(otelController *opentelemetry.Controller) (*Manager, error) {
+	m := &Manager{
 		instrumentors:  make(map[string]Instrumentor),
 		done:           make(chan bool, 1),
 		incomingEvents: make(chan *events.Event),
@@ -53,7 +55,7 @@ func NewManager(otelController *opentelemetry.Controller) (*instrumentorsManager
 	return m, nil
 }
 
-func (m *instrumentorsManager) registerInstrumentor(instrumentor Instrumentor) error {
+func (m *Manager) registerInstrumentor(instrumentor Instrumentor) error {
 	if _, exists := m.instrumentors[instrumentor.LibraryName()]; exists {
 		return fmt.Errorf("library %s registered twice, aborting", instrumentor.LibraryName())
 	}
@@ -62,7 +64,9 @@ func (m *instrumentorsManager) registerInstrumentor(instrumentor Instrumentor) e
 	return nil
 }
 
-func (m *instrumentorsManager) GetRelevantFuncs() map[string]interface{} {
+// GetRelevantFuncs returns the instrumented functions for all managed
+// Instrumentors.
+func (m *Manager) GetRelevantFuncs() map[string]interface{} {
 	funcsMap := make(map[string]interface{})
 	for _, i := range m.instrumentors {
 		for _, f := range i.FuncNames() {
@@ -73,7 +77,9 @@ func (m *instrumentorsManager) GetRelevantFuncs() map[string]interface{} {
 	return funcsMap
 }
 
-func (m *instrumentorsManager) FilterUnusedInstrumentors(target *process.TargetDetails) {
+// FilterUnusedInstrumentors filterers Instrumentors whose functions are
+// already instrumented out of the Manager.
+func (m *Manager) FilterUnusedInstrumentors(target *process.TargetDetails) {
 	existingFuncMap := make(map[string]interface{})
 	for _, f := range target.Functions {
 		existingFuncMap[f.Name] = nil
@@ -95,7 +101,7 @@ func (m *instrumentorsManager) FilterUnusedInstrumentors(target *process.TargetD
 	}
 }
 
-func registerInstrumentors(m *instrumentorsManager) error {
+func registerInstrumentors(m *Manager) error {
 	insts := []Instrumentor{
 		grpc.New(),
 		grpcServer.New(),

diff --git a/pkg/instrumentors/runner.go b/pkg/instrumentors/runner.go
@@ -19,15 +19,17 @@ import (
 
 	"github.com/cilium/ebpf/link"
 	"github.com/cilium/ebpf/rlimit"
+
 	"go.opentelemetry.io/auto/pkg/inject"
 	"go.opentelemetry.io/auto/pkg/instrumentors/context"
 	"go.opentelemetry.io/auto/pkg/log"
 	"go.opentelemetry.io/auto/pkg/process"
 )
 
-func (m *instrumentorsManager) Run(target *process.TargetDetails) error {
+// Run runs the event processing loop for all managed Instrumentors.
+func (m *Manager) Run(target *process.TargetDetails) error {
 	if len(m.instrumentors) == 0 {
-		log.Logger.V(0).Info("there are no avilable instrumentations for target process")
+		log.Logger.V(0).Info("there are no available instrumentations for target process")
 		return nil
 	}
 
@@ -52,7 +54,7 @@ func (m *instrumentorsManager) Run(target *process.TargetDetails) error {
 	}
 }
 
-func (m *instrumentorsManager) load(target *process.TargetDetails) error {
+func (m *Manager) load(target *process.TargetDetails) error {
 	// Allow the current process to lock memory for eBPF resources.
 	if err := rlimit.RemoveMemlock(); err != nil {
 		return err
@@ -93,13 +95,14 @@ func (m *instrumentorsManager) load(target *process.TargetDetails) error {
 	return nil
 }
 
-func (m *instrumentorsManager) cleanup() {
+func (m *Manager) cleanup() {
 	close(m.incomingEvents)
 	for _, i := range m.instrumentors {
 		i.Close()
 	}
 }
 
-func (m *instrumentorsManager) Close() {
+// Close closes m.
+func (m *Manager) Close() {
 	m.done <- true
 }