From a4a2421281f8f61f12425f9d5d0c9cd3ace0758d Mon Sep 17 00:00:00 2001 From: Jesse Donat Date: Sat, 21 Jun 2025 17:30:56 -0500 Subject: [PATCH 1/2] Clarify docs --- exec.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exec.go b/exec.go index 15ae991..e1b9032 100644 --- a/exec.go +++ b/exec.go @@ -65,8 +65,8 @@ func (h *HookExec) GetPathExecs(owner, repo, event, action string) ([]string, [] } // pathScan scans the given path for executable files -// returns a list of files and a list of error handlers -// error handlers are files that start with @@error. +// returns a list of files and a list of error handlers. +// Error handlers are files that start with @@error. func pathScan(path string) ([]string, []string, error) { files := []string{} errHandlers := []string{} From 9328f12ae7a01b2f9065be7910d0b3ad4c9d7a42 Mon Sep 17 00:00:00 2001 From: Jesse Donat Date: Sat, 21 Jun 2025 17:41:17 -0500 Subject: [PATCH 2/2] Utilize slog --- cmd/hookah/main.go | 69 +++++++++++++++++++++++++++++++++++++++------- exec.go | 7 +++-- exec_test.go | 6 ++-- server.go | 29 ++++++++++--------- 4 files changed, 80 insertions(+), 31 deletions(-) diff --git a/cmd/hookah/main.go b/cmd/hookah/main.go index 0e69984..dd746ef 100644 --- a/cmd/hookah/main.go +++ b/cmd/hookah/main.go @@ -3,7 +3,9 @@ package main import ( _ "embed" "flag" - "log" + "fmt" + "io" + "log/slog" "net/http" "os" "strconv" @@ -19,8 +21,14 @@ var ( secret = flag.String("secret", "", "Optional GitHub HMAC secret key") timeout = flag.Duration("timeout", 10*time.Minute, "Exec timeout on hook scripts") verbose = flag.Bool("v", false, "Enable verbose logger output") +) +var ( errlog = flag.String("err-log", "", "Path to write the error log to. Defaults to standard error.") + + logJSON = flag.Bool("log-json", false, "log in JSON format") + logLevel = flag.String("log-level", "info", "log level (options: debug, info, warn, error)") + logSource = flag.Bool("log-source", false, "log output includes source code location") ) //go:embed favicon.ico @@ -31,7 +39,14 @@ func init() { } func main() { - logger := getLogger(*errlog) + logger, err := getLogger(*errlog, *logLevel, *logSource, *logJSON) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to create logger: %v\n", err) + os.Exit(1) + } + + slog.SetDefault(logger) + options := []hookah.ServerOption{ hookah.ServerExecTimeout(*timeout), hookah.ServerErrorLog(logger), @@ -43,7 +58,8 @@ func main() { hServe, err := hookah.NewHookServer(*serverRoot, options...) if err != nil { - log.Fatal(err) + slog.Error("failed to create hook server", "error", err) + os.Exit(1) } var serve http.Handler = hServe @@ -58,21 +74,54 @@ func main() { w.Write(favicon) }) - logger.Println("listening on port", *httpPort) + logger.Info("listening on port", "port", *httpPort) err = http.ListenAndServe(":"+strconv.Itoa(int(*httpPort)), mux) if err != nil { - log.Fatal(err) + slog.Error("failed to start HTTP server", "error", err) + os.Exit(1) } } -func getLogger(filename string) hookah.Logger { +func getLogger(filename, logLevel string, addSource, json bool) (*slog.Logger, error) { + var handler slog.Handler + + var level slog.Level + switch logLevel { + case "debug": + level = slog.LevelDebug + case "info": + level = slog.LevelInfo + case "warn": + level = slog.LevelWarn + case "error": + level = slog.LevelError + default: + slog.Error("unknown log level", "level", logLevel) + os.Exit(3) + } + + opts := &slog.HandlerOptions{ + AddSource: addSource, + Level: level, + } + + var w io.Writer if filename != "" { - f, err := os.OpenFile(*errlog, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) + f, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) if err != nil { - log.Fatal(err) + return nil, fmt.Errorf("failed to open log file: %w", err) } - return log.New(f, "", log.LstdFlags) + + w = f + } else { + w = os.Stderr + } + + if json { + handler = slog.NewJSONHandler(w, opts) + } else { + handler = slog.NewTextHandler(w, opts) } - return log.New(os.Stderr, "", log.LstdFlags) + return slog.New(handler), nil } diff --git a/exec.go b/exec.go index e1b9032..e6da3c1 100644 --- a/exec.go +++ b/exec.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "log/slog" "os" "os/exec" "path/filepath" @@ -19,7 +20,7 @@ import ( type HookExec struct { RootDir string Data io.ReadSeeker - InfoLog Logger + InfoLog *slog.Logger Stdout io.Writer Stderr io.Writer @@ -124,13 +125,13 @@ func pathScan(path string) ([]string, []string, error) { // InfoLogf logs to the info logger if not nil func (h *HookExec) InfoLogf(format string, v ...any) { if h.InfoLog != nil { - h.InfoLog.Printf(format, v...) + h.InfoLog.Info(fmt.Sprintf(format, v...)) } } func (h *HookExec) InfoLogln(msg string) { if h.InfoLog != nil { - h.InfoLog.Println(msg) + h.InfoLog.Info(msg) } } diff --git a/exec_test.go b/exec_test.go index 9ba2416..cb08ce2 100644 --- a/exec_test.go +++ b/exec_test.go @@ -2,7 +2,7 @@ package hookah import ( "bytes" - "log" + "log/slog" "strings" "testing" "time" @@ -47,7 +47,7 @@ func TestOnlyExecutableBinsFound(t *testing.T) { return } - log.Printf("%#v", scripts) + slog.Debug("scripts", "scripts", scripts) assert.EqualValues(t, expectedScripts, scripts) @@ -98,7 +98,7 @@ func TestActionDirectoriesWorkAsExpected(t *testing.T) { return } - log.Printf("%#v", scripts) + slog.Debug("scripts", "scripts", scripts) assert.EqualValues(t, expectedScripts, scripts) diff --git a/server.go b/server.go index 0e6c6cd..5d488b5 100644 --- a/server.go +++ b/server.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "io" - "log" + "log/slog" "net/http" "os" "path/filepath" @@ -20,19 +20,13 @@ import ( var ErrPathIsNotDir = errors.New("path is not a dir") var validGhEvent = regexp.MustCompile(`^[a-z\d_]{1,30}$`) -// Logger handles Printf -type Logger interface { - Printf(format string, v ...any) - Println(v ...any) -} - // HookServer implements net/http.Handler type HookServer struct { RootDir string Timeout time.Duration - ErrorLog Logger - InfoLog Logger + ErrorLog *slog.Logger + InfoLog *slog.Logger sync.Mutex } @@ -86,7 +80,7 @@ func ServerExecTimeout(timeout time.Duration) ServerOption { } // ServerErrorLog configures the HookServer error logger -func ServerErrorLog(log Logger) ServerOption { +func ServerErrorLog(log *slog.Logger) ServerOption { return func(h *HookServer) error { h.ErrorLog = log return nil @@ -94,7 +88,7 @@ func ServerErrorLog(log Logger) ServerOption { } // ServerInfoLog configures the HookServer info logger -func ServerInfoLog(log Logger) ServerOption { +func ServerInfoLog(log *slog.Logger) ServerOption { return func(h *HookServer) error { h.InfoLog = log return nil @@ -123,7 +117,7 @@ func (h *HookServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { b, err := io.ReadAll(r.Body) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) - log.Println(ghDelivery, err) + slog.Info("request error", "delivery", ghDelivery, "error", err) return } buff := bytes.NewReader(b) @@ -134,7 +128,7 @@ func (h *HookServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { err = decoder.Decode(basicHook) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) - log.Println(ghDelivery, err) + slog.Info("json decode error", "delivery", ghDelivery, "error", err) return } @@ -143,7 +137,7 @@ func (h *HookServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { if repo == "" || login == "" { msg := "Unexpected JSON HTTP Body" http.Error(w, msg, http.StatusBadRequest) - log.Println(ghDelivery, msg) + slog.Info("invalid request body", "delivery", ghDelivery, "message", msg) return } @@ -170,7 +164,12 @@ func (h *HookServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { "HOOKAH_SERVER_ROOT="+h.RootDir, ) if err != nil && h.ErrorLog != nil { - h.ErrorLog.Printf("%s - %s/%s:%s - '%s'", ghDelivery, login, repo, ghEvent, err) + h.ErrorLog.Error("hook execution error", + "delivery", ghDelivery, + "login", login, + "repo", repo, + "event", ghEvent, + "error", err) } }() }