adds order to config fields, adds a sample config generator program, fixes #66

Author: petethepig

pkg/cli/cli.go

@@ -73,7 +73,7 @@ func (tf *timeFlag) Set(value string) error {
 }
 
 // this is mostly reflection magic
-func populateFlagSet(obj interface{}, flagSet *flag.FlagSet) {
+func PopulateFlagSet(obj interface{}, flagSet *flag.FlagSet) *SortedFlags {
 	v := reflect.ValueOf(obj).Elem()
 	t := reflect.TypeOf(v.Interface())
 	num := t.NumField()
@@ -142,6 +142,7 @@ func populateFlagSet(obj interface{}, flagSet *flag.FlagSet) {
 			log.Fatalf("type %s is not supported", field.Type)
 		}
 	}
+	return NewSortedFlags(obj, flagSet)
 }
 
 // on mac pyroscope is usually installed via homebrew. homebrew installs under a prefix
@@ -173,26 +174,22 @@ func resolvePath(path string) string {
 	return path
 }
 
-func printUsage(c *ffcli.Command) string {
-	return gradientBanner() + "\n" + DefaultUsageFunc(c)
-}
-
 func Start(cfg *config.Config) error {
 	var (
-		rootFlagSet      = flag.NewFlagSet("pyroscope", flag.ExitOnError)
 		agentFlagSet     = flag.NewFlagSet("pyroscope agent", flag.ExitOnError)
 		serverFlagSet    = flag.NewFlagSet("pyroscope server", flag.ExitOnError)
 		convertFlagSet   = flag.NewFlagSet("pyroscope convert", flag.ExitOnError)
 		execFlagSet      = flag.NewFlagSet("pyroscope exec", flag.ExitOnError)
 		dbmanagerFlagSet = flag.NewFlagSet("pyroscope dbmanager", flag.ExitOnError)
+		rootFlagSet      = flag.NewFlagSet("pyroscope", flag.ExitOnError)
 	)
 
-	populateFlagSet(cfg, rootFlagSet)
-	populateFlagSet(&cfg.Agent, agentFlagSet)
-	populateFlagSet(&cfg.Server, serverFlagSet)
-	populateFlagSet(&cfg.Convert, convertFlagSet)
-	populateFlagSet(&cfg.Exec, execFlagSet)
-	populateFlagSet(&cfg.DbManager, dbmanagerFlagSet)
+	agentSortedFlags := PopulateFlagSet(&cfg.Agent, agentFlagSet)
+	serverSortedFlags := PopulateFlagSet(&cfg.Server, serverFlagSet)
+	convertSortedFlags := PopulateFlagSet(&cfg.Convert, convertFlagSet)
+	execSortedFlags := PopulateFlagSet(&cfg.Exec, execFlagSet)
+	dbmanagerSortedFlags := PopulateFlagSet(&cfg.DbManager, dbmanagerFlagSet)
+	rootSortedFlags := PopulateFlagSet(cfg, rootFlagSet)
 
 	options := []ff.Option{
 		ff.WithConfigFileParser(ffyaml.Parser),
@@ -202,7 +199,7 @@ func Start(cfg *config.Config) error {
 	}
 
 	agentCmd := &ffcli.Command{
-		UsageFunc:  printUsage,
+		UsageFunc:  agentSortedFlags.printUsage,
 		Options:    options,
 		Name:       "agent",
 		ShortUsage: "pyroscope agent [flags]",
@@ -211,7 +208,7 @@ func Start(cfg *config.Config) error {
 	}
 
 	serverCmd := &ffcli.Command{
-		UsageFunc:  printUsage,
+		UsageFunc:  serverSortedFlags.printUsage,
 		Options:    options,
 		Name:       "server",
 		ShortUsage: "pyroscope server [flags]",
@@ -220,7 +217,7 @@ func Start(cfg *config.Config) error {
 	}
 
 	convertCmd := &ffcli.Command{
-		UsageFunc:  printUsage,
+		UsageFunc:  convertSortedFlags.printUsage,
 		Options:    options,
 		Name:       "convert",
 		ShortUsage: "pyroscope convert [flags] <input-file>",
@@ -229,7 +226,7 @@ func Start(cfg *config.Config) error {
 	}
 
 	execCmd := &ffcli.Command{
-		UsageFunc:  printUsage,
+		UsageFunc:  execSortedFlags.printUsage,
 		Options:    options,
 		Name:       "exec",
 		ShortUsage: "pyroscope exec [flags] <args>",
@@ -238,7 +235,7 @@ func Start(cfg *config.Config) error {
 	}
 
 	dbmanagerCmd := &ffcli.Command{
-		UsageFunc:  printUsage,
+		UsageFunc:  dbmanagerSortedFlags.printUsage,
 		Options:    options,
 		Name:       "dbmanager",
 		ShortUsage: "pyroscope dbmanager [flags] <args>",
@@ -247,7 +244,7 @@ func Start(cfg *config.Config) error {
 	}
 
 	rootCmd := &ffcli.Command{
-		UsageFunc:  printUsage,
+		UsageFunc:  rootSortedFlags.printUsage,
 		Options:    options,
 		ShortUsage: "pyroscope [flags] <subcommand>",
 		FlagSet:    rootFlagSet,
@@ -287,7 +284,7 @@ func Start(cfg *config.Config) error {
 		}
 		if len(args) == 0 || args[0] == "help" {
 			fmt.Println(gradientBanner())
-			fmt.Println(DefaultUsageFunc(execCmd))
+			fmt.Println(DefaultUsageFunc(execSortedFlags, execCmd))
 			return nil
 		}
 
@@ -306,7 +303,7 @@ func Start(cfg *config.Config) error {
 			fmt.Println("")
 		} else {
 			fmt.Println(gradientBanner())
-			fmt.Println(DefaultUsageFunc(rootCmd))
+			fmt.Println(DefaultUsageFunc(rootSortedFlags, rootCmd))
 		}
 		return nil
 	}

pkg/cli/cli_test.go

@@ -22,12 +22,12 @@ type FlagsStruct struct {
 }
 
 var _ = Describe("config package", func() {
-	Context("populateFlagSet", func() {
+	Context("PopulateFlagSet", func() {
 		Context("without config file", func() {
 			It("correctly sets all types of arguments", func() {
 				exampleFlagSet := flag.NewFlagSet("example flag set", flag.ExitOnError)
 				cfg := FlagsStruct{}
-				populateFlagSet(&cfg, exampleFlagSet)
+				PopulateFlagSet(&cfg, exampleFlagSet)
 
 				exampleCommand := &ffcli.Command{
 					FlagSet: exampleFlagSet,
@@ -58,7 +58,7 @@ var _ = Describe("config package", func() {
 			It("correctly sets all types of arguments", func() {
 				exampleFlagSet := flag.NewFlagSet("example flag set", flag.ExitOnError)
 				cfg := FlagsStruct{}
-				populateFlagSet(&cfg, exampleFlagSet)
+				PopulateFlagSet(&cfg, exampleFlagSet)
 
 				exampleCommand := &ffcli.Command{
 					FlagSet: exampleFlagSet,
@@ -86,7 +86,7 @@ var _ = Describe("config package", func() {
 			It("arguments take precendence", func() {
 				exampleFlagSet := flag.NewFlagSet("example flag set", flag.ExitOnError)
 				cfg := FlagsStruct{}
-				populateFlagSet(&cfg, exampleFlagSet)
+				PopulateFlagSet(&cfg, exampleFlagSet)
 
 				exampleCommand := &ffcli.Command{
 					FlagSet: exampleFlagSet,

pkg/cli/sortedflags.go

@@ -0,0 +1,68 @@
+package cli
+
+import (
+	"flag"
+	"reflect"
+	"sort"
+
+	"github.com/iancoleman/strcase"
+	"github.com/peterbourgon/ff/v3/ffcli"
+)
+
+// SortedFlags is needed because by default there's no way to provide order for flags
+//   this is kind of an ugly workaround
+type SortedFlags struct {
+	orderMap map[string]int
+	flags    []*flag.Flag
+}
+
+func (sf *SortedFlags) Len() int {
+	return len(sf.flags)
+}
+
+func (sf *SortedFlags) Swap(i, j int) {
+	sf.flags[i], sf.flags[j] = sf.flags[j], sf.flags[i]
+}
+
+func (sf *SortedFlags) Less(i, j int) bool {
+	return sf.orderMap[sf.flags[i].Name] < sf.orderMap[sf.flags[j].Name]
+}
+
+func (sf *SortedFlags) VisitAll(cb func(*flag.Flag)) {
+	for _, v := range sf.flags {
+		cb(v)
+	}
+}
+
+func (sf *SortedFlags) printUsage(c *ffcli.Command) string {
+	return gradientBanner() + "\n" + DefaultUsageFunc(sf, c)
+}
+
+func NewSortedFlags(obj interface{}, fs *flag.FlagSet) *SortedFlags {
+	v := reflect.ValueOf(obj).Elem()
+	t := reflect.TypeOf(v.Interface())
+	num := t.NumField()
+
+	res := SortedFlags{
+		orderMap: make(map[string]int),
+		flags:    []*flag.Flag{},
+	}
+
+	for i := 0; i < num; i++ {
+		field := t.Field(i)
+		nameVal := field.Tag.Get("name")
+		if nameVal == "" {
+			nameVal = strcase.ToKebab(field.Name)
+		}
+		// orderVal := field.Tag.Get("order")
+		// order, _ := strconv.Atoi(orderVal)
+		order := i
+		res.orderMap[nameVal] = order
+	}
+	fs.VisitAll(func(f *flag.Flag) {
+		res.flags = append(res.flags, f)
+	})
+
+	sort.Sort(&res)
+	return &res
+}

pkg/cli/usage.go

@@ -32,7 +32,7 @@ var hiddenCommands = []string{
 }
 
 // This is mostly copied from ffcli package
-func DefaultUsageFunc(c *ffcli.Command) string {
+func DefaultUsageFunc(sf *SortedFlags, c *ffcli.Command) string {
 	var b strings.Builder
 
 	fmt.Fprintf(&b, "continuous profiling platform\n\n")
@@ -67,7 +67,7 @@ func DefaultUsageFunc(c *ffcli.Command) string {
 
 		// TODO: it would be nice to sort by how often people would use these.
 		//   But for that we'd have to have a conversion from flag-set back to struct
-		c.FlagSet.VisitAll(func(f *flag.Flag) {
+		sf.VisitAll(func(f *flag.Flag) {
 			def := f.DefValue
 			// if def == "" {
 			// 	def = "..."

pkg/config/config.go

@@ -16,7 +16,7 @@ type Config struct {
 
 type Agent struct {
 	Config   string `def:"<installPrefix>/etc/pyroscope/agent.yml" desc:"location of config file"`
-	LogLevel string `def:"info", desc:"debug|info|warn|error"`
+	LogLevel string `def:"info", desc:"log level: debug|info|warn|error"`
 
 	// AgentCMD           []string
 	AgentSpyName           string        `desc:"name of the spy you want to use"` // TODO: add options
@@ -29,9 +29,11 @@ type Agent struct {
 }
 
 type Server struct {
+	AnalyticsOptOut bool `def:"false" desc:"disables analytics"`
+
 	Config         string `def:"<installPrefix>/etc/pyroscope/server.yml" desc:"location of config file"`
-	LogLevel       string `def:"info", desc:"debug|info|warn|error"`
-	BadgerLogLevel string `def:"error", desc:"debug|info|warn|error"`
+	LogLevel       string `def:"info", desc:"log level: debug|info|warn|error"`
+	BadgerLogLevel string `def:"error", desc:"log level: debug|info|warn|error"`
 
 	StoragePath string `def:"<installPrefix>/var/lib/pyroscope" desc:"directory where pyroscope stores profiling data"`
 	ApiBindAddr string `def:":4040" desc:"port for the HTTP server used for data ingestion and web UI"`
@@ -55,17 +57,15 @@ type Server struct {
 	MaxNodesRender        int `def:"2048" desc:"max number of nodes used to display data on the frontend"`
 
 	// currently only used in our demo app
-	HideApplications []string `def:""`
-
-	AnalyticsOptOut bool `def:"false" desc:"disables analytics"`
+	HideApplications []string `def:"" desc:"please don't use, this will soon be deprecated"`
 }
 
 type Convert struct {
 	Format string `def:"tree"`
 }
 
 type DbManager struct {
-	LogLevel        string `def:"error", desc:"debug|info|warn|error"`
+	LogLevel        string `def:"error", desc:"log level: debug|info|warn|error"`
 	StoragePath     string `def:"<installPrefix>/var/lib/pyroscope" desc:"directory where pyroscope stores profiling data"`
 	DstStartTime    time.Time
 	DstEndTime      time.Time
@@ -79,7 +79,7 @@ type Exec struct {
 	SpyName                string        `def:"auto" desc:"name of the profiler you want to use. Supported ones are: <supportedProfilers>"`
 	ApplicationName        string        `def:"" desc:"application name used when uploading profiling data"`
 	DetectSubprocesses     bool          `def:"true" desc:"makes pyroscope keep track of and profile subprocesses of the main process"`
-	LogLevel               string        `def:"info", desc:"debug|info|warn|error"`
+	LogLevel               string        `def:"info", desc:"log level: debug|info|warn|error"`
 	ServerAddress          string        `def:"http://localhost:4040" desc:"address of the pyroscope server"`
 	AuthToken              string        `def:"" desc:"authorization token used to upload profiling data"`
 	UpstreamThreads        int           `def:"4" desc:"number of upload threads"`

scripts/generate-sample-config.go

@@ -0,0 +1,45 @@
+package main
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/pyroscope-io/pyroscope/pkg/cli"
+	"github.com/pyroscope-io/pyroscope/pkg/config"
+	"github.com/sirupsen/logrus"
+)
+
+// to run this program:
+//   go run scripts/genrate-sample-config.go -format yaml
+//   go run scripts/genrate-sample-config.go -format md
+
+func main() {
+	var format string
+	flag.StringVar(&format, "format", "yaml", "yaml or md")
+	flag.Parse()
+
+	serverFlagSet := flag.NewFlagSet("pyroscope server", flag.ExitOnError)
+	cfg := config.Config{Server: config.Server{}}
+	cli.PopulateFlagSet(&cfg.Server, serverFlagSet)
+
+	sf := cli.NewSortedFlags(&cfg.Server, serverFlagSet)
+
+	if format == "yaml" {
+		fmt.Println("---")
+		sf.VisitAll(func(f *flag.Flag) {
+			if f.Name != "config" {
+				fmt.Printf("# %s\n%s: %q\n\n", f.Usage, f.Name, f.DefValue)
+			}
+		})
+	} else if format == "md" {
+		fmt.Printf("| %s | %s | %s |\n", "Name", "Default Value", "Usage")
+		fmt.Printf("| %s | %s | %s |\n", ":-", ":-", ":-")
+		sf.VisitAll(func(f *flag.Flag) {
+			if f.Name != "config" {
+				fmt.Printf("| %s | %s | %q |\n", f.Name, f.DefValue, f.Usage)
+			}
+		})
+	} else {
+		logrus.Fatalf("Unknown format %q", format)
+	}
+}