From 1e51a9b315ec8d3816f9e2b66375bcbc84c21ce6 Mon Sep 17 00:00:00 2001 From: kaidaguerre Date: Wed, 28 Feb 2024 05:01:49 -0600 Subject: [PATCH] Send logs to stderr. Fix Server output --- .github/workflows/test-acceptance.yml | 7 ---- go.mod | 4 ++- go.sum | 4 +++ internal/cmd/server.go | 7 ++-- internal/cmdconfig/cmd_hooks.go | 2 +- internal/cmdconfig/mappings.go | 28 +++++++++------ internal/dashboardserver/api.go | 2 +- internal/dashboardserver/output.go | 37 ++++++-------------- internal/dashboardserver/server.go | 6 ++-- internal/display/display.go | 6 ++-- internal/logger/logger.go | 25 ++++++------- internal/service/api/api.go | 6 ++-- tests/acceptance/test_files/mod_install.bats | 4 +-- 13 files changed, 63 insertions(+), 75 deletions(-) diff --git a/.github/workflows/test-acceptance.yml b/.github/workflows/test-acceptance.yml index 995af1fa..965a1e99 100644 --- a/.github/workflows/test-acceptance.yml +++ b/.github/workflows/test-acceptance.yml @@ -155,13 +155,6 @@ jobs: echo "exit_code=$(echo $?)" >> $GITHUB_OUTPUT echo ">> here" - - name: Save Test Suite Logs - uses: actions/upload-artifact@v3 - with: - name: test-logs-${{ matrix.test_block }}-${{ matrix.platform }} - path: ~/.powerpipe/logs - if-no-files-found: error - # This job checks whether the test suite has passed or not. # Since the exit_code is set only when the bats test suite pass, # we have added the if-conditional block diff --git a/go.mod b/go.mod index e96a829e..d37299f4 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/go-playground/validator/v10 v10.16.0 github.com/hashicorp/go-hclog v1.6.1 // indirect github.com/hashicorp/go-version v1.6.0 // indirect - github.com/hashicorp/hcl/v2 v2.19.1 + github.com/hashicorp/hcl/v2 v2.19.1 // indirect github.com/jackc/pgconn v1.14.1 // indirect github.com/karrick/gows v0.3.0 github.com/mattn/go-isatty v0.0.20 @@ -43,6 +43,7 @@ require ( require github.com/sethvargo/go-retry v0.2.4 // indirect require ( + github.com/FabienMht/ginslog v0.0.1 github.com/Masterminds/sprig/v3 v3.2.3 github.com/didip/tollbooth/v7 v7.0.1 github.com/gin-contrib/gzip v0.0.6 @@ -53,6 +54,7 @@ require ( github.com/logrusorgru/aurora v2.0.3+incompatible github.com/marcboeker/go-duckdb v1.5.6 github.com/mattn/go-sqlite3 v1.14.19 + github.com/samber/slog-gin v1.10.2 golang.org/x/sync v0.5.0 golang.org/x/text v0.14.0 ) diff --git a/go.sum b/go.sum index 467182dc..d5709f6d 100644 --- a/go.sum +++ b/go.sum @@ -191,6 +191,8 @@ github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 h1:bvDV9 github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= +github.com/FabienMht/ginslog v0.0.1 h1:d0FJQVnAV3whz4yNPcvXTDAaCW6XtYhi9vpIUaIuVnc= +github.com/FabienMht/ginslog v0.0.1/go.mod h1:F2vaTtDfWTCIpHEDuu9OqEDZtaMF4XKkNn82J9pWQV8= github.com/Machiel/slugify v1.0.1 h1:EfWSlRWstMadsgzmiV7d0yVd2IFlagWH68Q+DcYCm4E= github.com/Machiel/slugify v1.0.1/go.mod h1:fTFGn5uWEynW4CUMG7sWkYXOf1UgDxyTM3DbR6Qfg3k= github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= @@ -785,6 +787,8 @@ github.com/sagikazarmark/locafero v0.4.0 h1:HApY1R9zGo4DBgr7dqsTH/JJxLTTsOt7u6ke github.com/sagikazarmark/locafero v0.4.0/go.mod h1:Pe1W6UlPYUk/+wc/6KFhbORCfqzgYEpgQ3O5fPuL3H4= github.com/sagikazarmark/slog-shim v0.1.0 h1:diDBnUNK9N/354PgrxMywXnAwEr1QZcOr6gto+ugjYE= github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWRIczQRv+GVI1AkeQ= +github.com/samber/slog-gin v1.10.2 h1:ukSOVJzQEQIyigzioXHbKMGcAGmVWwirOc5Rba2ctQU= +github.com/samber/slog-gin v1.10.2/go.mod h1:rOS5GQQd/Dq4tTczgvdnqfATXk0ReEoVu5mpdMGMBrY= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= diff --git a/internal/cmd/server.go b/internal/cmd/server.go index 29b81bc5..faede496 100644 --- a/internal/cmd/server.go +++ b/internal/cmd/server.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "fmt" "os" "os/signal" @@ -34,7 +35,6 @@ func serverCmd() *cobra.Command { Powerpipe server runs in the foreground; Press Ctrl-C to exit.`, } - // TODO KAI CHECK ARGS (https://github.com/turbot/powerpipe/issues/106) cmdconfig. OnCmd(cmd). AddModLocationFlag(). @@ -102,6 +102,9 @@ func runServerCmd(cmd *cobra.Command, _ []string) { error_helpers.FailOnError(err) } - dashboardserver.OutputMessage(ctx, "server started") + dashboardserver.OutputReady(ctx, fmt.Sprintf("Dashboard server started on %d and listening on %s", serverPort, viper.GetString(constants.ArgListen))) + dashboardserver.OutputMessage(ctx, fmt.Sprintf("Visit http://localhost:%d", serverPort)) + dashboardserver.OutputMessage(ctx, "Press Ctrl+C to exit") + <-ctx.Done() } diff --git a/internal/cmdconfig/cmd_hooks.go b/internal/cmdconfig/cmd_hooks.go index 491810cc..c6e5c569 100644 --- a/internal/cmdconfig/cmd_hooks.go +++ b/internal/cmdconfig/cmd_hooks.go @@ -125,7 +125,7 @@ func initGlobalConfig() error_helpers.ErrorAndWarnings { // set-up viper with defaults from the env and default workspace profile cmdconfig.BootstrapViper(loader, cmd, - cmdconfig.WithConfigDefaults(configDefaults()), + cmdconfig.WithConfigDefaults(configDefaults(cmd)), cmdconfig.WithDirectoryEnvMappings(dirEnvMappings())) if err != nil { diff --git a/internal/cmdconfig/mappings.go b/internal/cmdconfig/mappings.go index a789eeb2..4ba4ae87 100644 --- a/internal/cmdconfig/mappings.go +++ b/internal/cmdconfig/mappings.go @@ -1,33 +1,42 @@ package cmdconfig import ( + "github.com/spf13/cobra" "github.com/turbot/pipe-fittings/app_specific" "github.com/turbot/pipe-fittings/cmdconfig" "github.com/turbot/pipe-fittings/constants" "github.com/turbot/pipe-fittings/filepaths" localconstants "github.com/turbot/powerpipe/internal/constants" + "golang.org/x/exp/maps" ) -func configDefaults() map[string]any { - return map[string]any{ +func configDefaults(cmd *cobra.Command) map[string]any { + defs := map[string]any{ // global general options constants.ArgTelemetry: constants.TelemetryInfo, constants.ArgUpdateCheck: true, constants.ArgPipesInstallDir: filepaths.DefaultPipesInstallDir, - // from global database options - // TODO KAI NEEDED??? - //constants.ArgDatabasePort: constants.DatabaseDefaultPort, - //constants.ArgDatabaseStartTimeout: constants.DBStartTimeout.Seconds(), - //constants.ArgServiceCacheEnabled: true, - //constants.ArgCacheMaxTtl: 300, - // dashboard constants.ArgDashboardStartTimeout: constants.DashboardServiceStartTimeout.Seconds(), // memory constants.ArgMemoryMaxMb: 1024, } + cmdSpecificDefaults, ok := cmdSpecificDefaults()[cmd.Name()] + if ok { + maps.Copy(defs, cmdSpecificDefaults) + } + return defs +} + +// command specific config defaults (keyed by comand name) +func cmdSpecificDefaults() map[string]map[string]any { + return map[string]map[string]any{ + "server": { + constants.ArgEnvironment: "release", + }, + } } // environment variable mappings for directory paths which must be set as part of the viper bootstrap process @@ -46,7 +55,6 @@ func envMappings() map[string]cmdconfig.EnvMapping { return map[string]cmdconfig.EnvMapping{ app_specific.EnvInstallDir: {ConfigVar: []string{constants.ArgInstallDir}, VarType: cmdconfig.EnvVarTypeString}, app_specific.EnvModLocation: {ConfigVar: []string{constants.ArgModLocation}, VarType: cmdconfig.EnvVarTypeString}, - app_specific.EnvIntrospection: {ConfigVar: []string{constants.ArgIntrospection}, VarType: cmdconfig.EnvVarTypeString}, app_specific.EnvTelemetry: {ConfigVar: []string{constants.ArgTelemetry}, VarType: cmdconfig.EnvVarTypeString}, app_specific.EnvUpdateCheck: {ConfigVar: []string{constants.ArgUpdateCheck}, VarType: cmdconfig.EnvVarTypeBool}, app_specific.EnvSnapshotLocation: {ConfigVar: []string{constants.ArgSnapshotLocation}, VarType: cmdconfig.EnvVarTypeString}, diff --git a/internal/dashboardserver/api.go b/internal/dashboardserver/api.go index d58e421b..fe2180e1 100644 --- a/internal/dashboardserver/api.go +++ b/internal/dashboardserver/api.go @@ -61,7 +61,7 @@ func startAPIAsync(ctx context.Context, webSocket *melody.Melody) chan struct{} } }() - outputReady(ctx, fmt.Sprintf("Dashboard server started on %d and listening on %s", dashboardServerPort, viper.GetString(constants.ArgListen))) + OutputReady(ctx, fmt.Sprintf("Dashboard server started on %d and listening on %s", dashboardServerPort, viper.GetString(constants.ArgListen))) OutputMessage(ctx, fmt.Sprintf("Visit http://localhost:%d", dashboardServerPort)) OutputMessage(ctx, "Press Ctrl+C to exit") <-ctx.Done() diff --git a/internal/dashboardserver/output.go b/internal/dashboardserver/output.go index ae169c13..610c24ed 100644 --- a/internal/dashboardserver/output.go +++ b/internal/dashboardserver/output.go @@ -3,68 +3,53 @@ package dashboardserver import ( "context" "fmt" - "io" - "os" - "path/filepath" - "time" - "github.com/fatih/color" "github.com/mattn/go-isatty" - "github.com/turbot/pipe-fittings/filepaths" + "log/slog" + "os" ) -var logSink io.Writer - const ( errorPrefix = "[ Error ]" + warningPrefix = "[ Warning ]" messagePrefix = "[ Message ]" readyPrefix = "[ Ready ]" waitPrefix = "[ Wait ]" ) -func initLogSink() { - logName := fmt.Sprintf("dashboard-%s.log", time.Now().Format("2006-01-02")) - logPath := filepath.Join(filepaths.EnsureLogDir(), logName) - f, err := os.OpenFile(logPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) - if err != nil { - fmt.Printf("failed to open dashboard manager log file: %s\n", err.Error()) //nolint:forbidigo // TODO: better way to log error? - os.Exit(3) - } - logSink = f -} - func output(_ context.Context, prefix string, msg interface{}) { - if logSink == nil { - logSink = os.Stdout - } - _, _ = fmt.Fprintf(logSink, "%s %v\n", prefix, msg) + _, _ = fmt.Fprintf(os.Stdout, "%s %v\n", prefix, msg) } func OutputMessage(ctx context.Context, msg string) { output(ctx, applyColor(messagePrefix, color.HiGreenString), msg) + slog.Info(msg) } func OutputWarning(ctx context.Context, msg string) { output(ctx, applyColor(messagePrefix, color.RedString), msg) + slog.Warn(msg) } func OutputError(ctx context.Context, err error) { output(ctx, applyColor(errorPrefix, color.RedString), err) + slog.Error("Error", "error", err) } -func outputReady(ctx context.Context, msg string) { +func OutputReady(ctx context.Context, msg string) { output(ctx, applyColor(readyPrefix, color.GreenString), msg) + slog.Info(msg) } func OutputWait(ctx context.Context, msg string) { output(ctx, applyColor(waitPrefix, color.CyanString), msg) + slog.Info(msg) } func applyColor(str string, color func(format string, a ...interface{}) string) string { - // TODO check streampipe logic is service mode if !isatty.IsTerminal(os.Stdout.Fd()) { return str } else { - return color((str)) + return color(str) } } diff --git a/internal/dashboardserver/server.go b/internal/dashboardserver/server.go index 443a9678..d627bfae 100644 --- a/internal/dashboardserver/server.go +++ b/internal/dashboardserver/server.go @@ -30,8 +30,6 @@ type Server struct { } func NewServer(ctx context.Context, w *dashboardworkspace.WorkspaceEvents, webSocket *melody.Melody) (*Server, error) { - initLogSink() - OutputWait(ctx, "Starting WorkspaceEvents Server") var dashboardClients = make(map[string]*DashboardClientInfo) @@ -125,7 +123,7 @@ func (s *Server) HandleDashboardEvent(ctx context.Context, event dashboardevents } dashboardName := e.Root.GetName() s.writePayloadToSession(e.Session, payload) - outputReady(ctx, fmt.Sprintf("Execution complete: %s", dashboardName)) + OutputReady(ctx, fmt.Sprintf("Execution complete: %s", dashboardName)) case *dashboardevents.ControlComplete: slog.Debug("ControlComplete event", "session", e.Session, "control", e.Control.GetControlId()) @@ -394,7 +392,7 @@ func (s *Server) handleMessageFunc(ctx context.Context) func(session *melody.Ses error_helpers.FailOnError(err) s.writePayloadToSession(sessionId, payload) - outputReady(ctx, fmt.Sprintf("Show snapshot complete: %s", snapshotName)) + OutputReady(ctx, fmt.Sprintf("Show snapshot complete: %s", snapshotName)) case "input_changed": s.setDashboardInputsForSession(sessionId, request.Payload.InputValues) _ = dashboardexecute.Executor.OnInputChanged(ctx, sessionId, request.Payload.InputValues, request.Payload.ChangedInput) diff --git a/internal/display/display.go b/internal/display/display.go index 516d71ab..57518fb9 100644 --- a/internal/display/display.go +++ b/internal/display/display.go @@ -7,9 +7,6 @@ import ( "encoding/csv" "encoding/json" "fmt" - "github.com/turbot/pipe-fittings/cmdconfig" - "github.com/turbot/pipe-fittings/error_helpers" - "github.com/turbot/powerpipe/internal/queryresult" "os" "strings" "time" @@ -21,7 +18,10 @@ import ( "github.com/karrick/gows" "github.com/spf13/viper" "github.com/turbot/go-kit/helpers" + "github.com/turbot/pipe-fittings/cmdconfig" "github.com/turbot/pipe-fittings/constants" + "github.com/turbot/pipe-fittings/error_helpers" + "github.com/turbot/powerpipe/internal/queryresult" ) // ShowQueryOutput displays the output using the proper formatter as applicable diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 213c446f..6810f199 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -2,6 +2,7 @@ package logger import ( "fmt" + "github.com/turbot/pipe-fittings/sanitize" "io" "log/slog" "os" @@ -9,11 +10,9 @@ import ( "time" "github.com/spf13/viper" - "github.com/turbot/go-kit/logging" "github.com/turbot/pipe-fittings/app_specific" "github.com/turbot/pipe-fittings/constants" "github.com/turbot/pipe-fittings/constants/runtime" - "github.com/turbot/pipe-fittings/filepaths" ) func Initialize() { @@ -40,19 +39,17 @@ func PowerpipeLogger() *slog.Logger { handlerOptions := &slog.HandlerOptions{ Level: level, - // TODO KAI SANITIZE - //ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr { - // sanitized := sanitize.Instance.SanitizeKeyValue(a.Key, a.Value.Any()) - // - // return slog.Attr{ - // Key: a.Key, - // Value: slog.AnyValue(sanitized), - // } - //}, + ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr { + sanitized := sanitize.Instance.SanitizeKeyValue(a.Key, a.Value.Any()) + + return slog.Attr{ + Key: a.Key, + Value: slog.AnyValue(sanitized), + } + }, } - logDestination := logging.NewRotatingLogWriter(filepaths.EnsureLogDir(), app_specific.AppName) - return slog.New(slog.NewJSONHandler(logDestination, handlerOptions)) + return slog.New(slog.NewJSONHandler(os.Stderr, handlerOptions)) } func getLogLevel() slog.Leveler { @@ -72,6 +69,6 @@ func getLogLevel() slog.Leveler { case "off": return constants.LogLevelOff default: - return slog.LevelInfo + return constants.LogLevelOff } } diff --git a/internal/service/api/api.go b/internal/service/api/api.go index c6d115e2..1842b808 100644 --- a/internal/service/api/api.go +++ b/internal/service/api/api.go @@ -19,6 +19,7 @@ import ( "github.com/gin-gonic/gin/binding" "github.com/go-playground/validator/v10" "github.com/spf13/viper" + "github.com/turbot/pipe-fittings/constants" "github.com/turbot/pipe-fittings/filepaths" "github.com/turbot/pipe-fittings/workspace" "github.com/turbot/powerpipe/internal/dashboardserver" @@ -118,11 +119,8 @@ func NewAPIService(ctx context.Context, opts ...APIServiceOption) (*APIService, // Start starts services managed by the Manager. func (api *APIService) Start() error { - // if api.mod == nil { - // return sperr.New("cannot start without a defined mod") - // } // Set the gin mode based on our environment, to configure logging etc as appropriate - gin.SetMode(viper.GetString("environment")) + gin.SetMode(viper.GetString(constants.ArgEnvironment)) binding.EnableDecoderDisallowUnknownFields = true // Initialize gin diff --git a/tests/acceptance/test_files/mod_install.bats b/tests/acceptance/test_files/mod_install.bats index f5b88fa0..71fb858d 100644 --- a/tests/acceptance/test_files/mod_install.bats +++ b/tests/acceptance/test_files/mod_install.bats @@ -48,13 +48,13 @@ local } @test "install a mod with protocol in url" { - run powerpipe mod install https://github.com/turbot/steampipe-mod-hackernews-insights@0.3.0 --force + run powerpipe mod install https://github.com/turbot/steampipe-mod-hackernews-insights@0.4.0 --force # should install with the protocol in the url prefix assert_output ' Installed 1 mod: local -└── github.com/turbot/steampipe-mod-hackernews-insights@v0.3.0' +└── github.com/turbot/steampipe-mod-hackernews-insights@v0.4.0' } # Installed 4 mods: