Skip to content

Commit

Permalink
Remove all global log invocations
Browse files Browse the repository at this point in the history
  • Loading branch information
peterbourgon committed Mar 24, 2016
1 parent 16cb49a commit 124627b
Show file tree
Hide file tree
Showing 13 changed files with 84 additions and 62 deletions.
16 changes: 12 additions & 4 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,17 @@ type remoteConnection struct {
remoteTCPAddr string
outbound bool
established bool
logger *log.Logger
}

func newRemoteConnection(from, to *Peer, tcpAddr string, outbound bool, established bool) *remoteConnection {
func newRemoteConnection(from, to *Peer, tcpAddr string, outbound bool, established bool, logger *log.Logger) *remoteConnection {
return &remoteConnection{
local: from,
remote: to,
remoteTCPAddr: tcpAddr,
outbound: outbound,
established: established,
logger: logger,
}
}

Expand All @@ -57,6 +59,12 @@ func (conn *remoteConnection) isOutbound() bool { return conn.outbound }

func (conn *remoteConnection) isEstablished() bool { return conn.established }

func (conn *remoteConnection) shutdown(error) {}

func (conn *remoteConnection) log(args ...interface{}) {
conn.logger.Println(append(append([]interface{}{}, fmt.Sprintf("->[%s|%s]:", conn.remoteTCPAddr, conn.remote)), args...)...)
}

// LocalConnection is the local (our) side of a connection.
// It implements ProtocolSender, and manages per-channel GossipSenders.
type LocalConnection struct {
Expand All @@ -82,7 +90,7 @@ type LocalConnection struct {
// connections map.
func startLocalConnection(connRemote *remoteConnection, tcpConn *net.TCPConn, router *Router, acceptNewPeer bool) {
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)
Expand Down Expand Up @@ -377,14 +385,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)
}
}

Expand Down
8 changes: 5 additions & 3 deletions connection_maker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)
cm.logger.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] error during connection attempt: %v", address, err)
cm.connectionAborted(address, err)
}
}
Expand Down
26 changes: 12 additions & 14 deletions examples/increment-only-counter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"flag"
"fmt"
"io/ioutil"
"log"
"net"
"net/http"
Expand All @@ -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{
Expand All @@ -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 {
Expand Down
8 changes: 5 additions & 3 deletions gossip_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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.
Expand All @@ -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()
Expand Down
6 changes: 4 additions & 2 deletions gossip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package mesh

import (
"fmt"
"io/ioutil"
"log"
"sync"
"testing"

Expand All @@ -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
}
Expand Down Expand Up @@ -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: remoteConnection{router.Ourself.Peer, toPeer, "", false, true, log.New(ioutil.Discard, "", 0)},
dest: r,
start: make(chan struct{}),
}
Expand Down
16 changes: 9 additions & 7 deletions local_peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,20 @@ type localPeer struct {
*Peer
router *Router
actionChan chan<- localPeerAction
logger *log.Logger
}

// The actor closure used by localPeer.
type localPeerAction func()

// newLocalPeer returns a usable LocalPeer.
func newLocalPeer(name PeerName, nickName string, router *Router) *localPeer {
func newLocalPeer(name PeerName, nickName string, router *Router, logger *log.Logger) *localPeer {
actionChan := make(chan localPeerAction, ChannelSize)
peer := &localPeer{
Peer: newPeer(name, nickName, randomPeerUID(), 0, randomPeerShortID()),
router: router,
actionChan: actionChan,
logger: logger,
}
go peer.actorLoop(actionChan)
return peer
Expand Down Expand Up @@ -95,7 +97,7 @@ func (peer *localPeer) createConnection(localAddr string, peerAddr string, accep
if err != nil {
return err
}
connRemote := newRemoteConnection(peer.Peer, nil, tcpConn.RemoteAddr().String(), true, false)
connRemote := newRemoteConnection(peer.Peer, nil, tcpConn.RemoteAddr().String(), true, false, peer.logger)
startLocalConnection(connRemote, tcpConn, peer.router, acceptNewPeer)
return nil
}
Expand Down Expand Up @@ -150,10 +152,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())
Expand Down Expand Up @@ -196,7 +198,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"))
Expand All @@ -211,10 +213,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 {
Expand Down
2 changes: 1 addition & 1 deletion metcd/metcdsrv/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion metcd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions mocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package mesh

import (
"fmt"
"io/ioutil"
"log"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -29,7 +31,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(&remoteConnection{fromPeer, toPeer, "", false, false, log.New(ioutil.Discard, "", 0)})
}

func (peers *Peers) DeleteTestConnection(p *Peer) {
Expand All @@ -46,7 +48,7 @@ func (peers *Peers) DeleteTestConnection(p *Peer) {
// 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}}
return &mockConnection{remoteConnection{from, to, "", false, false, log.New(ioutil.Discard, "", 0)}}
}

func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Connection) {
Expand Down
3 changes: 1 addition & 2 deletions peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"crypto/rand"
"encoding/binary"
"fmt"
"log"
"sort"
"strconv"
)
Expand Down Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 124627b

Please sign in to comment.