From 256908116563bf895448ba4785d92b870d1d38e6 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 4 Aug 2023 08:03:47 -0400 Subject: [PATCH] chore: minor refactoring in the grafana server cli command (#72847) This is preparative work for extending the grafana server cli command to target individual dskit modules that seemed worth breaking into this smaller PR. This moves the CLI flags and various reusable chunks of code into variables and methods. --- go.mod | 2 + go.sum | 5 + pkg/cmd/grafana-server/commands/buildinfo.go | 25 +++ pkg/cmd/grafana-server/commands/cli.go | 187 ++++-------------- .../grafana-server/commands/diagnostics.go | 54 +++++ pkg/cmd/grafana-server/commands/flags.go | 90 +++++++++ 6 files changed, 209 insertions(+), 154 deletions(-) create mode 100644 pkg/cmd/grafana-server/commands/buildinfo.go create mode 100644 pkg/cmd/grafana-server/commands/flags.go diff --git a/go.mod b/go.mod index 99347c72b46..9c136c83306 100644 --- a/go.mod +++ b/go.mod @@ -294,6 +294,7 @@ require ( github.com/Masterminds/goutils v1.1.1 // indirect github.com/NYTimes/gziphandler v1.1.1 // indirect github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect + github.com/alvaroloes/enumer v1.1.2 // indirect github.com/antlr/antlr4/runtime/Go/antlr v1.4.10 // indirect github.com/apache/arrow/go/v12 v12.0.1 // indirect github.com/apache/thrift v0.18.1 // indirect @@ -371,6 +372,7 @@ require ( github.com/ory/go-convenience v0.1.0 // indirect github.com/ory/viper v1.7.5 // indirect github.com/ory/x v0.0.214 // indirect + github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1 // indirect github.com/pborman/uuid v1.2.0 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/perimeterx/marshmallow v1.1.4 // indirect diff --git a/go.sum b/go.sum index 81a10944b7d..361817f81e7 100644 --- a/go.sum +++ b/go.sum @@ -685,6 +685,8 @@ github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a h1:HbKu58rmZp github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc= github.com/alicebob/miniredis/v2 v2.30.1 h1:HM1rlQjq1bm9yQcsawJqSZBJ9AYgxvjkMsNtddh90+g= github.com/alicebob/miniredis/v2 v2.30.1/go.mod h1:b25qWj4fCEsBeAAR2mlb0ufImGC6uH3VlUfb/HS5zKg= +github.com/alvaroloes/enumer v1.1.2 h1:5khqHB33TZy1GWCO/lZwcroBFh7u+0j40T83VUbfAMY= +github.com/alvaroloes/enumer v1.1.2/go.mod h1:FxrjvuXoDAx9isTJrv4c+T410zFi0DtXIT0m65DJ+Wo= github.com/andybalholm/brotli v1.0.4 h1:V7DdXeJtZscaqfNuAdSRuRFzuiKlHSC/Zh3zl9qY3JY= github.com/andybalholm/brotli v1.0.4/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= @@ -2463,6 +2465,8 @@ github.com/parnurzeal/gorequest v0.2.15/go.mod h1:3Kh2QUMJoqw3icWAecsyzkpY7UzRfD github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY= github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= +github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1 h1:/I3lTljEEDNYLho3/FUB7iD/oc2cEFgVmbHzV+O0PtU= +github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1/go.mod h1:eD5JxqMiuNYyFNmyY9rkJ/slN8y59oEu4Ei7F8OoKWQ= github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc= github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g= @@ -3588,6 +3592,7 @@ golang.org/x/tools v0.0.0-20190425163242-31fd60d6bfdc/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/tools v0.0.0-20190425222832-ad9eeb80039a/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= +golang.org/x/tools v0.0.0-20190524210228-3d17549cdc6b/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190531172133-b3315ee88b7d/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190613204242-ed0dc450797f/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= diff --git a/pkg/cmd/grafana-server/commands/buildinfo.go b/pkg/cmd/grafana-server/commands/buildinfo.go new file mode 100644 index 00000000000..d6d56c09b31 --- /dev/null +++ b/pkg/cmd/grafana-server/commands/buildinfo.go @@ -0,0 +1,25 @@ +package commands + +import ( + "strconv" + "time" + + "github.com/grafana/grafana/pkg/extensions" + "github.com/grafana/grafana/pkg/infra/metrics" + "github.com/grafana/grafana/pkg/setting" +) + +func setBuildInfo(opts ServerOptions) { + buildstampInt64, err := strconv.ParseInt(opts.BuildStamp, 10, 64) + if err != nil || buildstampInt64 == 0 { + buildstampInt64 = time.Now().Unix() + } + setting.BuildVersion = opts.Version + setting.BuildCommit = opts.Commit + setting.BuildStamp = buildstampInt64 + setting.BuildBranch = opts.BuildBranch + setting.IsEnterprise = extensions.IsEnterprise + setting.Packaging = validPackaging(Packaging) + + metrics.SetBuildInformation(opts.Version, opts.Commit, opts.BuildBranch, buildstampInt64) +} diff --git a/pkg/cmd/grafana-server/commands/cli.go b/pkg/cmd/grafana-server/commands/cli.go index 293bdb713c7..884740335d9 100644 --- a/pkg/cmd/grafana-server/commands/cli.go +++ b/pkg/cmd/grafana-server/commands/cli.go @@ -3,14 +3,10 @@ package commands import ( "context" "fmt" - "net/http" _ "net/http/pprof" "os" "os/signal" - "runtime" "runtime/debug" - "runtime/trace" - "strconv" "strings" "syscall" "time" @@ -18,9 +14,7 @@ import ( "github.com/urfave/cli/v2" "github.com/grafana/grafana/pkg/api" - "github.com/grafana/grafana/pkg/extensions" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/process" "github.com/grafana/grafana/pkg/server" _ "github.com/grafana/grafana/pkg/services/alerting/conditions" @@ -40,59 +34,7 @@ func ServerCommand(version, commit, buildBranch, buildstamp string) *cli.Command return &cli.Command{ Name: "server", Usage: "run the grafana server", - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: "config", - Usage: "Path to config file", - }, - &cli.StringFlag{ - Name: "homepath", - Usage: "Path to Grafana install/home path, defaults to working directory", - }, - &cli.StringFlag{ - Name: "pidfile", - Usage: "Path to Grafana pid file", - }, - &cli.StringFlag{ - Name: "packaging", - Value: "unknown", - Usage: "describes the way Grafana was installed", - }, - &cli.StringFlag{ - Name: "configOverrides", - Usage: "Configuration options to override defaults as a string. e.g. cfg:default.paths.log=/dev/null", - }, - cli.VersionFlag, - &cli.BoolFlag{ - Name: "vv", - Usage: "prints current version, all dependencies and exits", - }, - &cli.BoolFlag{ - Name: "profile", - Value: false, - Usage: "Turn on pprof profiling", - }, - &cli.StringFlag{ - Name: "profile-addr", - Value: "localhost", - Usage: "Define custom address for profiling", - }, - &cli.Uint64Flag{ - Name: "profile-port", - Value: 6060, - Usage: "Define custom port for profiling", - }, - &cli.BoolFlag{ - Name: "tracing", - Value: false, - Usage: "Turn on tracing", - }, - &cli.StringFlag{ - Name: "tracing-file", - Value: "trace.out", - Usage: "Define tracing output file", - }, - }, + Flags: commonFlags, Action: func(context *cli.Context) error { return RunServer(ServerOptions{ Version: version, @@ -105,27 +47,10 @@ func ServerCommand(version, commit, buildBranch, buildstamp string) *cli.Command } } -func RunServer(opt ServerOptions) error { - var ( - configFile = opt.Context.String("config") - homePath = opt.Context.String("homepath") - pidFile = opt.Context.String("pidfile") - packaging = opt.Context.String("packaging") - - configOverrides = opt.Context.String("configOverrides") - - v = opt.Context.Bool("version") - vv = opt.Context.Bool("vv") - profile = opt.Context.Bool("profile") - profileAddr = opt.Context.String("profile-addr") - profilePort = opt.Context.Uint64("profile-port") - tracing = opt.Context.Bool("tracing") - tracingFile = opt.Context.String("tracing-file") - ) - - if v || vv { - fmt.Printf("Version %s (commit: %s, branch: %s)\n", opt.Version, opt.Commit, opt.BuildBranch) - if vv { +func RunServer(opts ServerOptions) error { + if Version || VerboseVersion { + fmt.Printf("Version %s (commit: %s, branch: %s)\n", opts.Version, opts.Commit, opts.BuildBranch) + if VerboseVersion { fmt.Println("Dependencies:") if info, ok := debug.ReadBuildInfo(); ok { for _, dep := range info.Deps { @@ -136,38 +61,19 @@ func RunServer(opt ServerOptions) error { return nil } - profileDiagnostics := newProfilingDiagnostics(profile, profileAddr, profilePort) - if err := profileDiagnostics.overrideWithEnv(); err != nil { - return err - } - - traceDiagnostics := newTracingDiagnostics(tracing, tracingFile) - if err := traceDiagnostics.overrideWithEnv(); err != nil { - return err - } - - if profileDiagnostics.enabled { - fmt.Println("diagnostics: pprof profiling enabled", "addr", profileDiagnostics.addr, "port", profileDiagnostics.port) - runtime.SetBlockProfileRate(1) - go func() { - // TODO: We should enable the linter and fix G114 here. - // G114: Use of net/http serve function that has no support for setting timeouts (gosec) - // - //nolint:gosec - err := http.ListenAndServe(fmt.Sprintf("%s:%d", profileDiagnostics.addr, profileDiagnostics.port), nil) - if err != nil { - panic(err) - } - }() - } - + logger := log.New("cli") defer func() { if err := log.Close(); err != nil { fmt.Fprintf(os.Stderr, "Failed to close log: %s\n", err) } }() - clilog := log.New("cli") + if err := setupProfiling(Profile, ProfileAddr, ProfilePort); err != nil { + return err + } + if err := setupTracing(Tracing, TracingFile, logger); err != nil { + return err + } defer func() { // If we've managed to initialize them, this is the last place @@ -178,65 +84,28 @@ func RunServer(opt ServerOptions) error { // our regular log locations before exiting. if r := recover(); r != nil { reason := fmt.Sprintf("%v", r) - clilog.Error("Critical error", "reason", reason, "stackTrace", string(debug.Stack())) + logger.Error("Critical error", "reason", reason, "stackTrace", string(debug.Stack())) panic(r) } }() - if traceDiagnostics.enabled { - fmt.Println("diagnostics: tracing enabled", "file", traceDiagnostics.file) - f, err := os.Create(traceDiagnostics.file) - if err != nil { - panic(err) - } - defer func() { - if err := f.Close(); err != nil { - clilog.Error("Failed to write trace diagnostics", "path", traceDiagnostics.file, "err", err) - } - }() + setBuildInfo(opts) + checkPrivileges() - if err := trace.Start(f); err != nil { - panic(err) - } - defer trace.Stop() - } - - buildstampInt64, err := strconv.ParseInt(opt.BuildStamp, 10, 64) - if err != nil || buildstampInt64 == 0 { - buildstampInt64 = time.Now().Unix() - } - - setting.BuildVersion = opt.Version - setting.BuildCommit = opt.Commit - setting.BuildStamp = buildstampInt64 - setting.BuildBranch = opt.BuildBranch - setting.IsEnterprise = extensions.IsEnterprise - setting.Packaging = validPackaging(packaging) - - metrics.SetBuildInformation(opt.Version, opt.Commit, opt.BuildBranch, buildstampInt64) - - elevated, err := process.IsRunningWithElevatedPrivileges() - if err != nil { - fmt.Fprintf(os.Stderr, "Error checking server process execution privilege. error: %s\n", err.Error()) - } - if elevated { - fmt.Println("Grafana server is running with elevated privileges. This is not recommended") - } - - configOptions := strings.Split(configOverrides, " ") + configOptions := strings.Split(ConfigOverrides, " ") s, err := server.Initialize( setting.CommandLineArgs{ - Config: configFile, - HomePath: homePath, + Config: ConfigFile, + HomePath: HomePath, // tailing arguments have precedence over the options string - Args: append(configOptions, opt.Context.Args().Slice()...), + Args: append(configOptions, opts.Context.Args().Slice()...), }, server.Options{ - PidFile: pidFile, - Version: opt.Version, - Commit: opt.Commit, - BuildBranch: opt.BuildBranch, + PidFile: PidFile, + Version: opts.Version, + Commit: opts.Commit, + BuildBranch: opts.BuildBranch, }, api.ServerOptions{}, ) @@ -284,3 +153,13 @@ func listenToSystemSignals(ctx context.Context, s *server.Server) { } } } + +func checkPrivileges() { + elevated, err := process.IsRunningWithElevatedPrivileges() + if err != nil { + fmt.Fprintf(os.Stderr, "Error checking server process execution privilege. error: %s\n", err.Error()) + } + if elevated { + fmt.Println("Grafana server is running with elevated privileges. This is not recommended") + } +} diff --git a/pkg/cmd/grafana-server/commands/diagnostics.go b/pkg/cmd/grafana-server/commands/diagnostics.go index 97b644109fa..06a992b45b8 100644 --- a/pkg/cmd/grafana-server/commands/diagnostics.go +++ b/pkg/cmd/grafana-server/commands/diagnostics.go @@ -2,8 +2,13 @@ package commands import ( "fmt" + "net/http" "os" + "runtime" + "runtime/trace" "strconv" + + "github.com/grafana/grafana/pkg/infra/log" ) const ( @@ -84,3 +89,52 @@ func (td *tracingDiagnostics) overrideWithEnv() error { return nil } + +func setupProfiling(profile bool, profileAddr string, profilePort uint64) error { + profileDiagnostics := newProfilingDiagnostics(profile, profileAddr, profilePort) + if err := profileDiagnostics.overrideWithEnv(); err != nil { + return err + } + + if profileDiagnostics.enabled { + fmt.Println("diagnostics: pprof profiling enabled", "addr", profileDiagnostics.addr, "port", profileDiagnostics.port) + runtime.SetBlockProfileRate(1) + go func() { + // TODO: We should enable the linter and fix G114 here. + // G114: Use of net/http serve function that has no support for setting timeouts (gosec) + // + //nolint:gosec + err := http.ListenAndServe(fmt.Sprintf("%s:%d", profileDiagnostics.addr, profileDiagnostics.port), nil) + if err != nil { + panic(err) + } + }() + } + return nil +} + +func setupTracing(tracing bool, tracingFile string, logger *log.ConcreteLogger) error { + traceDiagnostics := newTracingDiagnostics(tracing, tracingFile) + if err := traceDiagnostics.overrideWithEnv(); err != nil { + return err + } + + if traceDiagnostics.enabled { + fmt.Println("diagnostics: tracing enabled", "file", traceDiagnostics.file) + f, err := os.Create(traceDiagnostics.file) + if err != nil { + panic(err) + } + defer func() { + if err := f.Close(); err != nil { + logger.Error("Failed to write trace diagnostics", "path", traceDiagnostics.file, "err", err) + } + }() + + if err := trace.Start(f); err != nil { + panic(err) + } + defer trace.Stop() + } + return nil +} diff --git a/pkg/cmd/grafana-server/commands/flags.go b/pkg/cmd/grafana-server/commands/flags.go new file mode 100644 index 00000000000..8749ce96ca0 --- /dev/null +++ b/pkg/cmd/grafana-server/commands/flags.go @@ -0,0 +1,90 @@ +package commands + +import "github.com/urfave/cli/v2" + +// flags for the grafana server command(s) +var ( + ConfigFile string + HomePath string + PidFile string + Packaging string + ConfigOverrides string + Version bool + VerboseVersion bool + Profile bool + ProfileAddr string + ProfilePort uint64 + Tracing bool + TracingFile string +) + +var commonFlags = []cli.Flag{ + &cli.StringFlag{ + Name: "config", + Usage: "Path to config file", + Destination: &ConfigFile, + }, + &cli.StringFlag{ + Name: "homepath", + Usage: "Path to Grafana install/home path, defaults to working directory", + Destination: &HomePath, + }, + &cli.StringFlag{ + Name: "pidfile", + Usage: "Path to Grafana pid file", + Destination: &PidFile, + }, + &cli.StringFlag{ + Name: "packaging", + Value: "unknown", + Usage: "describes the way Grafana was installed", + Destination: &Packaging, + }, + &cli.StringFlag{ + Name: "configOverrides", + Usage: "Configuration options to override defaults as a string. e.g. cfg:default.paths.log=/dev/null", + Destination: &ConfigOverrides, + }, + &cli.BoolFlag{ + Name: "version", + Aliases: []string{"v"}, + Usage: "print the version", + DisableDefaultText: true, + Destination: &Version, + }, + &cli.BoolFlag{ + Name: "vv", + Usage: "prints current version, all dependencies and exits", + Destination: &VerboseVersion, + }, + &cli.BoolFlag{ + Name: "profile", + Value: false, + Usage: "Turn on pprof profiling", + Destination: &Profile, + }, + &cli.StringFlag{ + Name: "profile-addr", + Value: "localhost", + Usage: "Define custom address for profiling", + Destination: &ProfileAddr, + }, + &cli.Uint64Flag{ + Name: "profile-port", + Value: 6060, + Usage: "Define custom port for profiling", + Destination: &ProfilePort, + }, + &cli.BoolFlag{ + Name: "tracing", + Value: false, + Usage: "Turn on tracing", + Destination: &Tracing, + }, + &cli.StringFlag{ + Name: "tracing-file", + Value: "trace.out", + Usage: "Define tracing output file", + Destination: &TracingFile, + }, +}