diff options
author | karthik nayak <knayak@gitlab.com> | 2023-11-13 12:54:05 +0300 |
---|---|---|
committer | GitLab <noreply@gitlab.com> | 2023-12-15 06:20:09 +0300 |
commit | 896a4e366570a23c30f5f54d8aeec2ee4b07a4f1 (patch) | |
tree | 571208cfb3e6e6da0ba98d78e067626c88bbfd3d | |
parent | f32cf6c05668202929326c6e14fd1ceb8a70dfb8 (diff) |
Merge branch 'qmnguyen0711/fix-loopy-logger-issue' into 'master'cherry-pick-abb22ce3
Fix chatty loopWriter logs when log level config is empty
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6513
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Eric Ju <eju@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
(cherry picked from commit abb22ce3654f045c0c8b6fec1ba8b7a09eaca5f5)
b6ee4711 Fix chatty loopWriter logs when log level config is empty
136473dd Set logging level config to "info" explicitly
-rw-r--r-- | internal/gitaly/config/config.go | 9 | ||||
-rw-r--r-- | internal/gitaly/config/config_test.go | 6 | ||||
-rw-r--r-- | internal/log/configure.go | 2 | ||||
-rw-r--r-- | internal/log/configure_test.go | 5 |
4 files changed, 21 insertions, 1 deletions
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index a938b7c0c..71b0fa399 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -525,6 +525,14 @@ func (scc StreamCacheConfig) Validate() error { AsError() } +func defaultLoggingConfig() Logging { + return Logging{ + Config: log.Config{ + Level: "info", + }, + } +} + func defaultPackObjectsCacheConfig() StreamCacheConfig { return StreamCacheConfig{ // The Pack-Objects cache is effective at deduplicating concurrent @@ -557,6 +565,7 @@ func defaultPackObjectsLimiting() PackObjectsLimiting { func Load(file io.Reader) (Cfg, error) { cfg := Cfg{ Prometheus: prometheus.DefaultConfig(), + Logging: defaultLoggingConfig(), PackObjectsCache: defaultPackObjectsCacheConfig(), PackObjectsLimiting: defaultPackObjectsLimiting(), } diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index beccede6f..a15382d31 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -20,6 +20,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/sentry" "gitlab.com/gitlab-org/gitaly/v16/internal/helper/duration" "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" + "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" ) @@ -45,6 +46,7 @@ func TestLoadEmptyConfig(t *testing.T) { require.NoError(t, err) expectedCfg := Cfg{ + Logging: defaultLoggingConfig(), Prometheus: prometheus.DefaultConfig(), PackObjectsCache: defaultPackObjectsCacheConfig(), PackObjectsLimiting: defaultPackObjectsLimiting(), @@ -139,6 +141,9 @@ sentry_dsn = "abc123"`) require.NoError(t, err) require.Equal(t, Logging{ + Config: log.Config{ + Level: "info", + }, Sentry: Sentry(sentry.Config{ Environment: "production", DSN: "abc123", @@ -186,6 +191,7 @@ func TestLoadConfigCommand(t *testing.T) { modifyDefaultConfig := func(modify func(cfg *Cfg)) Cfg { cfg := &Cfg{ + Logging: defaultLoggingConfig(), Prometheus: prometheus.DefaultConfig(), PackObjectsCache: defaultPackObjectsCacheConfig(), PackObjectsLimiting: defaultPackObjectsLimiting(), diff --git a/internal/log/configure.go b/internal/log/configure.go index 691ffa7b3..da3eda3aa 100644 --- a/internal/log/configure.go +++ b/internal/log/configure.go @@ -127,7 +127,7 @@ func mapGRPCLogLevel(level string) string { // grpc-go is too verbose at level 'info'. So when config.toml requests // level info, we tell grpc-go to log at 'warn' instead. - if level == "info" { + if level == "info" || level == "" { return "warning" } diff --git a/internal/log/configure_test.go b/internal/log/configure_test.go index db2f0fe2e..01853d39f 100644 --- a/internal/log/configure_test.go +++ b/internal/log/configure_test.go @@ -161,6 +161,11 @@ func TestMapGRPCLogLevel(t *testing.T) { expectedLevel: "warning", }, { + desc: "empty level gets mapped", + level: "", + expectedLevel: "warning", + }, + { desc: "environment overrides value", environmentLevel: "ERROR", level: "info", |