Refactor group and check to not be tightly coupled

api/v1/group_routes.go

@@ -6,7 +6,7 @@ import (
 )
 
 func handleGetGroups(cs *ControlSurface, rw http.ResponseWriter, _ *http.Request) {
-	root := NewGroup(cs.RunState.Runner.GetDefaultGroup(), nil)
+	root := NewGroup(cs.RunState.GroupSummary.Group(), nil)
 	groups := FlattenGroup(root)
 
 	data, err := json.Marshal(newGroupsJSONAPI(groups))
@@ -18,7 +18,7 @@ func handleGetGroups(cs *ControlSurface, rw http.ResponseWriter, _ *http.Request
 }
 
 func handleGetGroup(cs *ControlSurface, rw http.ResponseWriter, _ *http.Request, id string) {
-	root := NewGroup(cs.RunState.Runner.GetDefaultGroup(), nil)
+	root := NewGroup(cs.RunState.GroupSummary.Group(), nil)
 	groups := FlattenGroup(root)
 
 	var group *Group

api/v1/group_routes_test.go

@@ -37,6 +37,7 @@ func getTestRunState(tb testing.TB, options lib.Options, runner lib.Runner) *lib
 		TestPreInitState: piState,
 		Options:          options,
 		Runner:           runner,
+		GroupSummary:     lib.NewGroupSummary(piState.Logger),
 		RunTags:          piState.Registry.RootTagSet().WithTagsFromMap(options.RunTags),
 	}
 }
