Summarize: only run webserver on demand (#89)

* Only run webserver on demand for summarize. Output html if no server launched. Make default format markdown

Signed-off-by: Rohit Nayak <rohit@planetscale.com>

* Reuse summarize template, but different layout for standalone pages including style/js inline

Signed-off-by: Rohit Nayak <rohit@planetscale.com>

* refactor: move code around

Signed-off-by: Andres Taylor <andres@planetscale.com>

* test: improve CLI testing

Signed-off-by: Andres Taylor <andres@planetscale.com>

---------

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Co-authored-by: Andres Taylor <andres@planetscale.com>

Author: rohit-nayak-ps

go/cmd/keys.go

@@ -35,7 +35,7 @@ func keysCmd() *cobra.Command {
 		PreRun: func(cmd *cobra.Command, _ []string) {
 			csvConfig = csvFlagsToConfig(cmd, *flags)
 		},
-		RunE: func(_ *cobra.Command, args []string) error {
+		RunE: func(c *cobra.Command, args []string) error {
 			cfg := keys.Config{
 				FileName: args[0],
 			}
@@ -46,7 +46,7 @@ func keysCmd() *cobra.Command {
 			}
 			cfg.Loader = loader
 
-			return keys.Run(cfg)
+			return keys.Run(c.OutOrStdout(), cfg)
 		},
 	}
 

go/cmd/main_test.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2024 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestApp(t *testing.T) {
+	tests := []struct {
+		args []string
+		want string
+	}{
+		{[]string{"help"}, "Utils tools for testing, running and benchmarking Vitess"},
+		{[]string{"keys", "../../t/demo.test"}, `"queryStructure"`},
+		{[]string{"keys", "--help"}, `Runs vexplain keys on all queries of the test file`},
+		{[]string{"txs", "--help"}, `Analyze transactions on a query log`},
+		{[]string{"test", "--help"}, `Test the given workload against both Vitess and MySQL`},
+		{[]string{"trace", "--help"}, `Runs the given workload and does a`},
+		{[]string{"summarize", "--help"}, `Compares and analyses a trace output`},
+		{[]string{"dbinfo", "--help"}, `Loads info from the database including row counts`},
+		{[]string{"planalyze", "--help"}, `Analyze the query plan`},
+	}
+	for _, tt := range tests {
+		t.Run("vt  "+strings.Join(tt.args, " "), func(t *testing.T) {
+			cmd := getRootCmd()
+			buf := new(bytes.Buffer)
+			cmd.SetOut(buf)
+			cmd.SetErr(buf)
+			cmd.SetArgs(tt.args)
+
+			err := cmd.Execute()
+
+			out := buf.String()
+			require.NoError(t, err, out)
+			require.Contains(t, out, tt.want)
+		})
+	}
+}

go/cmd/root.go

@@ -17,77 +17,34 @@ limitations under the License.
 package cmd
 
 import (
-	"fmt"
 	"os"
-	"time"
 
 	"github.com/spf13/cobra"
-
-	"github.com/vitessio/vt/go/web"
 )
 
 // Execute adds all child commands to the root command and sets flags appropriately.
 // This is called by main.main(). It only needs to happen once to the rootCmd.
 func Execute() {
+	err := getRootCmd().Execute()
+	if err != nil {
+		os.Exit(1)
+	}
+}
+
+func getRootCmd() *cobra.Command {
 	// rootCmd represents the base command when called without any subcommands
-	var port int64
-	webserverStarted := false
-	ch := make(chan int, 2)
 	root := &cobra.Command{
 		Use:   "vt",
 		Short: "Utils tools for testing, running and benchmarking Vitess.",
-		RunE: func(_ *cobra.Command, _ []string) error {
-			if port > 0 {
-				if webserverStarted {
-					return nil
-				}
-				webserverStarted = true
-				go startWebServer(port, ch)
-				<-ch
-			}
-			return nil
-		},
 	}
-	root.PersistentFlags().Int64VarP(&port, "port", "p", 8080, "Port to run the web server on")
 	root.CompletionOptions.HiddenDefaultCmd = true
 
-	root.AddCommand(summarizeCmd(&port))
+	root.AddCommand(summarizeCmd())
 	root.AddCommand(testerCmd())
 	root.AddCommand(tracerCmd())
 	root.AddCommand(keysCmd())
 	root.AddCommand(dbinfoCmd())
 	root.AddCommand(transactionsCmd())
 	root.AddCommand(planalyzeCmd())
-
-	if err := root.ParseFlags(os.Args[1:]); err != nil {
-		panic(err)
-	}
-
-	if !webserverStarted && port > 0 {
-		webserverStarted = true
-		go startWebServer(port, ch)
-	} else {
-		ch <- 1
-	}
-
-	// FIXME: add sync b/w webserver and root command, for now just add a wait to make sure webserver is running
-	time.Sleep(2 * time.Second)
-
-	err := root.Execute()
-	if err != nil {
-		fmt.Printf("Error: %v\n", err)
-		os.Exit(1)
-	}
-	<-ch
-}
-
-func startWebServer(port int64, ch chan int) {
-	if port > 0 && port != 8080 {
-		panic("(FIXME: make port configurable) Port is not 8080")
-	}
-	web.Run(port)
-	if os.WriteFile("/dev/stderr", []byte("Web server is running, use Ctrl-C to exit\n"), 0o600) != nil {
-		panic("Failed to write to /dev/stderr")
-	}
-	ch <- 1
+	return root
 }

go/cmd/summarize.go

@@ -22,10 +22,11 @@ import (
 	"github.com/vitessio/vt/go/summarize"
 )
 
-func summarizeCmd(port *int64) *cobra.Command {
+func summarizeCmd() *cobra.Command {
 	var hotMetric string
 	var showGraph bool
 	var outputFormat string
+	var launchWebServer bool
 
 	cmd := &cobra.Command{
 		Use:     "summarize old_file.json [new_file.json]",
@@ -34,13 +35,13 @@ func summarizeCmd(port *int64) *cobra.Command {
 		Example: "vt summarize old.json new.json",
 		Args:    cobra.RangeArgs(1, 2),
 		Run: func(_ *cobra.Command, args []string) {
-			summarize.Run(args, hotMetric, showGraph, outputFormat, port)
+			summarize.Run(args, hotMetric, showGraph, outputFormat, launchWebServer)
 		},
 	}
 
 	cmd.Flags().StringVar(&hotMetric, "hot-metric", "total-time", "Metric to determine hot queries (options: usage-count, total-rows-examined, avg-rows-examined, avg-time, total-time)")
 	cmd.Flags().BoolVar(&showGraph, "graph", false, "Show the query graph in the browser")
-	cmd.Flags().StringVar(&outputFormat, "format", "html", "Output format (options: html, markdown)")
-
+	cmd.Flags().StringVar(&outputFormat, "format", "markdown", "Output format (options: html, markdown)")
+	cmd.Flags().BoolVar(&launchWebServer, "web", false, "Start a web server to view the summary")
 	return cmd
 }

go/keys/keys.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 	"sort"
 
 	querypb "vitess.io/vitess/go/vt/proto/query"
@@ -73,11 +72,7 @@ type (
 	}
 )
 
-func Run(cfg Config) error {
-	return run(os.Stdout, cfg)
-}
-
-func run(out io.Writer, cfg Config) error {
+func Run(out io.Writer, cfg Config) error {
 	si := &SchemaInfo{
 		Tables: make(map[string]Columns),
 	}

go/keys/keys_test.go

@@ -65,7 +65,7 @@ func TestKeys(t *testing.T) {
 	for _, tcase := range cases {
 		t.Run(tcase.expectedFile, func(t *testing.T) {
 			sb := &strings.Builder{}
-			err := run(sb, tcase.cfg)
+			err := Run(sb, tcase.cfg)
 			require.NoError(t, err)
 
 			out, err := os.ReadFile("../testdata/keys-output/" + tcase.expectedFile)

go/summarize/summarize.go

@@ -17,7 +17,6 @@ limitations under the License.
 package summarize
 
 import (
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -31,6 +30,7 @@ import (
 	"golang.org/x/term"
 
 	"github.com/vitessio/vt/go/data"
+	"github.com/vitessio/vt/go/web"
 )
 
 type (
@@ -40,12 +40,22 @@ type (
 	}
 )
 
+type SummaryOutput struct {
+	Summary
+	DateOfAnalysis string
+}
+
 type summaryWorker = func(s *Summary) error
 
-func Run(files []string, hotMetric string, showGraph bool, outputFormat string, port *int64) {
+func Run(files []string, hotMetric string, showGraph bool, outputFormat string, launchWebServer bool) {
 	var traces []traceSummary
 	var workers []summaryWorker
 
+	if launchWebServer && outputFormat != "html" {
+		fmt.Println("cannot use --web flag without --format=html")
+		os.Exit(1)
+	}
+
 	for _, file := range files {
 		typ, err := data.GetFileType(file)
 		exitIfError(err)
@@ -77,7 +87,7 @@ func Run(files []string, hotMetric string, showGraph bool, outputFormat string,
 
 	traceCount := len(traces)
 	if traceCount <= 0 {
-		s, err := printSummary(hotMetric, workers, outputFormat, port)
+		s, err := printSummary(hotMetric, workers, outputFormat, launchWebServer)
 		exitIfError(err)
 		if showGraph {
 			err := renderQueryGraph(s)
@@ -106,7 +116,7 @@ func exitIfError(err error) {
 	os.Exit(1)
 }
 
-func printSummary(hotMetric string, workers []summaryWorker, outputFormat string, port *int64) (*Summary, error) {
+func printSummary(hotMetric string, workers []summaryWorker, outputFormat string, launchWebServer bool) (*Summary, error) {
 	s, err := NewSummary(hotMetric)
 	if err != nil {
 		return nil, err
@@ -118,25 +128,27 @@ func printSummary(hotMetric string, workers []summaryWorker, outputFormat string
 		}
 	}
 	outputFormat = strings.ToLower(outputFormat)
-	if *port == 0 && outputFormat == "html" {
-		fmt.Println("port is required when output format is html")
-		os.Exit(1)
-	}
 	switch outputFormat {
 	case "html":
-		summaryJSON, err := json.Marshal(*s)
-		if err != nil {
-			fmt.Println("Error marshalling summary:", err)
-			return nil, err
+		summarizeOutput := SummaryOutput{
+			Summary:        *s,
+			DateOfAnalysis: time.Now().Format(time.DateTime),
 		}
 
-		tmpFile, err := writeToTempFile(summaryJSON)
-		if err != nil {
-			return s, err
-		}
-		err = launchInBrowser(tmpFile)
-		if err != nil {
-			return s, err
+		if launchWebServer {
+			err = launchInBrowser(summarizeOutput)
+			if err != nil {
+				return s, err
+			}
+		} else {
+			html, err := web.RenderFile("summarize.html", "layout_standalone.html", summarizeOutput)
+			if err != nil {
+				return nil, err
+			}
+			_, err = io.Copy(os.Stdout, html)
+			if err != nil {
+				return nil, err
+			}
 		}
 	case "markdown":
 		// Print the response
@@ -150,16 +162,30 @@ func printSummary(hotMetric string, workers []summaryWorker, outputFormat string
 	return s, nil
 }
 
-func launchInBrowser(tmpFile *os.File) error {
+func launchInBrowser(summarizeOutput SummaryOutput) error {
+	html, err := web.RenderFile("summarize.html", "layout.html", summarizeOutput)
+	if err != nil {
+		return err
+	}
+	tmpFile, err := writeToTempFile(html.Bytes())
+	if err != nil {
+		return err
+	}
+
+	ch := make(chan error)
+	go func() {
+		web.Run(8080)
+		ch <- nil
+	}()
 	port := int64(8080) // FIXME: take this from flags
 	url := fmt.Sprintf("http://localhost:%d/summarize?file=", port) + tmpFile.Name()
-	err := exec.Command("open", url).Start()
+	err = exec.Command("open", url).Start()
 	if err != nil {
 		fmt.Println("Error launching browser:", err)
 		return err
 	}
 	fmt.Println("URL launched in default browser:", url)
-	return nil
+	return <-ch
 }
 
 func writeToTempFile(summaryJSON []byte) (*os.File, error) {