Skip to content

Setup logging after reading the config file #856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Setup logging after reading the config file #856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5dcd9c7
Setup logging after reading the config file
dveeden Sep 4, 2025
5d4a10a
update tests
dveeden Sep 4, 2025
3051600
cleanup
dveeden Sep 4, 2025
95d51e9
Update TestChecksum
dveeden Sep 4, 2025
5ade0f8
Merge remote-tracking branch 'upstream/main' into log_encoder_config
dveeden Sep 8, 2025
6dcca59
Log config changes
dveeden Sep 8, 2025
6bb4da9
Merge remote-tracking branch 'upstream/main' into log_encoder_config
dveeden Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/manager/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/BurntSushi/toml"
"github.com/pingcap/tiproxy/lib/config"
"github.com/pingcap/tiproxy/lib/util/errors"
"go.uber.org/zap"
)

func (e *ConfigManager) reloadConfigFile(file string) error {
Expand Down Expand Up @@ -68,7 +67,6 @@ func (e *ConfigManager) SetTOMLConfig(data []byte) (err error) {
if originalData == nil || !bytes.Equal(originalData, newData) {
e.sts.checksum = crc32.ChecksumIEEE(newData)
e.sts.data = newData
e.logger.Info("current config", zap.Any("cfg", e.sts.current))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's crucial to print the config every time it changes. You can print it when the loggerManager watches a config change instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed 6dcca59 to do this.

for _, list := range e.sts.listeners {
list <- base.Clone()
}
Expand Down
11 changes: 5 additions & 6 deletions pkg/manager/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -299,19 +298,19 @@ func TestFilePath(t *testing.T) {
}

func TestChecksum(t *testing.T) {
cfgmgr, text, _ := testConfigManager(t, "", "")
require.Equal(t, 1, strings.Count(text.String(), "current config"))
cfgmgr, _, _ := testConfigManager(t, "", "")

c1 := cfgmgr.GetConfigChecksum()
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "gg"`)))
require.Equal(t, 2, strings.Count(text.String(), "current config"))
// same config, shouldn't log it again
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "gg"`)))
require.Equal(t, 2, strings.Count(text.String(), "current config"))

c2 := cfgmgr.GetConfigChecksum()
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "vv"`)))

c3 := cfgmgr.GetConfigChecksum()
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr="gg"`)))
require.Equal(t, 4, strings.Count(text.String(), "current config"))

c4 := cfgmgr.GetConfigChecksum()
require.Equal(t, c2, c4)
require.NotEqual(t, c1, c2)
Expand Down
15 changes: 6 additions & 9 deletions pkg/manager/config/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ package config

import (
"context"
"fmt"
"os"
"sync"
"time"

"github.com/pingcap/tiproxy/lib/config"
"github.com/pingcap/tiproxy/lib/util/errors"
"github.com/pingcap/tiproxy/pkg/util/waitgroup"
"github.com/tidwall/btree"
"go.uber.org/zap"
)

