diff --git a/cmd/report.go b/cmd/report.go index 74a6e9d2e31..874a5c595cc 100644 --- a/cmd/report.go +++ b/cmd/report.go @@ -15,9 +15,12 @@ import ( func createReport( u *usage.Usage, execScheduler *execution.Scheduler, importedModules []string, outputs []string, -) map[string]any { +) (map[string]any, error) { for _, ec := range execScheduler.GetExecutorConfigs() { - u.Uint64("executors/"+ec.GetType(), 1) + err := u.Uint64("executors/"+ec.GetType(), 1) + if err != nil { // TODO report it as an error but still send the report? + return nil, err + } } // collect the report only with k6 public modules @@ -33,7 +36,7 @@ func createReport( if !strings.HasPrefix(module, "k6") { continue } - u.Strings("modules", module) + _ = u.Strings("modules", module) // TODO this will be moved and the error can be logged there } builtinOutputs := builtinOutputStrings() @@ -54,19 +57,23 @@ func createReport( if !builtinOutputsIndex[o] { continue } - u.Strings("outputs", o) + _ = u.Strings("outputs", o) // TODO this will be moved and the error can be logged there } execState := execScheduler.GetState() - u.Uint64("vus_max", uint64(execState.GetInitializedVUsCount())) - u.Uint64("iterations", execState.GetFullIterationCount()) - m := u.Map() + m, err := u.Map() - m["k6_version"] = consts.Version - m["duration"] = execState.GetCurrentTestRunDuration().String() - m["goos"] = runtime.GOOS - m["goarch"] = runtime.GOARCH + if m != nil { + m["k6_version"] = consts.Version + m["duration"] = execState.GetCurrentTestRunDuration().String() + m["goos"] = runtime.GOOS + m["goarch"] = runtime.GOARCH + m["goarch"] = runtime.GOARCH + m["goarch"] = runtime.GOARCH + m["vus_max"] = uint64(execState.GetInitializedVUsCount()) + m["iterations"] = execState.GetFullIterationCount() + } - return m + return m, err } func reportUsage(ctx context.Context, execScheduler *execution.Scheduler, test *loadedAndConfiguredTest) error { @@ -76,7 +83,12 @@ func reportUsage(ctx context.Context, execScheduler *execution.Scheduler, test * outputs = append(outputs, outputName) } - body, err := json.Marshal(createReport(test.usage, execScheduler, test.moduleResolver.Imported(), outputs)) + m, err := createReport(test.usage, execScheduler, test.moduleResolver.Imported(), outputs) + if err != nil { + // TODO actually log the error but continue if there is something to report + return err + } + body, err := json.Marshal(m) if err != nil { return err } diff --git a/cmd/report_test.go b/cmd/report_test.go index b7966b8a859..d0e3286621d 100644 --- a/cmd/report_test.go +++ b/cmd/report_test.go @@ -52,7 +52,8 @@ func TestCreateReport(t *testing.T) { time.Sleep(10 * time.Millisecond) s.GetState().MarkEnded() - m := createReport(usage.New(), s, importedModules, outputs) + m, err := createReport(usage.New(), s, importedModules, outputs) + require.NoError(t, err) assert.Equal(t, consts.Version, m["k6_version"]) assert.Equal(t, map[string]interface{}{"shared-iterations": uint64(1)}, m["executors"]) diff --git a/output/cloud/output.go b/output/cloud/output.go index 01b367d56ba..40531a416e6 100644 --- a/output/cloud/output.go +++ b/output/cloud/output.go @@ -345,7 +345,10 @@ func (out *Output) startVersionedOutput() error { } var err error - out.usage.Strings("cloud/test_run_id", out.testRunID) + usageErr := out.usage.Strings("cloud/test_run_id", out.testRunID) + if usageErr != nil { + out.logger.Warning("Couldn't report test run id to usage as part of writing to k6 cloud") + } // TODO: move here the creation of a new cloudapi.Client // so in the case the config has been overwritten the client uses the correct diff --git a/usage/usage.go b/usage/usage.go index 0850c3fe5d6..45a0fc9a996 100644 --- a/usage/usage.go +++ b/usage/usage.go @@ -2,6 +2,9 @@ package usage import ( + "errors" + "fmt" + "sort" "strings" "sync" ) @@ -22,49 +25,54 @@ func New() *Usage { // Strings appends the provided value to a slice of strings that is the value. // Appending to the slice if the key is already there. -func (u *Usage) Strings(k, v string) { +func (u *Usage) Strings(k, v string) error { u.l.Lock() defer u.l.Unlock() oldV, ok := u.m[k] if !ok { u.m[k] = []string{v} - return + return nil } switch oldV := oldV.(type) { - case string: - u.m[k] = []string{oldV, v} case []string: u.m[k] = append(oldV, v) default: - // TODO: error, panic?, nothing, log? + return fmt.Errorf("value of key %s is not []string as expected but %T", k, oldV) } + return nil } // Uint64 adds the provided value to a given key. Creating the key if needed -func (u *Usage) Uint64(k string, v uint64) { +func (u *Usage) Uint64(k string, v uint64) error { u.l.Lock() defer u.l.Unlock() oldV, ok := u.m[k] if !ok { u.m[k] = v - return + return nil } - switch oldV := oldV.(type) { + switch oldVUint64 := oldV.(type) { case uint64: - u.m[k] = oldV + v + u.m[k] = oldVUint64 + v default: + return fmt.Errorf("!value of key %s is not uint64 as expected but %T", k, oldV) // TODO: error, panic?, nothing, log? } + return nil } // Map returns a copy of the internal map plus making subusages from keys that have a slash in them // only a single level is being respected -func (u *Usage) Map() map[string]any { +func (u *Usage) Map() (map[string]any, error) { u.l.Lock() defer u.l.Unlock() + var errs []error + keys := mapKeys(u.m) + sort.Strings(keys) result := make(map[string]any, len(u.m)) - for k, v := range u.m { + for _, k := range keys { + v := u.m[k] prefix, post, found := strings.Cut(k, "/") if !found { result[k] = v @@ -78,30 +86,20 @@ func (u *Usage) Map() map[string]any { } topLevelMap, ok := topLevel.(map[string]any) if !ok { - continue // TODO panic?, error? - } - keyLevel, ok := topLevelMap[post] - switch value := v.(type) { - case uint64: - switch i := keyLevel.(type) { - case uint64: - keyLevel = i + value - default: - // TODO:panic? error? - } - case []string: - switch i := keyLevel.(type) { - case []string: - keyLevel = append(i, value...) //nolint:gocritic // we assign to the final value - default: - // TODO:panic? error? - } - } - if !ok { - keyLevel = v + errs = append(errs, fmt.Errorf("key %s was expected to be a map[string]any but was %T", prefix, topLevel)) + continue } - topLevelMap[post] = keyLevel + topLevelMap[post] = v } - return result + return result, errors.Join(errs...) +} + +// replace with map.Keys from go 1.23 after that is the minimal version +func mapKeys(m map[string]any) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys } diff --git a/usage/usage_test.go b/usage/usage_test.go new file mode 100644 index 00000000000..7400c6b76c7 --- /dev/null +++ b/usage/usage_test.go @@ -0,0 +1,40 @@ +package usage + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestErrors(t *testing.T) { + t.Parallel() + u := New() + require.NoError(t, u.Uint64("test/one", 1)) + require.NoError(t, u.Uint64("test/two", 1)) + require.NoError(t, u.Uint64("test/two", 1)) + require.NoError(t, u.Strings("test/three", "three")) + require.NoError(t, u.Strings("test2/one", "one")) + + require.ErrorContains(t, u.Strings("test/one", "one"), + "test/one is not []string as expected but uint64") + require.ErrorContains(t, u.Uint64("test2/one", 1), + "test2/one is not uint64 as expected but []string") + + require.NoError(t, u.Strings("test3", "some")) + require.NoError(t, u.Strings("test3/one", "one")) + + m, err := u.Map() + require.ErrorContains(t, err, + "key test3 was expected to be a map[string]any but was []string") + require.EqualValues(t, map[string]any{ + "test": map[string]any{ + "one": uint64(1), + "two": uint64(2), + "three": []string{"three"}, + }, + "test2": map[string]any{ + "one": []string{"one"}, + }, + "test3": []string{"some"}, + }, m) +}