From 070783b28b5b2866a477a60aae6172c0f316ebaa Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 23 Mar 2016 15:41:53 +0100 Subject: [PATCH] Remove all global log invocations --- connection.go | 10 ++++++---- connection_maker.go | 10 ++++++---- examples/increment-only-counter/main.go | 26 ++++++++++++------------- gossip_channel.go | 8 +++++--- gossip_test.go | 6 ++++-- local_peer.go | 14 ++++++------- metcd/metcdsrv/main.go | 2 +- metcd/server.go | 2 +- mocks_test.go | 6 +++--- peer.go | 3 +-- peers.go | 5 ++--- router.go | 26 +++++++++++++------------ 12 files changed, 62 insertions(+), 56 deletions(-) diff --git a/connection.go b/connection.go index 7677253..d574ece 100644 --- a/connection.go +++ b/connection.go @@ -76,13 +76,14 @@ type LocalConnection struct { errorChan chan<- error finished <-chan struct{} // closed to signal that actorLoop has finished senders *gossipSenders + logger *log.Logger } // If the connection is successful, it will end up in the local peer's // connections map. -func startLocalConnection(connRemote *remoteConnection, tcpConn *net.TCPConn, router *Router, acceptNewPeer bool) { +func startLocalConnection(connRemote *remoteConnection, tcpConn *net.TCPConn, router *Router, acceptNewPeer bool, logger *log.Logger) { if connRemote.local != router.Ourself.Peer { - log.Fatal("Attempt to create local connection from a peer which is not ourself") + panic("attempt to create local connection from a peer which is not ourself") } actionChan := make(chan connectionAction, ChannelSize) errorChan := make(chan error, 1) @@ -96,6 +97,7 @@ func startLocalConnection(connRemote *remoteConnection, tcpConn *net.TCPConn, ro actionChan: actionChan, errorChan: errorChan, finished: finished, + logger: logger, } conn.senders = newGossipSenders(conn, finished) go conn.run(actionChan, errorChan, finished, acceptNewPeer) @@ -377,14 +379,14 @@ func (conn *LocalConnection) actorLoop(actionChan <-chan connectionAction, error func (conn *LocalConnection) teardown(err error) { if conn.remote == nil { - log.Printf("->[%s] connection shutting down due to error during handshake: %v", conn.remoteTCPAddr, err) + conn.logger.Printf("->[%s] connection shutting down due to error during handshake: %v", conn.remoteTCPAddr, err) } else { conn.log("connection shutting down due to error:", err) } if conn.tcpConn != nil { if err := conn.tcpConn.Close(); err != nil { - log.Printf("warning: %v", err) + conn.logger.Printf("warning: %v", err) } } diff --git a/connection_maker.go b/connection_maker.go index bece670..5299137 100644 --- a/connection_maker.go +++ b/connection_maker.go @@ -28,6 +28,7 @@ type connectionMaker struct { connections map[Connection]struct{} directPeers peerAddrs actionChan chan<- connectionMakerAction + logger *log.Logger } // TargetState describes the connection state of a remote target. @@ -57,7 +58,7 @@ type connectionMakerAction func() bool // peers, making outbound connections from localAddr, and listening on // port. If discovery is true, ConnectionMaker will attempt to // initiate new connections with peers it's not directly connected to. -func newConnectionMaker(ourself *localPeer, peers *Peers, localAddr string, port int, discovery bool) *connectionMaker { +func newConnectionMaker(ourself *localPeer, peers *Peers, localAddr string, port int, discovery bool, logger *log.Logger) *connectionMaker { actionChan := make(chan connectionMakerAction, ChannelSize) cm := &connectionMaker{ ourself: ourself, @@ -69,6 +70,7 @@ func newConnectionMaker(ourself *localPeer, peers *Peers, localAddr string, port targets: make(map[string]*target), connections: make(map[Connection]struct{}), actionChan: actionChan, + logger: logger, } go cm.queryLoop(actionChan) return cm @@ -365,9 +367,9 @@ func (cm *connectionMaker) connectToTargets(validTarget map[string]struct{}, dir } func (cm *connectionMaker) attemptConnection(address string, acceptNewPeer bool) { - log.Printf("->[%s] attempting connection", address) - if err := cm.ourself.createConnection(cm.localAddr, address, acceptNewPeer); err != nil { - log.Printf("->[%s] error during connection attempt: %v", address, err) + cm.logger.Printf("->[%s] attempting connection", address) + if err := cm.ourself.createConnection(cm.localAddr, address, acceptNewPeer, cm.logger); err != nil { + cm.logger.Printf("->[%s] error during connection attempt: %v", address, err) cm.connectionAborted(address, err) } } diff --git a/examples/increment-only-counter/main.go b/examples/increment-only-counter/main.go index 26b670b..127e0a6 100644 --- a/examples/increment-only-counter/main.go +++ b/examples/increment-only-counter/main.go @@ -3,6 +3,7 @@ package main import ( "flag" "fmt" + "io/ioutil" "log" "net" "net/http" @@ -29,20 +30,20 @@ func main() { flag.Var(peers, "peer", "initial peer (may be repeated)") flag.Parse() - log.SetPrefix(*nickname + "> ") + logger := log.New(os.Stderr, *nickname+"> ", log.LstdFlags) host, portStr, err := net.SplitHostPort(*meshListen) if err != nil { - log.Fatalf("mesh address: %s: %v", *meshListen, err) + logger.Fatalf("mesh address: %s: %v", *meshListen, err) } port, err := strconv.Atoi(portStr) if err != nil { - log.Fatalf("mesh address: %s: %v", *meshListen, err) + logger.Fatalf("mesh address: %s: %v", *meshListen, err) } name, err := mesh.PeerNameFromString(*hwaddr) if err != nil { - log.Fatalf("%s: %v", *hwaddr, err) + logger.Fatalf("%s: %v", *hwaddr, err) } router := mesh.NewRouter(mesh.Config{ @@ -53,38 +54,35 @@ func main() { ConnLimit: 64, PeerDiscovery: true, TrustedSubnets: []*net.IPNet{}, - }, name, *nickname, mesh.NullOverlay{}) + }, name, *nickname, mesh.NullOverlay{}, log.New(ioutil.Discard, "", 0)) - peer := newPeer(name, log.New(os.Stderr, *nickname+"> ", log.LstdFlags)) + peer := newPeer(name, logger) gossip := router.NewGossip(*channel, peer) peer.register(gossip) func() { - log.Printf("mesh router starting (%s)", *meshListen) + logger.Printf("mesh router starting (%s)", *meshListen) router.Start() }() defer func() { - log.Printf("mesh router stopping") + logger.Printf("mesh router stopping") router.Stop() }() router.ConnectionMaker.InitiateConnections(peers.slice(), true) - errs := make(chan error, 2) - + errs := make(chan error) go func() { c := make(chan os.Signal) signal.Notify(c, syscall.SIGINT) errs <- fmt.Errorf("%s", <-c) }() - go func() { - log.Printf("HTTP server starting (%s)", *httpListen) + logger.Printf("HTTP server starting (%s)", *httpListen) http.HandleFunc("/", handle(peer)) errs <- http.ListenAndServe(*httpListen, nil) }() - - log.Print(<-errs) + logger.Print(<-errs) } type counter interface { diff --git a/gossip_channel.go b/gossip_channel.go index 3b585e8..c7bd36a 100644 --- a/gossip_channel.go +++ b/gossip_channel.go @@ -13,16 +13,18 @@ type gossipChannel struct { ourself *localPeer routes *routes gossiper Gossiper + logger *log.Logger } // newGossipChannel returns a named, usable channel. // It delegates receiving duties to the passed Gossiper. -func newGossipChannel(channelName string, ourself *localPeer, r *routes, g Gossiper) *gossipChannel { +func newGossipChannel(channelName string, ourself *localPeer, r *routes, g Gossiper, logger *log.Logger) *gossipChannel { return &gossipChannel{ name: channelName, ourself: ourself, routes: r, gossiper: g, + logger: logger, } } @@ -134,7 +136,7 @@ func (c *gossipChannel) makeBroadcastMsg(srcName PeerName, msg []byte) protocolM } func (c *gossipChannel) log(args ...interface{}) { - log.Println(append(append([]interface{}{}, "[gossip "+c.name+"]:"), args...)...) + c.logger.Println(append(append([]interface{}{}, "[gossip "+c.name+"]:"), args...)...) } // GobEncode gob-encodes each item and returns the resulting byte slice. @@ -143,7 +145,7 @@ func gobEncode(items ...interface{}) []byte { enc := gob.NewEncoder(buf) for _, i := range items { if err := enc.Encode(i); err != nil { - log.Fatal(err) + panic(err) } } return buf.Bytes() diff --git a/gossip_test.go b/gossip_test.go index df5a346..fd34edc 100644 --- a/gossip_test.go +++ b/gossip_test.go @@ -2,6 +2,8 @@ package mesh import ( "fmt" + "io/ioutil" + "log" "sync" "testing" @@ -22,7 +24,7 @@ var _ gossipConnection = &mockGossipConnection{} func newTestRouter(name string) *Router { peerName, _ := PeerNameFromString(name) - router := NewRouter(Config{}, peerName, "nick", nil) + router := NewRouter(Config{}, peerName, "nick", nil, log.New(ioutil.Discard, "", 0)) router.Start() return router } @@ -74,7 +76,7 @@ func (router *Router) newTestGossipConnection(r *Router) *mockGossipConnection { toPeer = router.Peers.fetchWithDefault(toPeer) // Has side-effect of incrementing refcount conn := &mockGossipConnection{ - remoteConnection: remoteConnection{router.Ourself.Peer, toPeer, "", false, true}, + remoteConnection: *newRemoteConnection(router.Ourself.Peer, toPeer, "", false, true), dest: r, start: make(chan struct{}), } diff --git a/local_peer.go b/local_peer.go index 5e1c224..9a393f4 100644 --- a/local_peer.go +++ b/local_peer.go @@ -79,7 +79,7 @@ func (peer *localPeer) ConnectionsTo(names []PeerName) []Connection { // createConnection creates a new connection, originating from // localAddr, to peerAddr. If acceptNewPeer is false, peerAddr must // already be a member of the mesh. -func (peer *localPeer) createConnection(localAddr string, peerAddr string, acceptNewPeer bool) error { +func (peer *localPeer) createConnection(localAddr string, peerAddr string, acceptNewPeer bool, logger *log.Logger) error { if err := peer.checkConnectionLimit(); err != nil { return err } @@ -96,7 +96,7 @@ func (peer *localPeer) createConnection(localAddr string, peerAddr string, accep return err } connRemote := newRemoteConnection(peer.Peer, nil, tcpConn.RemoteAddr().String(), true, false) - startLocalConnection(connRemote, tcpConn, peer.router, acceptNewPeer) + startLocalConnection(connRemote, tcpConn, peer.router, acceptNewPeer, logger) return nil } @@ -150,10 +150,10 @@ func (peer *localPeer) actorLoop(actionChan <-chan localPeerAction) { func (peer *localPeer) handleAddConnection(conn ourConnection) error { if peer.Peer != conn.getLocal() { - log.Fatal("Attempt made to add connection to peer where peer is not the source of connection") + panic("Attempt made to add connection to peer where peer is not the source of connection") } if conn.Remote() == nil { - log.Fatal("Attempt made to add connection to peer with unknown remote peer") + panic("Attempt made to add connection to peer with unknown remote peer") } toName := conn.Remote().Name dupErr := fmt.Errorf("Multiple connections to %s added to %s", conn.Remote(), peer.String()) @@ -196,7 +196,7 @@ func (peer *localPeer) handleAddConnection(conn ourConnection) error { func (peer *localPeer) handleConnectionEstablished(conn ourConnection) { if peer.Peer != conn.getLocal() { - log.Fatal("Peer informed of active connection where peer is not the source of connection") + panic("Peer informed of active connection where peer is not the source of connection") } if dupConn, found := peer.connections[conn.Remote().Name]; !found || conn != dupConn { conn.shutdown(fmt.Errorf("Cannot set unknown connection active")) @@ -211,10 +211,10 @@ func (peer *localPeer) handleConnectionEstablished(conn ourConnection) { func (peer *localPeer) handleDeleteConnection(conn ourConnection) { if peer.Peer != conn.getLocal() { - log.Fatal("Attempt made to delete connection from peer where peer is not the source of connection") + panic("Attempt made to delete connection from peer where peer is not the source of connection") } if conn.Remote() == nil { - log.Fatal("Attempt made to delete connection to peer with unknown remote peer") + panic("Attempt made to delete connection to peer with unknown remote peer") } toName := conn.Remote().Name if connFound, found := peer.connections[toName]; !found || connFound != conn { diff --git a/metcd/metcdsrv/main.go b/metcd/metcdsrv/main.go index 3468a28..a81a3fb 100644 --- a/metcd/metcdsrv/main.go +++ b/metcd/metcdsrv/main.go @@ -77,7 +77,7 @@ func main() { ConnLimit: 64, PeerDiscovery: true, TrustedSubnets: []*net.IPNet{}, - }, name, *nickname, mesh.NullOverlay{}) + }, name, *nickname, mesh.NullOverlay{}, logger) // Create a meshconn.Peer. peer := meshconn.NewPeer(name, router.Ourself.UID, logger) diff --git a/metcd/server.go b/metcd/server.go index a86406d..6a077e6 100644 --- a/metcd/server.go +++ b/metcd/server.go @@ -140,7 +140,7 @@ func NewDefaultServer(minPeerCount int, logger *log.Logger) *wackygrpc.Server { ConnLimit: 64, PeerDiscovery: true, TrustedSubnets: []*net.IPNet{}, - }, peerName, nickName, mesh.NullOverlay{}) + }, peerName, nickName, mesh.NullOverlay{}, logger) // Create a meshconn.Peer and connect it to a channel. peer := meshconn.NewPeer(router.Ourself.Peer.Name, router.Ourself.UID, logger) diff --git a/mocks_test.go b/mocks_test.go index 9cc436b..f724a63 100644 --- a/mocks_test.go +++ b/mocks_test.go @@ -29,7 +29,7 @@ func (peers *Peers) AddTestRemoteConnection(p1, p2 *Peer) { fromPeer = peers.fetchWithDefault(fromPeer) toPeer := newPeerFrom(p2) toPeer = peers.fetchWithDefault(toPeer) - peers.ourself.addConnection(&remoteConnection{fromPeer, toPeer, "", false, false}) + peers.ourself.addConnection(newRemoteConnection(fromPeer, toPeer, "", false, false)) } func (peers *Peers) DeleteTestConnection(p *Peer) { @@ -45,8 +45,8 @@ func (peers *Peers) DeleteTestConnection(p *Peer) { // separate type in order to distinguish what is created by the test // from what is created by the real code. func newMockConnection(from, to *Peer) Connection { - type mockConnection struct{ remoteConnection } - return &mockConnection{remoteConnection{from, to, "", false, false}} + type mockConnection struct{ *remoteConnection } + return &mockConnection{newRemoteConnection(from, to, "", false, false)} } func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Connection) { diff --git a/peer.go b/peer.go index 899441b..22b4138 100644 --- a/peer.go +++ b/peer.go @@ -4,7 +4,6 @@ import ( "crypto/rand" "encoding/binary" "fmt" - "log" "sort" "strconv" ) @@ -169,7 +168,7 @@ func randomPeerShortID() PeerShortID { func randBytes(n int) []byte { buf := make([]byte, n) if _, err := rand.Read(buf); err != nil { - log.Fatal(err) + panic(err) } return buf } diff --git a/peers.go b/peers.go index 56bcd01..1190450 100644 --- a/peers.go +++ b/peers.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/gob" "io" - "log" "math/rand" "sync" ) @@ -521,7 +520,7 @@ func (peers *Peers) applyDecodedUpdate(decodedUpdate []*Peer, decodedConns [][]c func (peer *Peer) encode(enc *gob.Encoder) { if err := enc.Encode(peer.peerSummary); err != nil { - log.Fatal(err) + panic(err) } connSummaries := []connectionSummary{} @@ -535,7 +534,7 @@ func (peer *Peer) encode(enc *gob.Encoder) { } if err := enc.Encode(connSummaries); err != nil { - log.Fatal(err) + panic(err) } } diff --git a/router.go b/router.go index 08d9fe3..4db6ee1 100644 --- a/router.go +++ b/router.go @@ -53,10 +53,11 @@ type Router struct { gossipChannels gossipChannels topologyGossip Gossip acceptLimiter *tokenBucket + logger *log.Logger } // NewRouter returns a new router. It must be started. -func NewRouter(config Config, name PeerName, nickName string, overlay Overlay) *Router { +func NewRouter(config Config, name PeerName, nickName string, overlay Overlay, logger *log.Logger) *Router { router := &Router{Config: config, gossipChannels: make(gossipChannels)} if overlay == nil { @@ -67,12 +68,13 @@ func NewRouter(config Config, name PeerName, nickName string, overlay Overlay) * router.Ourself = newLocalPeer(name, nickName, router) router.Peers = newPeers(router.Ourself) router.Peers.OnGC(func(peer *Peer) { - log.Println("Removed unreachable peer", peer) + logger.Println("Removed unreachable peer", peer) }) router.Routes = newRoutes(router.Ourself, router.Peers) - router.ConnectionMaker = newConnectionMaker(router.Ourself, router.Peers, net.JoinHostPort(router.Host, "0"), router.Port, router.PeerDiscovery) + router.ConnectionMaker = newConnectionMaker(router.Ourself, router.Peers, net.JoinHostPort(router.Host, "0"), router.Port, router.PeerDiscovery, logger) router.topologyGossip = router.NewGossip("topology", router) router.acceptLimiter = newTokenBucket(acceptMaxTokens, acceptTokenDelay) + router.logger = logger return router } @@ -96,18 +98,18 @@ func (router *Router) usingPassword() bool { func (router *Router) listenTCP() { localAddr, err := net.ResolveTCPAddr("tcp4", net.JoinHostPort(router.Host, fmt.Sprint(router.Port))) if err != nil { - log.Fatal(err) + panic(err) } ln, err := net.ListenTCP("tcp4", localAddr) if err != nil { - log.Fatal(err) + panic(err) } go func() { defer ln.Close() for { tcpConn, err := ln.AcceptTCP() if err != nil { - log.Println(err) + router.logger.Println(err) continue } router.acceptTCP(tcpConn) @@ -118,20 +120,20 @@ func (router *Router) listenTCP() { func (router *Router) acceptTCP(tcpConn *net.TCPConn) { remoteAddrStr := tcpConn.RemoteAddr().String() - log.Printf("->[%s] connection accepted", remoteAddrStr) + router.logger.Printf("->[%s] connection accepted", remoteAddrStr) connRemote := newRemoteConnection(router.Ourself.Peer, nil, remoteAddrStr, false, false) - startLocalConnection(connRemote, tcpConn, router, true) + startLocalConnection(connRemote, tcpConn, router, true, router.logger) } // NewGossip returns a usable GossipChannel from the router. // // TODO(pb): rename? func (router *Router) NewGossip(channelName string, g Gossiper) Gossip { - channel := newGossipChannel(channelName, router.Ourself, router.Routes, g) + channel := newGossipChannel(channelName, router.Ourself, router.Routes, g, router.logger) router.gossipLock.Lock() defer router.gossipLock.Unlock() if _, found := router.gossipChannels[channelName]; found { - log.Fatalf("[gossip] duplicate channel %s", channelName) + panic(fmt.Sprintf("[gossip] duplicate channel %s", channelName)) } router.gossipChannels[channelName] = channel return channel @@ -149,7 +151,7 @@ func (router *Router) gossipChannel(channelName string) *gossipChannel { if channel, found = router.gossipChannels[channelName]; found { return channel } - channel = newGossipChannel(channelName, router.Ourself, router.Routes, &surrogateGossiper{}) + channel = newGossipChannel(channelName, router.Ourself, router.Routes, &surrogateGossiper{}, router.logger) channel.log("created surrogate channel") router.gossipChannels[channelName] = channel return channel @@ -277,7 +279,7 @@ func (router *Router) trusts(remote *remoteConnection) bool { } } else { // Should not happen as remoteTCPAddr was obtained from TCPConn - log.Printf("Unable to parse remote TCP addr: %s", err) + router.logger.Printf("Unable to parse remote TCP addr: %s", err) } return false }