chore(metrics): expose reset as part of the interface

[Stebalien]

#71 added the function to the implementation but didn't change the interface.

[kpp]

@Kubuxu compat-check was intoduced in #40. How do we pass the test when we add a new method to an interface?

[Kubuxu]

In this case, it isn't that important but as mater of fact that change is not compatible with older libp2p-core.

If someone was implementing custom Reporter, this change will break it.

[Stebalien]

Actually, I don't think we need this. Users construct a BandwidthCounter and then pass it into libp2p. Users can still call Reset on the BandwidthCounter so it doesn't need to appear on the reporter interface.

[kpp]

The only way I found to construct a node is NewNode with cfg.Swarm.DisableBandwidthMetrics = false which returns core.IpfsNode with Reporter metrics.Reporter inside.

Is there another way I can Reset metrics?

BTW if I could replace P2P.BandwidthCounter directly why did we add those methods? I could have replace BandwidthCounter with a new one each time I needed a fresh instance of BandwidthCounter

[Stebalien]

The only way I found to construct a node is NewNode with cfg.Swarm.DisableBandwidthMetrics = false which returns core.IpfsNode with Reporter metrics.Reporter inside.

I was talking about libp2p. In libp2p, you construct the bandwidth reporter by passing the libp2p.BandwidthReporter(reporer) option.

In go-ipfs, we can:

1. Create a new super-interface with the Reset method.
2. Replace the IpfsNode.Reporter type with this new interface.
3. Change the BandwidthCounter (in core/node/libp2p/transport.go) to return this new interface instead of metrics.Reporter).

BTW if I could replace P2P.BandwidthCounter directly why did we add those methods? I could have replace BandwidthCounter with a new one each time I needed a fresh instance of BandwidthCounter

You can't. We pass a single bandwidth counter into libp2p and can't replace it with a new one.

Well, we could create a special "replaceable" bandwidth counter protected by a read/write lock.

[kpp]

Is there anything I can help with?

[Stebalien]

Could you add a subcommand to IPFS: ipfs stats bw reset. This should call Reset on the metrics reporter.

To expose the metrics reporter, apply the following patch:

diff --git a/core/core.go b/core/core.go
index c8633f975..0649a8c68 100644
--- a/core/core.go
+++ b/core/core.go
@@ -66,16 +66,16 @@ type IpfsNode struct {
 	PNetFingerprint libp2p.PNetFingerprint `optional:"true"` // fingerprint of private network
 
 	// Services
-	Peerstore       pstore.Peerstore     `optional:"true"` // storage for other Peer instances
-	Blockstore      bstore.GCBlockstore  // the block store (lower level)
-	Filestore       *filestore.Filestore `optional:"true"` // the filestore blockstore
-	BaseBlocks      node.BaseBlocks      // the raw blockstore, no filestore wrapping
-	GCLocker        bstore.GCLocker      // the locker used to protect the blockstore during gc
-	Blocks          bserv.BlockService   // the block service, get/add blocks.
-	DAG             ipld.DAGService      // the merkle dag service, get/add objects.
-	Resolver        *resolver.Resolver   // the path resolution system
-	Reporter        metrics.Reporter     `optional:"true"`
-	Discovery       discovery.Service    `optional:"true"`
+	Peerstore       pstore.Peerstore          `optional:"true"` // storage for other Peer instances
+	Blockstore      bstore.GCBlockstore       // the block store (lower level)
+	Filestore       *filestore.Filestore      `optional:"true"` // the filestore blockstore
+	BaseBlocks      node.BaseBlocks           // the raw blockstore, no filestore wrapping
+	GCLocker        bstore.GCLocker           // the locker used to protect the blockstore during gc
+	Blocks          bserv.BlockService        // the block service, get/add blocks.
+	DAG             ipld.DAGService           // the merkle dag service, get/add objects.
+	Resolver        *resolver.Resolver        // the path resolution system
+	Reporter        *metrics.BandwidthCounter `optional:"true"`
+	Discovery       discovery.Service         `optional:"true"`
 	FilesRoot       *mfs.Root
 	RecordValidator record.Validator
 
diff --git a/core/node/libp2p/transport.go b/core/node/libp2p/transport.go
index 526776ab3..a08ec86c1 100644
--- a/core/node/libp2p/transport.go
+++ b/core/node/libp2p/transport.go
@@ -31,7 +31,7 @@ func Security(enabled, preferTLS bool) interface{} {
 	}
 }
 
-func BandwidthCounter() (opts Libp2pOpts, reporter metrics.Reporter) {
+func BandwidthCounter() (opts Libp2pOpts, reporter *metrics.BandwidthCounter) {
 	reporter = metrics.NewBandwidthCounter()
 	opts.Opts = append(opts.Opts, libp2p.BandwidthReporter(reporter))
 	return opts, reporter