diff --git a/kv/memberlist/memberlist_client.go b/kv/memberlist/memberlist_client.go index d074347a4..5880bd394 100644 --- a/kv/memberlist/memberlist_client.go +++ b/kv/memberlist/memberlist_client.go @@ -193,7 +193,7 @@ func (cfg *KVConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefix string) { f.StringVar(&cfg.AdvertiseAddr, prefix+"memberlist.advertise-addr", mlDefaults.AdvertiseAddr, "Gossip address to advertise to other members in the cluster. Used for NAT traversal.") f.IntVar(&cfg.AdvertisePort, prefix+"memberlist.advertise-port", mlDefaults.AdvertisePort, "Gossip port to advertise to other members in the cluster. Used for NAT traversal.") - cfg.TCPTransport.RegisterFlags(f, prefix) + cfg.TCPTransport.RegisterFlagsWithPrefix(f, prefix) } func (cfg *KVConfig) RegisterFlags(f *flag.FlagSet) { diff --git a/kv/memberlist/tcp_transport.go b/kv/memberlist/tcp_transport.go index afbd1b201..daccd30ec 100644 --- a/kv/memberlist/tcp_transport.go +++ b/kv/memberlist/tcp_transport.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net" + "strings" "sync" "time" @@ -60,8 +61,12 @@ type TCPTransportConfig struct { TLS dstls.ClientConfig `yaml:",inline"` } -// RegisterFlags registers flags. -func (cfg *TCPTransportConfig) RegisterFlags(f *flag.FlagSet, prefix string) { +func (cfg *TCPTransportConfig) RegisterFlags(f *flag.FlagSet) { + cfg.RegisterFlagsWithPrefix(f, "") +} + +// RegisterFlagsWithPrefix registers flags with prefix. +func (cfg *TCPTransportConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefix string) { // "Defaults to hostname" -- memberlist sets it to hostname by default. f.Var(&cfg.BindAddrs, prefix+"memberlist.bind-addr", "IP address to listen on for gossip messages. Multiple addresses may be specified. Defaults to 0.0.0.0") f.IntVar(&cfg.BindPort, prefix+"memberlist.bind-port", 7946, "Port to listen on for gossip messages.") @@ -414,7 +419,13 @@ func (t *TCPTransport) WriteTo(b []byte, addr string) (time.Time, error) { if err != nil { t.sentPacketsErrors.Inc() - level.Warn(t.logger).Log("msg", "WriteTo failed", "addr", addr, "err", err) + logLevel := level.Warn(t.logger) + if strings.Contains(err.Error(), "connection refused") { + // The connection refused is a common error that could happen during normal operations when a node + // shutdown (or crash). It shouldn't be considered a warning condition on the sender side. + logLevel = t.debugLog() + } + logLevel.Log("msg", "WriteTo failed", "addr", addr, "err", err) // WriteTo is used to send "UDP" packets. Since we use TCP, we can detect more errors, // but memberlist library doesn't seem to cope with that very well. That is why we return nil instead. diff --git a/kv/memberlist/tcp_transport_test.go b/kv/memberlist/tcp_transport_test.go new file mode 100644 index 000000000..13683dd88 --- /dev/null +++ b/kv/memberlist/tcp_transport_test.go @@ -0,0 +1,61 @@ +package memberlist + +import ( + "testing" + + "github.com/go-kit/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/dskit/concurrency" + "github.com/grafana/dskit/flagext" +) + +func TestTCPTransport_WriteTo_ShouldNotLogAsWarningExpectedFailures(t *testing.T) { + tests := map[string]struct { + setup func(t *testing.T, cfg *TCPTransportConfig) + remoteAddr string + expectedLogs string + unexpectedLogs string + }{ + "should not log 'connection refused' by default": { + remoteAddr: "localhost:12345", + unexpectedLogs: "connection refused", + }, + "should log 'connection refused' if debug log level is enabled": { + setup: func(t *testing.T, cfg *TCPTransportConfig) { + cfg.TransportDebug = true + }, + remoteAddr: "localhost:12345", + expectedLogs: "connection refused", + }, + } + + for testName, testData := range tests { + t.Run(testName, func(t *testing.T) { + logs := &concurrency.SyncBuffer{} + logger := log.NewLogfmtLogger(logs) + + cfg := TCPTransportConfig{} + flagext.DefaultValues(&cfg) + cfg.BindAddrs = []string{"localhost"} + cfg.BindPort = 0 + if testData.setup != nil { + testData.setup(t, &cfg) + } + + transport, err := NewTCPTransport(cfg, logger) + require.NoError(t, err) + + _, err = transport.WriteTo([]byte("test"), testData.remoteAddr) + require.NoError(t, err) + + if testData.expectedLogs != "" { + assert.Contains(t, logs.String(), testData.expectedLogs) + } + if testData.unexpectedLogs != "" { + assert.NotContains(t, logs.String(), testData.unexpectedLogs) + } + }) + } +}