@@ -64,15 +65,37 @@ func getControlSurface(tb testing.TB, testState *lib.TestRunState) *ControlSurfa
 func TestGetGroups(t *testing.T) {
 	t.Parallel()
 
+	cs := getControlSurface(t, getTestRunState(t, lib.Options{}, &minirunner.MiniRunner{}))
+	require.NoError(t, cs.RunState.GroupSummary.Start())
+	cs.RunState.GroupSummary.AddMetricSamples([]metrics.SampleContainer{
+		metrics.Sample{
+			TimeSeries: metrics.TimeSeries{
+				Metric: cs.RunState.BuiltinMetrics.GroupDuration,
+				Tags:   cs.RunState.Registry.RootTagSet().With("group", "::group 1"),
+			},
+		},
+		metrics.Sample{
+			TimeSeries: metrics.TimeSeries{
+				Metric: cs.RunState.BuiltinMetrics.GroupDuration,
+				Tags:   cs.RunState.Registry.RootTagSet().With("group", ""),
+			},
+		},
+		metrics.Sample{
+			TimeSeries: metrics.TimeSeries{
+				Metric: cs.RunState.BuiltinMetrics.GroupDuration,
+				Tags:   cs.RunState.Registry.RootTagSet().With("group", "::group 1::group 2"),
+			},
+		},
+	})
+	require.NoError(t, cs.RunState.GroupSummary.Stop())
+
 	g0, err := lib.NewGroup("", nil)
 	assert.NoError(t, err)
 	g1, err := g0.Group("group 1")
 	assert.NoError(t, err)
 	g2, err := g1.Group("group 2")
 	assert.NoError(t, err)
 
-	cs := getControlSurface(t, getTestRunState(t, lib.Options{}, &minirunner.MiniRunner{Group: g0}))
-
 	t.Run("list", func(t *testing.T) {
 		t.Parallel()
 

cmd/run.go

@@ -165,6 +165,8 @@ func (c *cmdRun) run(cmd *cobra.Command, args []string) (err error) {
 		return err
 	}
 
+	outputs = append(outputs, testRunState.GroupSummary)
+
 	metricsEngine, err := engine.NewMetricsEngine(testRunState.Registry, logger)
 	if err != nil {
 		return err
@@ -192,7 +194,7 @@ func (c *cmdRun) run(cmd *cobra.Command, args []string) (err error) {
 			logger.Debug("Generating the end-of-test summary...")
 			summaryResult, hsErr := test.initRunner.HandleSummary(globalCtx, &lib.Summary{
 				Metrics:         metricsEngine.ObservedMetrics,
-				RootGroup:       testRunState.Runner.GetDefaultGroup(),
+				RootGroup:       testRunState.GroupSummary.Group(),
 				TestRunDuration: executionState.GetCurrentTestRunDuration(),
 				NoColor:         c.gs.Flags.NoColor,
 				UIState: lib.UIState{

cmd/test_load.go

@@ -277,6 +277,7 @@ func (lct *loadedAndConfiguredTest) buildTestRunState(
 		Runner:           lct.initRunner,
 		Options:          lct.derivedConfig.Options, // we will always run with the derived options
 		RunTags:          lct.preInitState.Registry.RootTagSet().WithTagsFromMap(configToReinject.RunTags),
+		GroupSummary:     lib.NewGroupSummary(lct.preInitState.Logger),
 	}, nil
 }
 

cmd/ui.go

@@ -114,7 +114,8 @@ func printExecutionDescription(
 	default:
 		for _, out := range outputs {
 			desc := out.Description()
-			if desc == engine.IngesterDescription {
+			switch desc {
+			case engine.IngesterDescription, lib.GroupSummaryDescription:
 				continue
 			}
 			if strings.HasPrefix(desc, dashboard.OutputName) {

js/initcontext_test.go

@@ -342,9 +342,6 @@ func TestRequestWithBinaryFile(t *testing.T) {
 	bi, err := b.Instantiate(context.Background(), 0)
 	require.NoError(t, err)
 
-	root, err := lib.NewGroup("", nil)
-	require.NoError(t, err)
-
 	logger := logrus.New()
 	logger.Level = logrus.DebugLevel
 	logger.Out = io.Discard
@@ -354,7 +351,6 @@ func TestRequestWithBinaryFile(t *testing.T) {
 	bi.moduleVUImpl.state = &lib.State{
 		Options: lib.Options{},
 		Logger:  logger,
-		Group:   root,
 		Transport: &http.Transport{
 			DialContext: (netext.NewDialer(
 				net.Dialer{
@@ -488,9 +484,6 @@ func TestRequestWithMultipleBinaryFiles(t *testing.T) {
 	bi, err := b.Instantiate(context.Background(), 0)
 	require.NoError(t, err)
 
-	root, err := lib.NewGroup("", nil)
-	require.NoError(t, err)
-
 	logger := logrus.New()
 	logger.Level = logrus.DebugLevel
 	logger.Out = io.Discard
@@ -500,7 +493,6 @@ func TestRequestWithMultipleBinaryFiles(t *testing.T) {
 	bi.moduleVUImpl.state = &lib.State{
 		Options: lib.Options{},
 		Logger:  logger,
-		Group:   root,
 		Transport: &http.Transport{
 			DialContext: (netext.NewDialer(
 				net.Dialer{

js/modules/k6/grpc/params_test.go

@@ -148,15 +148,12 @@ func newParamsTestRuntime(t *testing.T, paramsJSON string) (*modulestest.Runtime
 
 	testRuntime := modulestest.NewRuntime(t)
 	registry := metrics.NewRegistry()
-	root, err := lib.NewGroup("", nil)
-	require.NoError(t, err)
 
 	logger := logrus.New()
 	logger.SetLevel(logrus.InfoLevel)
 	logger.Out = io.Discard
 
 	state := &lib.State{
-		Group: root,
 		Options: lib.Options{
 			SystemTags: metrics.NewSystemTagSet(
 				metrics.TagName,
@@ -171,7 +168,7 @@ func newParamsTestRuntime(t *testing.T, paramsJSON string) (*modulestest.Runtime
 
 	testRuntime.MoveToVUContext(state)
 
-	_, err = testRuntime.VU.Runtime().RunString(`let params = ` + paramsJSON + `;`)
+	_, err := testRuntime.VU.Runtime().RunString(`let params = ` + paramsJSON + `;`)
 	require.NoError(t, err)
 
 	params := testRuntime.VU.Runtime().Get("params")

js/modules/k6/grpc/teststate_test.go

@@ -142,13 +142,8 @@ func newTestState(t *testing.T) testState {
 // ToVUContext moves the test state to the VU context.
 func (ts *testState) ToVUContext() {
 	registry := metrics.NewRegistry()
-	root, err := lib.NewGroup("", nil)
-	if err != nil {
-		panic(err)
-	}
 
 	state := &lib.State{
-		Group:     root,
 		Dialer:    ts.httpBin.Dialer,
 		TLSConfig: ts.httpBin.TLSClientConfig,
 		Samples:   ts.samples,

js/modules/k6/http/request_test.go

@@ -128,8 +128,6 @@ type httpTestCase struct {
 func newTestCase(t testing.TB) *httpTestCase {
 	tb := httpmultibin.NewHTTPMultiBin(t)
 
-	root, err := lib.NewGroup("", nil)
-	require.NoError(t, err)
 	registry := metrics.NewRegistry()
 
 	logger := logrus.New()
@@ -152,13 +150,12 @@ func newTestCase(t testing.TB) *httpTestCase {
 	state := &lib.State{
 		Options:    options,
 		Logger:     logger,
-		Group:      root,
 		TLSConfig:  tb.TLSClientConfig,
 		Transport:  tb.HTTPTransport,
 		BufferPool: lib.NewBufferPool(),
 		Samples:    samples,
 		Tags: lib.NewVUStateTags(registry.RootTagSet().WithTagsFromMap(map[string]string{
-			"group": root.Path,
+			"group": lib.RootGroupPath,
 		})),
 		BuiltinMetrics: metrics.RegisterBuiltinMetrics(registry),
 	}

js/modules/k6/http/response_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.k6.io/k6/lib"
 	"go.k6.io/k6/metrics"
 )
 
@@ -109,7 +110,6 @@ func TestResponse(t *testing.T) {
 	samples := ts.samples
 	rt := ts.runtime.VU.Runtime()
 	state := ts.runtime.VU.State()
-	root := state.Group
 	sr := tb.Replacer.Replace
 
 	tb.Mux.HandleFunc("/myforms/get", myFormHandler)
@@ -147,17 +147,14 @@ func TestResponse(t *testing.T) {
 		})
 
 		t.Run("group", func(t *testing.T) {
-			g, err := root.Group("my group")
+			groupName, err := lib.NewGroupPath(lib.RootGroupPath, "my group")
 			require.NoError(t, err)
-			old := state.Group
-			state.Group = g
 			state.Tags.Modify(func(tagsAndMeta *metrics.TagsAndMeta) {
-				tagsAndMeta.SetTag("group", g.Path)
+				tagsAndMeta.SetTag("group", groupName)
 			})
 			defer func() {
-				state.Group = old
 				state.Tags.Modify(func(tagsAndMeta *metrics.TagsAndMeta) {
-					tagsAndMeta.SetTag("group", old.Path)
+					tagsAndMeta.SetTag("group", "")
 				})
 			}()
 

js/modules/k6/k6.go

@@ -5,13 +5,14 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
-	"sync/atomic"
+	"strings"
 	"time"
 
 	"github.com/dop251/goja"
 
 	"go.k6.io/k6/js/common"
 	"go.k6.io/k6/js/modules"
+	"go.k6.io/k6/lib"
 	"go.k6.io/k6/metrics"
 )
 
@@ -105,25 +106,23 @@ func (mi *K6) Group(name string, val goja.Value) (goja.Value, error) {
 	if common.IsAsyncFunction(mi.vu.Runtime(), val) {
 		return goja.Undefined(), fmt.Errorf(asyncFunctionNotSupportedMsg, "group")
 	}
-	g, err := state.Group.Group(name)
+	oldGroupName, _ := state.Tags.GetCurrentValues().Tags.Get(metrics.TagGroup.String())
+	// TODO: what are we doing if group is not tagged
+	newGroupName, err := lib.NewGroupPath(oldGroupName, name)
 	if err != nil {
 		return goja.Undefined(), err
 	}
 
-	old := state.Group
-	state.Group = g
-
 	shouldUpdateTag := state.Options.SystemTags.Has(metrics.TagGroup)
 	if shouldUpdateTag {
 		state.Tags.Modify(func(tagsAndMeta *metrics.TagsAndMeta) {
-			tagsAndMeta.SetSystemTagOrMeta(metrics.TagGroup, g.Path)
+			tagsAndMeta.SetSystemTagOrMeta(metrics.TagGroup, newGroupName)
 		})
 	}
 	defer func() {
-		state.Group = old
 		if shouldUpdateTag {
 			state.Tags.Modify(func(tagsAndMeta *metrics.TagsAndMeta) {
-				tagsAndMeta.SetSystemTagOrMeta(metrics.TagGroup, old.Path)
+				tagsAndMeta.SetSystemTagOrMeta(metrics.TagGroup, oldGroupName)
 			})
 		}
 	}()
@@ -148,8 +147,6 @@ func (mi *K6) Group(name string, val goja.Value) (goja.Value, error) {
 }
 
 // Check will emit check metrics for the provided checks.
-//
-//nolint:funlen
 func (mi *K6) Check(arg0, checks goja.Value, extras ...goja.Value) (bool, error) {
 	state := mi.vu.State()
 	if state == nil {
@@ -174,17 +171,14 @@ func (mi *K6) Check(arg0, checks goja.Value, extras ...goja.Value) (bool, error)
 	var exc error
 	obj := checks.ToObject(rt)
 	for _, name := range obj.Keys() {
-		val := obj.Get(name)
-
-		// Resolve the check record.
-		check, err := state.Group.Check(name)
-		if err != nil {
-			return false, err
+		if strings.Contains(name, lib.GroupSeparator) {
+			return false, lib.ErrNameContainsGroupSeparator
 		}
+		val := obj.Get(name)
 
 		tags := commonTagsAndMeta.Tags
 		if state.Options.SystemTags.Has(metrics.TagCheck) {
-			tags = tags.With("check", check.Name)
+			tags = tags.With("check", name)
 		}
 
 		if common.IsAsyncFunction(rt, val) {
@@ -207,28 +201,19 @@ func (mi *K6) Check(arg0, checks goja.Value, extras ...goja.Value) (bool, error)
 			succ = false
 		}
 
-		// Emit! (But only if we have a valid context.)
-		select {
-		case <-ctx.Done():
-		default:
-			sample := metrics.Sample{
-				TimeSeries: metrics.TimeSeries{
-					Metric: state.BuiltinMetrics.Checks,
-					Tags:   tags,
-				},
-				Time:     t,
-				Metadata: commonTagsAndMeta.Metadata,
-				Value:    0,
-			}
-			if booleanVal {
-				atomic.AddInt64(&check.Passes, 1)
-				sample.Value = 1
-			} else {
-				atomic.AddInt64(&check.Fails, 1)
-			}
-
-			metrics.PushIfNotDone(ctx, state.Samples, sample)
+		sample := metrics.Sample{
+			TimeSeries: metrics.TimeSeries{
+				Metric: state.BuiltinMetrics.Checks,
+				Tags:   tags,
+			},
+			Time:     t,
+			Metadata: commonTagsAndMeta.Metadata,
 		}
+		if booleanVal {
+			sample.Value = 1
+		}
+
+		metrics.PushIfNotDone(ctx, state.Samples, sample)
 
 		if exc != nil {
 			return false, exc