test: remove TestTracer

This test is exceptionally racy and IMO useless (you can go read the 10 lines of code making up tracing and convaince yourself it's working.)

bitswap_test.go

@@ -12,10 +12,8 @@ import (
 	"github.com/ipfs/go-bitswap"
 	testinstance "github.com/ipfs/go-bitswap/client/testinstance"
 	bsmsg "github.com/ipfs/go-bitswap/message"
-	pb "github.com/ipfs/go-bitswap/message/pb"
 	"github.com/ipfs/go-bitswap/server"
 	tn "github.com/ipfs/go-bitswap/testnet"
-	"github.com/ipfs/go-bitswap/tracer"
 	blocks "github.com/ipfs/go-block-format"
 	cid "github.com/ipfs/go-cid"
 	detectrace "github.com/ipfs/go-detect-race"
@@ -830,158 +828,3 @@ func TestWithScoreLedger(t *testing.T) {
 		t.Fatal("Expected the score ledger to be closed within 5s")
 	}
 }
-
-type logItem struct {
-	dir byte
-	pid peer.ID
-	msg bsmsg.BitSwapMessage
-}
-type mockTracer struct {
-	mu  sync.Mutex
-	log []logItem
-}
-
-func (m *mockTracer) MessageReceived(p peer.ID, msg bsmsg.BitSwapMessage) {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	m.log = append(m.log, logItem{'r', p, msg})
-}
-func (m *mockTracer) MessageSent(p peer.ID, msg bsmsg.BitSwapMessage) {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	m.log = append(m.log, logItem{'s', p, msg})
-}
-
-func (m *mockTracer) getLog() []logItem {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	return m.log[:len(m.log):len(m.log)]
-}
-
-func TestTracer(t *testing.T) {
-	net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(kNetworkDelay))
-	ig := testinstance.NewTestInstanceGenerator(net, nil, nil)
-	defer ig.Close()
-	bg := blocksutil.NewBlockGenerator()
-
-	instances := ig.Instances(3)
-	blocks := bg.Blocks(2)
-
-	// Install Tracer
-	wiretap := new(mockTracer)
-	updateTracer(instances[0].Exchange, wiretap)
-
-	// First peer has block
-	addBlock(t, context.Background(), instances[0], blocks[0])
-
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
-	defer cancel()
-
-	// Second peer broadcasts want for block CID
-	// (Received by first and third peers)
-	_, err := instances[1].Exchange.GetBlock(ctx, blocks[0].Cid())
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// When second peer receives block, it should send out a cancel, so third
-	// peer should no longer keep second peer's want
-	if err = tu.WaitFor(ctx, func() error {
-		if len(instances[2].Exchange.WantlistForPeer(instances[1].Peer)) != 0 {
-			return fmt.Errorf("should have no items in other peers wantlist")
-		}
-		if len(instances[1].Exchange.GetWantlist()) != 0 {
-			return fmt.Errorf("shouldnt have anything in wantlist")
-		}
-		return nil
-	}); err != nil {
-		t.Fatal(err)
-	}
-
-	log := wiretap.getLog()
-
-	// After communication, 3 messages should be logged via Tracer
-	if l := len(log); l != 3 {
-		t.Fatal("expected 3 items logged via Tracer, found", l)
-	}
-
-	// Received: 'Have'
-	if log[0].dir != 'r' {
-		t.Error("expected message to be received")
-	}
-	if log[0].pid != instances[1].Peer {
-		t.Error("expected peer", instances[1].Peer, ", found", log[0].pid)
-	}
-	if l := len(log[0].msg.Wantlist()); l != 1 {
-		t.Fatal("expected 1 entry in Wantlist, found", l)
-	}
-	if log[0].msg.Wantlist()[0].WantType != pb.Message_Wantlist_Have {
-		t.Error("expected WantType equal to 'Have', found 'Block'")
-	}
-
-	// Sent: Block
-	if log[1].dir != 's' {
-		t.Error("expected message to be sent")
-	}
-	if log[1].pid != instances[1].Peer {
-		t.Error("expected peer", instances[1].Peer, ", found", log[1].pid)
-	}
-	if l := len(log[1].msg.Blocks()); l != 1 {
-		t.Fatal("expected 1 entry in Blocks, found", l)
-	}
-	if log[1].msg.Blocks()[0].Cid() != blocks[0].Cid() {
-		t.Error("wrong block Cid")
-	}
-
-	// Received: 'Cancel'
-	if log[2].dir != 'r' {
-		t.Error("expected message to be received")
-	}
-	if log[2].pid != instances[1].Peer {
-		t.Error("expected peer", instances[1].Peer, ", found", log[2].pid)
-	}
-	if l := len(log[2].msg.Wantlist()); l != 1 {
-		t.Fatal("expected 1 entry in Wantlist, found", l)
-	}
-	if log[2].msg.Wantlist()[0].WantType != pb.Message_Wantlist_Block {
-		t.Error("expected WantType equal to 'Block', found 'Have'")
-	}
-	if log[2].msg.Wantlist()[0].Cancel != true {
-		t.Error("expected entry with Cancel set to 'true'")
-	}
-
-	// After disabling WireTap, no new messages are logged
-	updateTracer(instances[0].Exchange, nil)
-
-	addBlock(t, context.Background(), instances[0], blocks[1])
-
-	_, err = instances[1].Exchange.GetBlock(ctx, blocks[1].Cid())
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err = tu.WaitFor(ctx, func() error {
-		if len(instances[1].Exchange.GetWantlist()) != 0 {
-			return fmt.Errorf("shouldnt have anything in wantlist")
-		}
-		return nil
-	}); err != nil {
-		t.Fatal(err)
-	}
-
-	log = wiretap.getLog()
-
-	if l := len(log); l != 3 {
-		t.Fatal("expected 3 items logged via WireTap, found", l)
-	}
-
-	for _, inst := range instances {
-		err := inst.Exchange.Close()
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-}
-
-func updateTracer(bs *bitswap.Bitswap, tap tracer.Tracer) {
-	bitswap.WithTracer(tap).V.(func(*bitswap.Bitswap))(bs)
-}

options.go

@@ -14,7 +14,7 @@ type option func(*Bitswap)
 // Option is interface{} of server.Option or client.Option or func(*Bitswap)
 // wrapped in a struct to gain strong type checking.
 type Option struct {
-	V interface{}
+	v interface{}
 }
 
 func EngineBlockstoreWorkerCount(count int) Option {
@@ -74,15 +74,6 @@ func WithTracer(tap tracer.Tracer) Option {
 	return Option{
 		func(bs *Bitswap) {
 			bs.tracer = tap
-			// the tests use this to hot update tracers, we need to update tracers of impls if we are running
-			if bs.Client != nil {
-				if tap != nil {
-					tap = nopReceiveTracer{tap}
-				}
-				client.WithTracer(tap)(bs.Client)
-				// no need to check for server as they can't not be both running
-				server.WithTracer(tap)(bs.Server)
-			}
 		},
 	}
 }

polyfill.go

@@ -63,7 +63,7 @@ func New(ctx context.Context, net network.BitSwapNetwork, bstore blockstore.Bloc
 	var clientOptions []client.Option
 
 	for _, o := range options {
-		switch typedOption := o.V.(type) {
+		switch typedOption := o.v.(type) {
 		case server.Option:
 			serverOptions = append(serverOptions, typedOption)
 		case client.Option: