Refactor pprof labels and process desc (#32909)

* Deprecate "gopid" in log, it is not useful and requires very hacky
approach
* Remove "git.Command.SetDescription" because it is not useful and only
makes the logs too flexible
This commit is contained in:
wxiaoguang 2024-12-20 12:17:14 +08:00 committed by GitHub
parent c66de245c4
commit 52b319bc00
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
31 changed files with 182 additions and 247 deletions

View file

@ -12,6 +12,7 @@ import (
"io"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"time"
@ -43,18 +44,24 @@ type Command struct {
prog string
args []string
parentContext context.Context
desc string
globalArgsLength int
brokenArgs []string
}
func (c *Command) String() string {
return c.toString(false)
func logArgSanitize(arg string) string {
if strings.Contains(arg, "://") && strings.Contains(arg, "@") {
return util.SanitizeCredentialURLs(arg)
} else if filepath.IsAbs(arg) {
base := filepath.Base(arg)
dir := filepath.Dir(arg)
return filepath.Join(filepath.Base(dir), base)
}
return arg
}
func (c *Command) toString(sanitizing bool) string {
func (c *Command) LogString() string {
// WARNING: this function is for debugging purposes only. It's much better than old code (which only joins args with space),
// It's impossible to make a simple and 100% correct implementation of argument quoting for different platforms.
// It's impossible to make a simple and 100% correct implementation of argument quoting for different platforms here.
debugQuote := func(s string) string {
if strings.ContainsAny(s, " `'\"\t\r\n") {
return fmt.Sprintf("%q", s)
@ -63,12 +70,11 @@ func (c *Command) toString(sanitizing bool) string {
}
a := make([]string, 0, len(c.args)+1)
a = append(a, debugQuote(c.prog))
for _, arg := range c.args {
if sanitizing && (strings.Contains(arg, "://") && strings.Contains(arg, "@")) {
a = append(a, debugQuote(util.SanitizeCredentialURLs(arg)))
} else {
a = append(a, debugQuote(arg))
}
if c.globalArgsLength > 0 {
a = append(a, "...global...")
}
for i := c.globalArgsLength; i < len(c.args); i++ {
a = append(a, debugQuote(logArgSanitize(c.args[i])))
}
return strings.Join(a, " ")
}
@ -112,12 +118,6 @@ func (c *Command) SetParentContext(ctx context.Context) *Command {
return c
}
// SetDescription sets the description for this command which be returned on c.String()
func (c *Command) SetDescription(desc string) *Command {
c.desc = desc
return c
}
// isSafeArgumentValue checks if the argument is safe to be used as a value (not an option)
func isSafeArgumentValue(s string) bool {
return s == "" || s[0] != '-'
@ -271,8 +271,12 @@ var ErrBrokenCommand = errors.New("git command is broken")
// Run runs the command with the RunOpts
func (c *Command) Run(opts *RunOpts) error {
return c.run(1, opts)
}
func (c *Command) run(skip int, opts *RunOpts) error {
if len(c.brokenArgs) != 0 {
log.Error("git command is broken: %s, broken args: %s", c.String(), strings.Join(c.brokenArgs, " "))
log.Error("git command is broken: %s, broken args: %s", c.LogString(), strings.Join(c.brokenArgs, " "))
return ErrBrokenCommand
}
if opts == nil {
@ -285,20 +289,14 @@ func (c *Command) Run(opts *RunOpts) error {
timeout = defaultCommandExecutionTimeout
}
if len(opts.Dir) == 0 {
log.Debug("git.Command.Run: %s", c)
} else {
log.Debug("git.Command.RunDir(%s): %s", opts.Dir, c)
}
desc := c.desc
if desc == "" {
if opts.Dir == "" {
desc = fmt.Sprintf("git: %s", c.toString(true))
} else {
desc = fmt.Sprintf("git(dir:%s): %s", opts.Dir, c.toString(true))
}
var desc string
callerInfo := util.CallerFuncName(1 /* util */ + 1 /* this */ + skip /* parent */)
if pos := strings.LastIndex(callerInfo, "/"); pos >= 0 {
callerInfo = callerInfo[pos+1:]
}
// these logs are for debugging purposes only, so no guarantee of correctness or stability
desc = fmt.Sprintf("git.Run(by:%s, repo:%s): %s", callerInfo, logArgSanitize(opts.Dir), c.LogString())
log.Debug("git.Command: %s", desc)
var ctx context.Context
var cancel context.CancelFunc
@ -401,7 +399,7 @@ func IsErrorExitCode(err error, code int) bool {
// RunStdString runs the command with options and returns stdout/stderr as string. and store stderr to returned error (err combined with stderr).
func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr RunStdError) {
stdoutBytes, stderrBytes, err := c.RunStdBytes(opts)
stdoutBytes, stderrBytes, err := c.runStdBytes(opts)
stdout = util.UnsafeBytesToString(stdoutBytes)
stderr = util.UnsafeBytesToString(stderrBytes)
if err != nil {
@ -413,6 +411,10 @@ func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr Run
// RunStdBytes runs the command with options and returns stdout/stderr as bytes. and store stderr to returned error (err combined with stderr).
func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) {
return c.runStdBytes(opts)
}
func (c *Command) runStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) {
if opts == nil {
opts = &RunOpts{}
}
@ -435,7 +437,7 @@ func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS
PipelineFunc: opts.PipelineFunc,
}
err := c.Run(newOpts)
err := c.run(2, newOpts)
stderr = stderrBuf.Bytes()
if err != nil {
return nil, stderr, &runStdError{err: err, stderr: util.UnsafeBytesToString(stderr)}