Fix linter complaints and improve exporter setup.

.github/workflows/golangci-lint.yml

@@ -21,8 +21,12 @@ jobs:
       - uses: actions/checkout@v4
       - uses: golangci/golangci-lint-action@v3
         with:
-          # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
+          # Required: the version of golangci-lint is required and must be
+          # specified without patch version: we always use the latest patch
+          # version.
           version: v1.52
 
-          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # Optional: show only new issues if it's a pull request. The default
+          # value is `false`.  Be careful upgrading because this won't show
+          # existing lints.
           only-new-issues: true

.golangci.yml

@@ -52,7 +52,7 @@ linters:
     - interfacer # deprecated
     - lll # go says long lines are ok, and this is trivially automatable
     - maligned # deprecated
-    - musttag # don't tell me how to live
+    - musttag # don't agree with the premise
     - nakedret    # weird thing to report on
     - nestif # cognitive complexity
     - nlreturn    # Not a fan of this one, looks messy

Makefile

@@ -104,7 +104,7 @@ veryclean: clean depclean
 
 # This version should match the one in .github/workflows/golangci-lint.yml
 GOLANGCILINT_VERSION=$(shell grep 'version: v' .github/workflows/golangci-lint.yml | cut -f2 -d: | tr -d ' ')
-#GOLANGCILINT_VERSION=v1.52
+
 # lint
 .PHONY: lint
 lint:  $(GOFILES) $(GOGENFILES) $(GOTESTFILES)

internal/exporter/export.go

@@ -30,15 +30,16 @@ var (
 
 // Exporter manages the export of metrics to passive and active collectors.
 type Exporter struct {
-	ctx           context.Context
-	wg            sync.WaitGroup
-	store         *metrics.Store
-	pushInterval  time.Duration
-	hostname      string
-	omitProgLabel bool
-	emitTimestamp bool
-	pushTargets   []pushOptions
-	initDone      chan struct{}
+	ctx            context.Context
+	wg             sync.WaitGroup
+	store          *metrics.Store
+	pushInterval   time.Duration
+	hostname       string
+	omitProgLabel  bool
+	emitTimestamp  bool
+	exportDisabled bool
+	pushTargets    []pushOptions
+	initDone       chan struct{}
 }
 
 // Option configures a new Exporter.
@@ -75,6 +76,13 @@ func PushInterval(opt time.Duration) Option {
 	}
 }
 
