Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2023-08-16 11:15:07 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2023-09-05 08:40:40 +0300
commitfea96d943f4948e7fe1777904668a6acc02ed375 (patch)
tree31e433df85c3462c9396df9f581a45c0de5347fb
parent9ca8b86102188f14e9eeb600127d0ecd15eaea8c (diff)
cli/praefect: Fix logging setup for client-side subcommands
Many of the Praefect subcommands do not set up the logger, but immediately request them by calling `log.Default()`. This has multiple consequences: - The log level is fixed at "info" level, which is potentially quite noisy. - The gRPC logger isn't replaced properly, which has the consequence that the logger is overly verbose. Furthermore, we do not pay attention to the "GRPC_GO_LOG_SEVERITY_LEVEL" environment variable as we should be doing. - The logger will print its output to standard output. This breaks the ability to consume the output of some commands whose output is intended to be machine-parsable. Fix these issues by calling `log.Configure()` instead of `log.Default()` such that the logging infrastructure is getting properly initialized. Note that we don't use the configuration to set up the log format and level. This is intentional, because changing the server's configuration should not influence how clients perform logs. In any case, the loggers are configured to output to standard error instead of standard output to fix (3). Furthermore, the verbosity of the logger is decreased to "error" level to address (1) and (2). We could either add client-specific logging configuration to Praefect's configuration or alternatively add switches to the commands to configure logging ad-hoc. For the time being though we just care about fixing the above issues, so this is left for a later point in time if we see that there is indeed demand for such flexibility. Changelog: fixed
-rw-r--r--internal/cli/praefect/subcmd_dataloss.go6
-rw-r--r--internal/cli/praefect/subcmd_list_untracked_repositories.go6
-rw-r--r--internal/cli/praefect/subcmd_remove_repository.go6
-rw-r--r--internal/cli/praefect/subcmd_track_repositories.go5
-rw-r--r--internal/cli/praefect/subcmd_track_repository.go6
5 files changed, 24 insertions, 5 deletions
diff --git a/internal/cli/praefect/subcmd_dataloss.go b/internal/cli/praefect/subcmd_dataloss.go
index 6b904d5bb..a97e8e2d6 100644
--- a/internal/cli/praefect/subcmd_dataloss.go
+++ b/internal/cli/praefect/subcmd_dataloss.go
@@ -3,6 +3,7 @@ package praefect
import (
"fmt"
"io"
+ "os"
"sort"
"strings"
@@ -62,7 +63,10 @@ func datalossAction(ctx *cli.Context) error {
return err
}
- logger := log.Default()
+ logger, err := log.Configure(os.Stderr, "text", "error")
+ if err != nil {
+ return fmt.Errorf("configuring logger: %w", err)
+ }
includePartiallyAvailable := ctx.Bool("partially-unavailable")
diff --git a/internal/cli/praefect/subcmd_list_untracked_repositories.go b/internal/cli/praefect/subcmd_list_untracked_repositories.go
index 6685f660d..0551061c1 100644
--- a/internal/cli/praefect/subcmd_list_untracked_repositories.go
+++ b/internal/cli/praefect/subcmd_list_untracked_repositories.go
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
+ "os"
"time"
"github.com/urfave/cli/v2"
@@ -68,7 +69,10 @@ func listUntrackedRepositoriesAction(appCtx *cli.Context) error {
return err
}
- logger := log.Default()
+ logger, err := log.Configure(os.Stderr, "text", "error")
+ if err != nil {
+ return fmt.Errorf("configuring logger: %w", err)
+ }
onlyIncludeOlderThan := appCtx.Duration("older-than")
delimiter := appCtx.String("delimiter")
diff --git a/internal/cli/praefect/subcmd_remove_repository.go b/internal/cli/praefect/subcmd_remove_repository.go
index e095a60b4..1efdd701b 100644
--- a/internal/cli/praefect/subcmd_remove_repository.go
+++ b/internal/cli/praefect/subcmd_remove_repository.go
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
+ "os"
"sync"
"time"
@@ -78,7 +79,10 @@ func removeRepositoryAction(appCtx *cli.Context) error {
return err
}
- logger := log.Default()
+ logger, err := log.Configure(os.Stderr, "text", "error")
+ if err != nil {
+ return fmt.Errorf("configuring logger: %w", err)
+ }
ctx := appCtx.Context
openDBCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
diff --git a/internal/cli/praefect/subcmd_track_repositories.go b/internal/cli/praefect/subcmd_track_repositories.go
index 863e8f375..4c942cc35 100644
--- a/internal/cli/praefect/subcmd_track_repositories.go
+++ b/internal/cli/praefect/subcmd_track_repositories.go
@@ -63,7 +63,10 @@ func trackRepositoriesAction(appCtx *cli.Context) error {
return err
}
- logger := log.Default()
+ logger, err := log.Configure(os.Stderr, "text", "error")
+ if err != nil {
+ return fmt.Errorf("configuring logger: %w", err)
+ }
db, clean, err := openDB(conf.DB, appCtx.App.ErrWriter)
if err != nil {
diff --git a/internal/cli/praefect/subcmd_track_repository.go b/internal/cli/praefect/subcmd_track_repository.go
index 54a0a47f0..576913f88 100644
--- a/internal/cli/praefect/subcmd_track_repository.go
+++ b/internal/cli/praefect/subcmd_track_repository.go
@@ -7,6 +7,7 @@ import (
"fmt"
"io"
"math/rand"
+ "os"
"time"
"github.com/sirupsen/logrus"
@@ -91,7 +92,10 @@ func trackRepositoryAction(appCtx *cli.Context) error {
return err
}
- logger := log.Default()
+ logger, err := log.Configure(os.Stderr, "text", "error")
+ if err != nil {
+ return fmt.Errorf("configuring logger: %w", err)
+ }
virtualStorage := appCtx.String(paramVirtualStorage)
relativePath := appCtx.String(paramRelativePath)