feat(cli): add json output to coder speedtest (#13475)

cli/cliui/output.go

@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/jedib0t/go-pretty/v6/table"
 	"golang.org/x/xerrors"
 
 	"github.com/coder/serpent"
@@ -143,7 +144,11 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) {
 
 // Format implements OutputFormat.
 func (f *tableFormat) Format(_ context.Context, data any) (string, error) {
-	return DisplayTable(data, f.sort, f.columns)
+	headers := make(table.Row, len(f.allColumns))
+	for i, header := range f.allColumns {
+		headers[i] = header
+	}
+	return renderTable(data, f.sort, headers, f.columns)
 }
 
 type jsonFormat struct{}

cli/cliui/table.go

@@ -22,6 +22,13 @@ func Table() table.Writer {
 	return tableWriter
 }
 
+// This type can be supplied as part of a slice to DisplayTable
+// or to a `TableFormat` `Format` call to render a separator.
+// Leading separators are not supported and trailing separators
+// are ignored by the table formatter.
+// e.g. `[]any{someRow, TableSeparator, someRow}`
+type TableSeparator struct{}
+
 // filterTableColumns returns configurations to hide columns
 // that are not provided in the array. If the array is empty,
 // no filtering will occur!
@@ -47,8 +54,12 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig
 	return columnConfigs
 }
 
-// DisplayTable renders a table as a string. The input argument must be a slice
+// DisplayTable renders a table as a string. The input argument can be:
-// of structs. At least one field in the struct must have a `table:""` tag
+//   - a struct slice.
+//   - an interface slice, where the first element is a struct,
+//     and all other elements are of the same type, or a TableSeparator.
+//
+// At least one field in the struct must have a `table:""` tag
 // containing the name of the column in the outputted table.
 //
 // If `sort` is not specified, the field with the `table:"$NAME,default_sort"`
@@ -66,11 +77,20 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
 	v := reflect.Indirect(reflect.ValueOf(out))
 
 	if v.Kind() != reflect.Slice {
-		return "", xerrors.Errorf("DisplayTable called with a non-slice type")
+		return "", xerrors.New("DisplayTable called with a non-slice type")
+	}
+	var tableType reflect.Type
+	if v.Type().Elem().Kind() == reflect.Interface {
+		if v.Len() == 0 {
+			return "", xerrors.New("DisplayTable called with empty interface slice")
+		}
+		tableType = reflect.Indirect(reflect.ValueOf(v.Index(0).Interface())).Type()
+	} else {
+		tableType = v.Type().Elem()
 	}
 
 	// Get the list of table column headers.
-	headersRaw, defaultSort, err := typeToTableHeaders(v.Type().Elem(), true)
+	headersRaw, defaultSort, err := typeToTableHeaders(tableType, true)
 	if err != nil {
 		return "", xerrors.Errorf("get table headers recursively for type %q: %w", v.Type().Elem().String(), err)
 	}
@@ -82,9 +102,8 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
 	}
 	headers := make(table.Row, len(headersRaw))
 	for i, header := range headersRaw {
-		headers[i] = header
+		headers[i] = strings.ReplaceAll(header, "_", " ")
 	}
-
 	// Verify that the given sort column and filter columns are valid.
 	if sort != "" || len(filterColumns) != 0 {
 		headersMap := make(map[string]string, len(headersRaw))
@@ -130,6 +149,11 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
 			return "", xerrors.Errorf("specified sort column %q not found in table headers, available columns are %q", sort, strings.Join(headersRaw, `", "`))
 		}
 	}
+	return renderTable(out, sort, headers, filterColumns)
+}
+
+func renderTable(out any, sort string, headers table.Row, filterColumns []string) (string, error) {
+	v := reflect.Indirect(reflect.ValueOf(out))
 
 	// Setup the table formatter.
 	tw := Table()
@@ -143,15 +167,22 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
 
 	// Write each struct to the table.
 	for i := 0; i < v.Len(); i++ {
+		cur := v.Index(i).Interface()
+		_, ok := cur.(TableSeparator)
+		if ok {
+			tw.AppendSeparator()
+			continue
+		}
 		// Format the row as a slice.
-		rowMap, err := valueToTableMap(v.Index(i))
+		// ValueToTableMap does what `reflect.Indirect` does
+		rowMap, err := valueToTableMap(reflect.ValueOf(cur))
 		if err != nil {
 			return "", xerrors.Errorf("get table row map %v: %w", i, err)
 		}
 
 		rowSlice := make([]any, len(headers))
-		for i, h := range headersRaw {
+		for i, h := range headers {
-			v, ok := rowMap[h]
+			v, ok := rowMap[h.(string)]
 			if !ok {
 				v = nil
 			}
@@ -188,25 +219,28 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
 // returned. If the table tag is malformed, an error is returned.
 //
 // The returned name is transformed from "snake_case" to "normal text".
-func parseTableStructTag(field reflect.StructField) (name string, defaultSort, recursive bool, skipParentName bool, err error) {
+func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName bool, err error) {
 	tags, err := structtag.Parse(string(field.Tag))
 	if err != nil {
-		return "", false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err)
+		return "", false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err)
 	}
 
 	tag, err := tags.Get("table")
 	if err != nil || tag.Name == "-" {
 		// tags.Get only returns an error if the tag is not found.
-		return "", false, false, false, nil
+		return "", false, false, false, false, nil
 	}
 
 	defaultSortOpt := false
+	noSortOpt = false
 	recursiveOpt := false
 	skipParentNameOpt := false
 	for _, opt := range tag.Options {
 		switch opt {
 		case "default_sort":
 			defaultSortOpt = true
+		case "nosort":
+			noSortOpt = true
 		case "recursive":
 			recursiveOpt = true
 		case "recursive_inline":
@@ -216,11 +250,11 @@ func parseTableStructTag(field reflect.StructField) (name string, defaultSort, r
 			recursiveOpt = true
 			skipParentNameOpt = true
 		default:
-			return "", false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt)
+			return "", false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt)
 		}
 	}
 
-	return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, recursiveOpt, skipParentNameOpt, nil
+	return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, nil
 }
 
 func isStructOrStructPointer(t reflect.Type) bool {
@@ -244,12 +278,16 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string,
 
 	headers := []string{}
 	defaultSortName := ""
+	noSortOpt := false
 	for i := 0; i < t.NumField(); i++ {
 		field := t.Field(i)
-		name, defaultSort, recursive, skip, err := parseTableStructTag(field)
+		name, defaultSort, noSort, recursive, skip, err := parseTableStructTag(field)
 		if err != nil {
 			return nil, "", xerrors.Errorf("parse struct tags for field %q in type %q: %w", field.Name, t.String(), err)
 		}
+		if requireDefault && noSort {
+			noSortOpt = true
+		}
 
 		if name == "" && (recursive && skip) {
 			return nil, "", xerrors.Errorf("a name is required for the field %q. "+
@@ -292,8 +330,8 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string,
 		headers = append(headers, name)
 	}
 
-	if defaultSortName == "" && requireDefault {
+	if defaultSortName == "" && requireDefault && !noSortOpt {
-		return nil, "", xerrors.Errorf("no field marked as default_sort in type %q", t.String())
+		return nil, "", xerrors.Errorf("no field marked as default_sort or nosort in type %q", t.String())
 	}
 
 	return headers, defaultSortName, nil
@@ -320,7 +358,7 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) {
 	for i := 0; i < val.NumField(); i++ {
 		field := val.Type().Field(i)
 		fieldVal := val.Field(i)
-		name, _, recursive, skip, err := parseTableStructTag(field)
+		name, _, _, recursive, skip, err := parseTableStructTag(field)
 		if err != nil {
 			return nil, xerrors.Errorf("parse struct tags for field %q in type %T: %w", field.Name, val, err)
 		}

cli/cliui/table_test.go

@@ -218,6 +218,42 @@ Alice   25
 		compareTables(t, expected, out)
 	})
 
+	// This test ensures we can display dynamically typed slices
+	t.Run("Interfaces", func(t *testing.T) {
+		t.Parallel()
+
+		in := []any{tableTest1{}}
+		out, err := cliui.DisplayTable(in, "", nil)
+		t.Log("rendered table:\n" + out)
+		require.NoError(t, err)
+		other := []tableTest1{{}}
+		expected, err := cliui.DisplayTable(other, "", nil)
+		require.NoError(t, err)
+		compareTables(t, expected, out)
+	})
+
+	t.Run("WithSeparator", func(t *testing.T) {
+		t.Parallel()
+		expected := `
+NAME  AGE  ROLES    SUB 1 NAME  SUB 1 AGE  SUB 2 NAME  SUB 2 AGE  SUB 3 INNER NAME  SUB 3 INNER AGE  SUB 4       TIME                  TIME PTR              
+bar    20  [a]      bar1               21  <nil>       <nil>      bar3                           23  {bar4 24 }  2022-08-02T15:49:10Z  <nil>                 
+-------------------------------------------------------------------------------------------------------------------------------------------------------------
+baz    30  []       baz1               31  <nil>       <nil>      baz3                           33  {baz4 34 }  2022-08-02T15:49:10Z  <nil>                 
+-------------------------------------------------------------------------------------------------------------------------------------------------------------
+foo    10  [a b c]  foo1               11  foo2        12         foo3                           13  {foo4 14 }  2022-08-02T15:49:10Z  2022-08-02T15:49:10Z 
+		`
+
+		var inlineIn []any
+		for _, v := range in {
+			inlineIn = append(inlineIn, v)
+			inlineIn = append(inlineIn, cliui.TableSeparator{})
+		}
+		out, err := cliui.DisplayTable(inlineIn, "", nil)
+		t.Log("rendered table:\n" + out)
+		require.NoError(t, err)
+		compareTables(t, expected, out)
+	})
+
 	// This test ensures that safeties against invalid use of `table` tags
 	// causes errors (even without data).
 	t.Run("Errors", func(t *testing.T) {
@@ -255,14 +291,6 @@ Alice   25
 				_, err := cliui.DisplayTable(in, "", nil)
 				require.Error(t, err)
 			})
-
-			t.Run("WithData", func(t *testing.T) {
-				t.Parallel()
-
-				in := []any{tableTest1{}}
-				_, err := cliui.DisplayTable(in, "", nil)
-				require.Error(t, err)
-			})
 		})
 
 		t.Run("NotStruct", func(t *testing.T) {

cli/speedtest.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"time"
 
-	"github.com/jedib0t/go-pretty/v6/table"
 	"golang.org/x/xerrors"
 	tsspeedtest "tailscale.com/net/speedtest"
 	"tailscale.com/wgengine/capture"
@@ -19,12 +18,51 @@ import (
 	"github.com/coder/serpent"
 )
 
+type SpeedtestResult struct {
+	Overall   SpeedtestResultInterval   `json:"overall"`
+	Intervals []SpeedtestResultInterval `json:"intervals"`
+}
+
+type SpeedtestResultInterval struct {
+	StartTimeSeconds float64 `json:"start_time_seconds"`
+	EndTimeSeconds   float64 `json:"end_time_seconds"`
+	ThroughputMbits  float64 `json:"throughput_mbits"`
+}
+
+type speedtestTableItem struct {
+	Interval   string `table:"Interval,nosort"`
+	Throughput string `table:"Throughput"`
+}
+
 func (r *RootCmd) speedtest() *serpent.Command {
 	var (
 		direct    bool
 		duration  time.Duration
 		direction string
 		pcapFile  string
+		formatter = cliui.NewOutputFormatter(
+			cliui.ChangeFormatterData(cliui.TableFormat([]speedtestTableItem{}, []string{"Interval", "Throughput"}), func(data any) (any, error) {
+				res, ok := data.(SpeedtestResult)
+				if !ok {
+					// This should never happen
+					return "", xerrors.Errorf("expected speedtestResult, got %T", data)
+				}
+				tableRows := make([]any, len(res.Intervals)+2)
+				for i, r := range res.Intervals {
+					tableRows[i] = speedtestTableItem{
+						Interval:   fmt.Sprintf("%.2f-%.2f sec", r.StartTimeSeconds, r.EndTimeSeconds),
+						Throughput: fmt.Sprintf("%.4f Mbits/sec", r.ThroughputMbits),
+					}
+				}
+				tableRows[len(res.Intervals)] = cliui.TableSeparator{}
+				tableRows[len(res.Intervals)+1] = speedtestTableItem{
+					Interval:   fmt.Sprintf("%.2f-%.2f sec", res.Overall.StartTimeSeconds, res.Overall.EndTimeSeconds),
+					Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits),
+				}
+				return tableRows, nil
+			}),
+			cliui.JSONFormat(),
+		)
 	)
 	client := new(codersdk.Client)
 	cmd := &serpent.Command{
@@ -124,24 +162,32 @@ func (r *RootCmd) speedtest() *serpent.Command {
 			default:
 				return xerrors.Errorf("invalid direction: %q", direction)
 			}
-			cliui.Infof(inv.Stdout, "Starting a %ds %s test...", int(duration.Seconds()), tsDir)
+			cliui.Infof(inv.Stderr, "Starting a %ds %s test...", int(duration.Seconds()), tsDir)
 			results, err := conn.Speedtest(ctx, tsDir, duration)
 			if err != nil {
 				return err
 			}
-			tableWriter := cliui.Table()
+			var outputResult SpeedtestResult
-			tableWriter.AppendHeader(table.Row{"Interval", "Throughput"})
 			startTime := results[0].IntervalStart
-			for _, r := range results {
+			outputResult.Intervals = make([]SpeedtestResultInterval, len(results)-1)
-				if r.Total {
+			for i, r := range results {
-					tableWriter.AppendSeparator()
+				interval := SpeedtestResultInterval{
+					StartTimeSeconds: r.IntervalStart.Sub(startTime).Seconds(),
+					EndTimeSeconds:   r.IntervalEnd.Sub(startTime).Seconds(),
+					ThroughputMbits:  r.MBitsPerSecond(),
+				}
+				if r.Total {
+					interval.StartTimeSeconds = 0
+					outputResult.Overall = interval
+				} else {
+					outputResult.Intervals[i] = interval
 				}
-				tableWriter.AppendRow(table.Row{
-					fmt.Sprintf("%.2f-%.2f sec", r.IntervalStart.Sub(startTime).Seconds(), r.IntervalEnd.Sub(startTime).Seconds()),
-					fmt.Sprintf("%.4f Mbits/sec", r.MBitsPerSecond()),
-				})
 			}
-			_, err = fmt.Fprintln(inv.Stdout, tableWriter.Render())
+			out, err := formatter.Format(inv.Context(), outputResult)
+			if err != nil {
+				return err
+			}
+			_, err = fmt.Fprintln(inv.Stdout, out)
 			return err
 		},
 	}
@@ -173,5 +219,6 @@ func (r *RootCmd) speedtest() *serpent.Command {
 			Value:       serpent.StringOf(&pcapFile),
 		},
 	}
+	formatter.AttachOptions(&cmd.Options)
 	return cmd
 }

cli/speedtest_test.go

@@ -1,7 +1,9 @@
 package cli_test
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -10,6 +12,7 @@ import (
 	"cdr.dev/slog"
 	"cdr.dev/slog/sloggers/slogtest"
 	"github.com/coder/coder/v2/agent/agenttest"
+	"github.com/coder/coder/v2/cli"
 	"github.com/coder/coder/v2/cli/clitest"
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/codersdk"
@@ -56,3 +59,45 @@ func TestSpeedtest(t *testing.T) {
 	})
 	<-cmdDone
 }
+
+func TestSpeedtestJson(t *testing.T) {
+	t.Parallel()
+	t.Skip("Potentially flaky test - see https://github.com/coder/coder/issues/6321")
+	if testing.Short() {
+		t.Skip("This test takes a minimum of 5ms per a hardcoded value in Tailscale!")
+	}
+	client, workspace, agentToken := setupWorkspaceForAgent(t)
+	_ = agenttest.New(t, client.URL, agentToken)
+	coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
+
+	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+	defer cancel()
+
+	require.Eventually(t, func() bool {
+		ws, err := client.Workspace(ctx, workspace.ID)
+		if !assert.NoError(t, err) {
+			return false
+		}
+		a := ws.LatestBuild.Resources[0].Agents[0]
+		return a.Status == codersdk.WorkspaceAgentConnected &&
+			a.LifecycleState == codersdk.WorkspaceAgentLifecycleReady
+	}, testutil.WaitLong, testutil.IntervalFast, "agent is not ready")
+
+	inv, root := clitest.New(t, "speedtest", "--output=json", workspace.Name)
+	clitest.SetupConfig(t, client, root)
+	out := bytes.NewBuffer(nil)
+	inv.Stdout = out
+	ctx, cancel = context.WithTimeout(context.Background(), testutil.WaitLong)
+	defer cancel()
+
+	inv.Logger = slogtest.Make(t, nil).Named("speedtest").Leveled(slog.LevelDebug)
+	cmdDone := tGo(t, func() {
+		err := inv.WithContext(ctx).Run()
+		assert.NoError(t, err)
+	})
+	<-cmdDone
+
+	var result cli.SpeedtestResult
+	require.NoError(t, json.Unmarshal(out.Bytes(), &result))
+	require.Len(t, result.Intervals, 5)
+}

cli/testdata/coder_speedtest_--help.golden

@@ -6,6 +6,10 @@ USAGE:
   Run upload and download tests from your machine to a workspace
 
 OPTIONS:
+  -c, --column string-array (default: Interval,Throughput)
+          Columns to display in table output. Available columns: Interval,
+          Throughput.
+
   -d, --direct bool
           Specifies whether to wait for a direct connection before testing
           speed.
@@ -14,6 +18,9 @@ OPTIONS:
           Specifies whether to run in reverse mode where the client receives and
           the server sends.
 
+  -o, --output string (default: table)
+          Output format. Available formats: table, json.
+
       --pcap-file string
           Specifies a file to write a network capture to.
 

docs/cli/speedtest.md

@@ -45,3 +45,21 @@ Specifies the duration to monitor traffic.
 | Type | <code>string</code> |
 
 Specifies a file to write a network capture to.
+
+### -c, --column
+
+|         |                                  |
+| ------- | -------------------------------- |
+| Type    | <code>string-array</code>        |
+| Default | <code>Interval,Throughput</code> |
+
+Columns to display in table output. Available columns: Interval, Throughput.
+
+### -o, --output
+
+|         |                     |
+| ------- | ------------------- |
+| Type    | <code>string</code> |
+| Default | <code>table</code>  |
+
+Output format. Available formats: table, json.