const (
Expand All @@ -24,9 +25,7 @@ const (
checkFileInterval = 2 * time.Second
)

var (
ErrNoResults = errors.Errorf("has no results")
)
var ErrNoResults = errors.Errorf("has no results")

type KVValue struct {
Key string
Expand All @@ -36,7 +35,6 @@ type KVValue struct {
type ConfigManager struct {
wg waitgroup.WaitGroup
cancel context.CancelFunc
logger *zap.Logger
advertiseAddr string

kv *btree.BTreeG[KVValue]
Expand All @@ -58,11 +56,10 @@ func NewConfigManager() *ConfigManager {
}
}

func (e *ConfigManager) Init(ctx context.Context, logger *zap.Logger, configFile string, advertiseAddr string) error {
func (e *ConfigManager) Init(ctx context.Context, configFile string, advertiseAddr string) error {
var nctx context.Context
nctx, e.cancel = context.WithCancel(ctx)

e.logger = logger
e.advertiseAddr = advertiseAddr

// for namespace persistence
Expand All @@ -89,11 +86,11 @@ func (e *ConfigManager) Init(ctx context.Context, logger *zap.Logger, configFile
return
case <-ticker.C:
if err := e.reloadConfigFile(configFile); err != nil {
e.logger.Warn("failed to reload file", zap.String("file", configFile), zap.Error(err))
fmt.Fprintf(os.Stderr, "failed to reload file %s: %v", configFile, err)
}
}
}
}, nil, e.logger)
}, nil, nil)
} else {
if err := e.SetTOMLConfig(nil); err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions pkg/manager/config/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

func testConfigManager(t *testing.T, configFile string, advertiseAddr string) (*ConfigManager, fmt.Stringer, context.Context) {
logger, text := logger.CreateLoggerForTest(t)
_, text := logger.CreateLoggerForTest(t)

ctx, cancel := context.WithCancel(context.Background())
if ddl, ok := t.Deadline(); ok {
Expand All @@ -23,7 +23,7 @@ func testConfigManager(t *testing.T, configFile string, advertiseAddr string) (*

cfgmgr := NewConfigManager()
cfgmgr.checkFileInterval = 20 * time.Millisecond
require.NoError(t, cfgmgr.Init(ctx, logger, configFile, advertiseAddr))
require.NoError(t, cfgmgr.Init(ctx, configFile, advertiseAddr))

t.Cleanup(func() {
require.NoError(t, cfgmgr.Close())
Expand Down
1 change: 1 addition & 0 deletions pkg/manager/logger/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func (lm *LoggerManager) watchCfg(ctx context.Context, cfgch <-chan *config.Conf
zap.NamedError("cfg marshal error", merr),
)
}
lm.logger.Info("current config", zap.Any("cfg", acfg))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/api/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func createServer(t *testing.T) (*Server, doHTTPFunc) {
lg, _ := logger.CreateLoggerForTest(t)
ready := atomic.NewBool(true)
cfgmgr := mgrcfg.NewConfigManager()
require.NoError(t, cfgmgr.Init(context.Background(), lg, "", ""))
require.NoError(t, cfgmgr.Init(context.Background(), "", ""))
crtmgr := mgrcrt.NewCertManager()
require.NoError(t, crtmgr.Init(cfgmgr.GetConfig(), lg, cfgmgr.WatchConfig()))
nsMgr := newMockNamespaceManager()
Expand Down
19 changes: 10 additions & 9 deletions pkg/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,18 @@ func NewServer(ctx context.Context, sctx *sctx.Context) (srv *Server, err error)
handler := sctx.Handler
ready := atomic.NewBool(false)

// set up logger
var lg *zap.Logger
if srv.loggerManager, lg, err = logger.NewLoggerManager(nil); err != nil {
// setup config manager
if err = srv.configManager.Init(ctx, sctx.ConfigFile, sctx.AdvertiseAddr); err != nil {
return
}
srv.loggerManager.Init(srv.configManager.WatchConfig())
cfg := srv.configManager.GetConfig()

// setup config manager
if err = srv.configManager.Init(ctx, lg.Named("config"), sctx.ConfigFile, sctx.AdvertiseAddr); err != nil {
// set up logger
var lg *zap.Logger
if srv.loggerManager, lg, err = logger.NewLoggerManager(&cfg.Log); err != nil {
return
}
cfg := srv.configManager.GetConfig()
srv.loggerManager.Init(srv.configManager.WatchConfig())

// welcome messages must be printed after initialization of configmager, because
// logfile backended zaplogger is enabled after cfgmgr.Init(..).
Expand All @@ -90,7 +90,7 @@ func NewServer(ctx context.Context, sctx *sctx.Context) (srv *Server, err error)
// Make sure the TiProxy info is always printed.
level := lg.Level()
srv.loggerManager.SetLoggerLevel(zap.InfoLevel)
printInfo(lg)
printInfo(lg, cfg)
srv.loggerManager.SetLoggerLevel(level)

// setup metrics
Expand Down Expand Up @@ -219,7 +219,7 @@ func NewServer(ctx context.Context, sctx *sctx.Context) (srv *Server, err error)
return
}

func printInfo(lg *zap.Logger) {
func printInfo(lg *zap.Logger, cfg *config.Config) {
fields := []zap.Field{
zap.String("Release Version", versioninfo.TiProxyVersion),
zap.String("Git Commit Hash", versioninfo.TiProxyGitHash),
Expand All @@ -230,6 +230,7 @@ func printInfo(lg *zap.Logger) {
zap.String("Arch", runtime.GOARCH),
}
lg.Info("Welcome to TiProxy.", fields...)
lg.Info("current config", zap.Any("cfg", cfg))
}

func (s *Server) preClose() {
Expand Down