Make code more readable based on PR reviews

cmd/run.go

@@ -153,15 +153,7 @@ func (c *cmdRun) run(cmd *cobra.Command, args []string) error {
 
 	// We do this here so we can get any output URLs below.
 	initBar.Modify(pb.WithConstProgress(0, "Starting outputs"))
-	// TODO: re-enable the code below
-	/*
-		outputManager := output.NewManager(outputs, logger, func(err error) {
-			if err != nil {
-				logger.WithError(err).Error("Received error to stop from output")
-			}
-			runCancel()
-		})
-	*/
+	// TODO: directly create the MutputManager here, not in the Engine
 	err = engine.OutputManager.StartOutputs()
 	if err != nil {
 		return err

core/engine.go

@@ -227,7 +227,7 @@ func (e *Engine) startBackgroundProcesses(
 			for {
 				select {
 				case <-ticker.C:
-					thresholdsTainted, shouldAbort := e.MetricsEngine.ProcessThresholds()
+					thresholdsTainted, shouldAbort := e.MetricsEngine.EvaluateThresholds()
 					e.thresholdsTaintedLock.Lock()
 					e.thresholdsTainted = thresholdsTainted
 					e.thresholdsTaintedLock.Unlock()
@@ -261,7 +261,7 @@ func (e *Engine) processMetrics(globalCtx context.Context, processMetricsAfterRu
 
 		if !e.runtimeOptions.NoThresholds.Bool {
 			// Process the thresholds one final time
-			thresholdsTainted, _ := e.MetricsEngine.ProcessThresholds()
+			thresholdsTainted, _ := e.MetricsEngine.EvaluateThresholds()
 			e.thresholdsTaintedLock.Lock()
 			e.thresholdsTainted = thresholdsTainted
 			e.thresholdsTaintedLock.Unlock()
@@ -300,7 +300,7 @@ func (e *Engine) processMetrics(globalCtx context.Context, processMetricsAfterRu
 			if !e.runtimeOptions.NoThresholds.Bool {
 				// Ensure the ingester flushes any buffered metrics
 				_ = e.ingester.Stop()
-				thresholdsTainted, _ := e.MetricsEngine.ProcessThresholds()
+				thresholdsTainted, _ := e.MetricsEngine.EvaluateThresholds()
 				e.thresholdsTaintedLock.Lock()
 				e.thresholdsTainted = thresholdsTainted
 				e.thresholdsTaintedLock.Unlock()

core/local/local.go

@@ -54,7 +54,7 @@ type ExecutionScheduler struct {
 
 	// TODO: remove these when we don't have separate Init() and Run() methods
 	// and can use a context + a WaitGroup (or something like that)
-	stopVusEmission, vusEmissionStopped chan struct{}
+	stopVUsEmission, vusEmissionStopped chan struct{}
 }
 
 // Check to see if we implement the lib.ExecutionScheduler interface
@@ -119,7 +119,7 @@ func NewExecutionScheduler(
 		maxPossibleVUs:  maxPossibleVUs,
 		state:           executionState,
 
-		stopVusEmission:    make(chan struct{}),
+		stopVUsEmission:    make(chan struct{}),
 		vusEmissionStopped: make(chan struct{}),
 	}, nil
 }
@@ -273,7 +273,7 @@ func (e *ExecutionScheduler) emitVUsAndVUsMax(ctx context.Context, out chan<- st
 				emitMetrics()
 			case <-ctx.Done():
 				return
-			case <-e.stopVusEmission:
+			case <-e.stopVUsEmission:
 				return
 			}
 		}
@@ -399,7 +399,7 @@ func (e *ExecutionScheduler) runExecutor(
 //nolint:funlen
 func (e *ExecutionScheduler) Run(globalCtx, runCtx context.Context, engineOut chan<- stats.SampleContainer) error {
 	defer func() {
-		close(e.stopVusEmission)
+		close(e.stopVUsEmission)
 		<-e.vusEmissionStopped
 	}()
 

metrics/engine/engine.go

@@ -71,7 +71,7 @@ func (me *MetricsEngine) GetIngester() output.Output {
 	}
 }
 
-func (me *MetricsEngine) getOrInitPotentialSubmetric(name string) (*stats.Metric, error) {
+func (me *MetricsEngine) getThresholdMetricOrSubmetric(name string) (*stats.Metric, error) {
 	// TODO: replace with strings.Cut after Go 1.18
 	nameParts := strings.SplitN(name, "{", 2)
 
@@ -83,10 +83,11 @@ func (me *MetricsEngine) getOrInitPotentialSubmetric(name string) (*stats.Metric
 		return metric, nil
 	}
 
-	if nameParts[1][len(nameParts[1])-1] != '}' {
+	submetricDefinition := nameParts[1]
+	if submetricDefinition[len(submetricDefinition)-1] != '}' {
 		return nil, fmt.Errorf("missing ending bracket, sub-metric format needs to be 'metric{key:value}'")
 	}
-	sm, err := metric.AddSubmetric(nameParts[1][:len(nameParts[1])-1])
+	sm, err := metric.AddSubmetric(submetricDefinition[:len(submetricDefinition)-1])
 	if err != nil {
 		return nil, err
 	}
@@ -102,7 +103,7 @@ func (me *MetricsEngine) markObserved(metric *stats.Metric) {
 
 func (me *MetricsEngine) initSubMetricsAndThresholds() error {
 	for metricName, thresholds := range me.options.Thresholds {
-		metric, err := me.getOrInitPotentialSubmetric(metricName)
+		metric, err := me.getThresholdMetricOrSubmetric(metricName)
 
 		if me.runtimeOptions.NoThresholds.Bool {
 			if err != nil {
@@ -130,7 +131,7 @@ func (me *MetricsEngine) initSubMetricsAndThresholds() error {
 	// TODO: refactor out of here when https://github.com/grafana/k6/issues/1321
 	// lands and there is a better way to enable a metric with tag
 	if me.options.SystemTags.Has(stats.TagExpectedResponse) {
-		_, err := me.getOrInitPotentialSubmetric("http_req_duration{expected_response:true}")
+		_, err := me.getThresholdMetricOrSubmetric("http_req_duration{expected_response:true}")
 		if err != nil {
 			return err // shouldn't happen, but ¯\_(ツ)_/¯
 		}
@@ -142,7 +143,7 @@ func (me *MetricsEngine) initSubMetricsAndThresholds() error {
 // ProcessThresholds processes all of the thresholds.
 //
 // TODO: refactor, make private, optimize
-func (me *MetricsEngine) ProcessThresholds() (thresholdsTainted, shouldAbort bool) {
+func (me *MetricsEngine) EvaluateThresholds() (thresholdsTainted, shouldAbort bool) {
 	me.MetricsLock.Lock()
 	defer me.MetricsLock.Unlock()
 
@@ -154,19 +155,20 @@ func (me *MetricsEngine) ProcessThresholds() (thresholdsTainted, shouldAbort boo
 		}
 		m.Tainted = null.BoolFrom(false)
 
-		me.logger.WithField("m", m.Name).Debug("running thresholds")
+		me.logger.WithField("metric_name", m.Name).Debug("running thresholds")
 		succ, err := m.Thresholds.Run(m.Sink, t)
 		if err != nil {
-			me.logger.WithField("m", m.Name).WithError(err).Error("Threshold error")
+			me.logger.WithField("metric_name", m.Name).WithError(err).Error("Threshold error")
 			continue
 		}
-		if !succ {
-			me.logger.WithField("m", m.Name).Debug("Thresholds failed")
-			m.Tainted = null.BoolFrom(true)
-			thresholdsTainted = true
-			if m.Thresholds.Abort {
-				shouldAbort = true
-			}
+		if succ {
+			continue // threshold passed
+		}
+		me.logger.WithField("metric_name", m.Name).Debug("Thresholds failed")
+		m.Tainted = null.BoolFrom(true)
+		thresholdsTainted = true
+		if m.Thresholds.Abort {
+			shouldAbort = true
 		}
 	}