Merge branch 'qmnguyen0711/fix-loopy-logger-issue' into 'master'

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>

4 files changed, 21 insertions, 1 deletions

diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index f21779c4f..edf978996 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -607,6 +607,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
@@ -639,6 +647,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 7ce1bd1b1..5e7d45689 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"
 )
 
@@ -55,6 +56,7 @@ func TestLoadEmptyConfig(t *testing.T) {
 	require.NoError(t, err)
 
 	expectedCfg := Cfg{
+		Logging:             defaultLoggingConfig(),
 		Prometheus:          prometheus.DefaultConfig(),
 		PackObjectsCache:    defaultPackObjectsCacheConfig(),
 		PackObjectsLimiting: defaultPackObjectsLimiting(),
@@ -212,6 +214,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",
@@ -259,6 +264,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",