+func DisableExport() Option {
+	return func(e *Exporter) error {
+		e.exportDisabled = true
+		return nil
+	}
+}
+
 // New creates a new Exporter.
 func New(ctx context.Context, wg *sync.WaitGroup, store *metrics.Store, options ...Option) (*Exporter, error) {
 	if store == nil {
@@ -112,13 +120,15 @@ func New(ctx context.Context, wg *sync.WaitGroup, store *metrics.Store, options
 	}
 	e.StartMetricPush()
 
-	// This routine manages shutdown of the Exporter.  TODO(jaq): This doesn't
-	// happen before mtail returns because of how context cancellation is set
-	// up..  How can we tie this shutdown in before mtail exits?  Should
-	// exporter be merged with httpserver?
+	wg.Add(1)
+	// This routine manages shutdown of the Exporter.
 	go func() {
+		defer wg.Done()
 		<-e.initDone
-		<-e.ctx.Done()
+		// Wait for the context to be completed before waiting for subroutines.
+		if !e.exportDisabled {
+			<-e.ctx.Done()
+		}
 		e.wg.Wait()
 	}()
 	return e, nil
@@ -213,6 +223,10 @@ func (e *Exporter) PushMetrics() {
 
 // StartMetricPush pushes metrics to the configured services each interval.
 func (e *Exporter) StartMetricPush() {
+	if e.exportDisabled {
+		glog.Info("Export loop disabled.")
+		return
+	}
 	if len(e.pushTargets) == 0 {
 		return
 	}

internal/exporter/graphite.go

@@ -53,7 +53,7 @@ func (e *Exporter) HandleGraphite(w http.ResponseWriter, r *http.Request) {
 
 // metricToGraphite encodes a metric in the graphite text protocol format.  The
 // metric lock is held before entering this function.
-func metricToGraphite(hostname string, m *metrics.Metric, l *metrics.LabelSet, _ time.Duration) string {
+func metricToGraphite(_ string, m *metrics.Metric, l *metrics.LabelSet, _ time.Duration) string {
 	var b strings.Builder
 	if m.Kind == metrics.Histogram && m.Type == metrics.Buckets {
 		d := m.LabelValues[0].Value

internal/exporter/json.go

@@ -14,7 +14,7 @@ import (
 var exportJSONErrors = expvar.NewInt("exporter_json_errors")
 
 // HandleJSON exports the metrics in JSON format via HTTP.
-func (e *Exporter) HandleJSON(w http.ResponseWriter, r *http.Request) {
+func (e *Exporter) HandleJSON(w http.ResponseWriter, _ *http.Request) {
 	b, err := json.MarshalIndent(e.store, "", "  ")
 	if err != nil {
 		exportJSONErrors.Add(1)

internal/exporter/statsd.go

@@ -24,7 +24,7 @@ var (
 
 // metricToStatsd encodes a metric in the statsd text protocol format.  The
 // metric lock is held before entering this function.
-func metricToStatsd(hostname string, m *metrics.Metric, l *metrics.LabelSet, _ time.Duration) string {
+func metricToStatsd(_ string, m *metrics.Metric, l *metrics.LabelSet, _ time.Duration) string {
 	var t string
 	switch m.Kind {
 	case metrics.Counter:

internal/metrics/datum/datum.go

@@ -94,7 +94,7 @@ func MakeString(v string, ts time.Time) Datum {
 
 // MakeBuckets creates a new bucket datum with the provided list of ranges and
 // timestamp.  If no +inf bucket is provided, one is created.
-func MakeBuckets(buckets []Range, ts time.Time) Datum {
+func MakeBuckets(buckets []Range, _ time.Time) Datum {
 	d := &Buckets{}
 	seenInf := false
 	highest := 0.0

internal/metrics/metric.go

@@ -64,7 +64,7 @@ func (m Kind) String() string {
 }
 
 // Generate implements the quick.Generator interface for Kind.
-func (Kind) Generate(rand *rand.Rand, size int) reflect.Value {
+func (Kind) Generate(rand *rand.Rand, _ int) reflect.Value {
 	return reflect.ValueOf(Kind(rand.Intn(int(endKind))))
 }
 

internal/metrics/type.go

@@ -39,6 +39,6 @@ func (t Type) String() string {
 }
 
 // Generate implements the quick.Generator interface for Type.
-func (Type) Generate(rand *rand.Rand, size int) reflect.Value {
+func (Type) Generate(rand *rand.Rand, _ int) reflect.Value {
 	return reflect.ValueOf(Type(rand.Intn(int(endType))))
 }

internal/mtail/httpstatus.go

@@ -31,7 +31,7 @@ const statusTemplateEnd = `
 
 // ServeHTTP satisfies the http.Handler interface, and is used to serve the
 // root page of mtail for online status reporting.
-func (m *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+func (m *Server) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
 	t, err := template.New("status").Parse(statusTemplate)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -77,7 +77,7 @@ func (m *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 // FaviconHandler is used to serve up the favicon.ico for mtail's http server.
-func FaviconHandler(w http.ResponseWriter, r *http.Request) {
+func FaviconHandler(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "image/x-icon")
 	w.Header().Set("Cache-Control", "public, max-age=7776000")
 	if _, err := w.Write(logoFavicon); err != nil {

internal/mtail/mtail.go

@@ -54,6 +54,9 @@ type Server struct {
 	httpInfoEndpoints  bool   // if set, mtail will enable info endpoints for progz and varz
 }
 
+// We can only copy the build info once to the version library.  Protects tests from data races.
+var buildInfoOnce sync.Once
+
 // initRuntime constructs a new runtime and performs the initial load of program files in the program directory.
 func (m *Server) initRuntime() (err error) {
 	m.r, err = runtime.New(m.lines, &m.wg, m.programPath, m.store, m.rOpts...)
@@ -62,22 +65,20 @@ func (m *Server) initRuntime() (err error) {
 
 // initExporter sets up an Exporter for this Server.
 func (m *Server) initExporter() (err error) {
-	if m.oneShot {
-		// This is a hack to avoid a race in test, but assume that in oneshot
-		// mode we don't want to export anything.
-		return nil
-	}
 	m.e, err = exporter.New(m.ctx, &m.wg, m.store, m.eOpts...)
 	if err != nil {
 		return err
 	}
 	m.reg.MustRegister(m.e)
 
 	// Create mtail_build_info metric.
-	version.Branch = m.buildInfo.Branch
-	version.Version = m.buildInfo.Version
-	version.Revision = m.buildInfo.Revision
+	buildInfoOnce.Do(func() {
+		version.Branch = m.buildInfo.Branch
+		version.Version = m.buildInfo.Version
+		version.Revision = m.buildInfo.Revision
+	})
 	m.reg.MustRegister(version.NewCollector("mtail"))
+
 	return nil
 }
 

internal/mtail/options.go

@@ -164,6 +164,7 @@ var OneShot = &niladicOption{
 	func(m *Server) error {
 		m.rOpts = append(m.rOpts, runtime.ErrorsAbort())
 		m.tOpts = append(m.tOpts, tailer.OneShot)
+		m.eOpts = append(m.eOpts, exporter.DisableExport())
 		m.oneShot = true
 		return nil
 	},
@@ -173,6 +174,7 @@ var OneShot = &niladicOption{
 var CompileOnly = &niladicOption{
 	func(m *Server) error {
 		m.rOpts = append(m.rOpts, runtime.CompileOnly())
+		m.eOpts = append(m.eOpts, exporter.DisableExport())
 		m.compileOnly = true
 		return nil
 	},
@@ -186,7 +188,7 @@ var DumpAst = &niladicOption{
 	},
 }
 
-// DumpAstTypes instructs the Server's copmiler to print the AST after type checking.
+// DumpAstTypes instructs the Server's compiler to print the AST after type checking.
 var DumpAstTypes = &niladicOption{
 	func(m *Server) error {
 		m.rOpts = append(m.rOpts, runtime.DumpAstTypes())
@@ -261,7 +263,7 @@ var LogRuntimeErrors = &niladicOption{
 // JaegerReporter creates a new jaeger reporter that sends to the given Jaeger endpoint address.
 type JaegerReporter string
 
-func (opt JaegerReporter) apply(m *Server) error {
+func (opt JaegerReporter) apply(_ *Server) error {
 	je, err := jaeger.NewExporter(jaeger.Options{
 		CollectorEndpoint: string(opt),
 		Process: jaeger.Process{

internal/runtime/compiler/symbol/symtab_test.go

@@ -25,7 +25,7 @@ func TestInsertLookup(t *testing.T) {
 }
 
 // Generate implements the quick.Generator interface for SymbolKind.
-func (Kind) Generate(rand *rand.Rand, size int) reflect.Value {
+func (Kind) Generate(rand *rand.Rand, _ int) reflect.Value {
 	return reflect.ValueOf(Kind(rand.Intn(int(endSymbol))))
 }
 

internal/runtime/vm/vm.go

@@ -959,7 +959,7 @@ func (v *VM) execute(t *thread, i code.Instr) {
 
 // ProcessLogLine handles the incoming lines by running a fetch-execute cycle
 // on the VM bytecode with the line as input to the program, until termination.
-func (v *VM) ProcessLogLine(ctx context.Context, line *logline.LogLine) {
+func (v *VM) ProcessLogLine(_ context.Context, line *logline.LogLine) {
 	start := time.Now()
 	defer func() {
 		LineProcessingDurations.WithLabelValues(v.name).Observe(time.Since(start).Seconds())

internal/tailer/logstream/pipestream.go

@@ -41,7 +41,7 @@ func (ps *pipeStream) LastReadTime() time.Time {
 	return ps.lastReadTime
 }
 
-func (ps *pipeStream) stream(ctx context.Context, wg *sync.WaitGroup, waker waker.Waker, fi os.FileInfo) error {
+func (ps *pipeStream) stream(ctx context.Context, wg *sync.WaitGroup, waker waker.Waker, _ os.FileInfo) error {
 	// Open in nonblocking mode because the write end of the pipe may not have started yet.
 	fd, err := os.OpenFile(ps.pathname, os.O_RDONLY|syscall.O_NONBLOCK, 0o600)
 	if err != nil {