feat: stats API (stats.bitswap and stats.repo) #1198

hacdias · 2018-01-29T19:06:46Z

Right now, js-ipfs doesn't implement any of the STATS spec and it is required to make IPFS Desktop work with js-ipfs.

Prerequisites and related PRs

ToDos

Not Fundamental, but also done in this PR

/api/v0/repo/version
ipfs repo version
ipfs.repo.version()

Closes #1197
Closes #1116

hacdias · 2018-01-30T15:36:23Z

Now I need to develop some kind of bandwidth metrics and I already know it is related to libp2p. I saw that with go-ipfs there is this library that was developed to take out metrics.

I was wondering if any of you (@diasdavid @dignifiedquire @pgte) could point me to the right direction since you are more into this project than I😄

pgte · 2018-01-30T16:32:31Z

Regarding bandwidth, you'll have to look into js-ipfs-bitswap.
There are two metrics: dataReceived and dataSent (bytes). But this only gives you a cumulative measure.

If you want per second, you can use a moving average. You have moving averages for 1, 5 and 15 minutes (this is configurable if you want to add a new interval).

There are no API docs (there is an issue about this somewhere), but here are the tests for this functionality: https://github.com/ipfs/js-ipfs-bitswap/blob/feat/benchmarks/test/bitswap-stats.js#L137-L145

Please tell me if you need more pointers.

hacdias · 2018-01-30T17:46:14Z

Thanks @pgte!

Another question: when we have this on the API how should we do the response on the HTTP API? I tried this:

    const stream = Readable()
    reply(stream)

    stream.push(Buffer.from(JSON.stringify({
      TotalIn: stat.totalIn,
      TotalOut: stat.totalOut,
      RateIn: stat.rateIn,
      RateOut: stat.rateOut,
    })))

but it didn't work

hacdias · 2018-01-30T17:57:09Z

@pgte could you take a look here? It seems to always print 0 for every field. And how should I get the stats for specific peers and protocols?

Ref.: Go implementation

pgte · 2018-01-30T18:53:27Z

What is the client expecting? Is it a new-line delimited JSON stream? If so, you will need to also output “\n” after each entry.

…

On 30 Jan 2018, 17:46 +0000, Henrique Dias ***@***.***>, wrote: Thanks @pgte! Another question: when we have this on the API how should we do the response on the HTTP API? I tried this: const stream = Readable() reply(stream) stream.push(Buffer.from(JSON.stringify({ TotalIn: stat.totalIn, TotalOut: stat.totalOut, RateIn: stat.rateIn, RateOut: stat.rateOut, }))) but it didn't work — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

hacdias · 2018-01-30T21:02:26Z

@pgte after digging into the source code I noticed we are using two funny special header x-stream-output and x-chunked-output. Didn't notice 😄

pgte · 2018-01-31T11:44:26Z

@hacdias are you still getting problems with the stats?

Regarding peer-specific stats: currently, js-ipfs-bitswap doesn't keep stats per peer.
It should be easy to add it. I'd be extra careful in garbage collection (removing stats for peers that are no longer relevant to us)..

hacdias · 2018-01-31T11:53:51Z

@pgte the stats always have zero values:

bw: promisify((options, callback) => {
  if (typeof options === 'function') {
    callback = options
    options = {}
  }

  options = Object.assign({
    peer: '',
    proto: '',
    poll: false,
    interval: '1s'
  }, options)

  let stats

  if (options.peer) {
    // TODO: stats for a specific peer
  } else if (options.proto) {
    // TODO: stats for a specific proto
  } else {
    const stat = self._bitswap.stat()
    const snapshot = stat.snapshot
    const movingAverages = stat.movingAverages

    stats = {
      totalIn: snapshot.dataReceived,
      totalOut: snapshot.dataSent,
      rateIn: movingAverages.dataReceived[60000].movingAverage() / 60, // Bytes per second
      rateOut: movingAverages.dataSent[60000].movingAverage() / 60 // Bytes per second
    }
  }

  callback(null, stats)
})

That code always gives me a 0 on each key.

pgte · 2018-01-31T12:01:19Z

@hacdias this may be a timing issue.
Stats updates are throttled to, by default, a max of 1 seconds. This is configurable, by the way, but the stats are always updated in bulk to minimize interference with particular operations.

I don't know if it helps, but, like exemplified in the test, the stat object emits an update event once they're updated, so maybe you can subscribe to that before replying? I suggest making some experiments with that..

pgte · 2018-01-31T12:02:14Z

and, of course, opening an issue on js-ipfs-bitswap (preferably with a reproducible problem if you can :) ) if you're not able to get values.

hacdias · 2018-01-31T17:35:35Z

@pgte I just added this:

to the place where bitswap is initialized. It seems that it never gets called (at least waiting 10 mins). I'm running js-ipfs directly with node src/cli/bin.js daemon.

pgte · 2018-02-01T08:44:21Z

@hacdias I think you'll have to enable stats like this.

pgte · 2018-02-01T08:45:36Z

... which I think may have to become a default setting in js-ipfs.
Or you could change the default in js-ipfs-bitswap if you don't want to add to the config clutter.

pgte · 2018-02-01T08:58:50Z

I will also be needing per-peer stats because of ipfs-inactive/dynamic-data-and-capabilities#3 (comment).

I opened bitswap issue here: ipfs/js-ipfs-bitswap#165

@hacdias could you state your requirements there?

pgte · 2018-02-02T10:11:56Z

@hacdias any luck enabling stats?

hacdias · 2018-02-02T14:08:09Z

@pgte not really, they are always 0.

pgte · 2018-02-02T15:47:34Z

@hacdias are the stats tests passing?:

$ mocha test/bitswap-stats.js

They are for me, and they test for > 0 values.

(also, keep in mind that values are Big: https://github.com/MikeMcl/big.js#readme

pgte · 2018-02-02T16:19:22Z

Took a stab at documenting current stats API:
ipfs/js-ipfs-bitswap@4b27d36

Hope this + tests help.

hacdias · 2018-02-07T09:19:54Z

Since libp2p/js-libp2p#157 might take a bit, I suggest to move Stats.Bandwidth to another PR and finish Stats.Bitswap and Stats.Repo here.

daviddias · 2018-02-14T12:13:10Z

@hacdias js-ipfs-api was released. You should be able to finish this PR now :)

hacdias · 2018-02-14T15:59:00Z

@diasdavid there is still one thing missing: we need to merge ipfs-inactive/interface-js-ipfs-core#216 in order to finish this PR.

daviddias · 2018-02-14T17:38:11Z

merging #1216 first and then back at this one :)

hacdias · 2018-02-14T19:53:52Z

OKay, as soon as you merge it, I'll finish this PR, but I'll still need ipfs-inactive/interface-js-ipfs-core#216 to be merged.

daviddias · 2018-02-15T09:19:03Z

@hacdias do it like this:

Rebase master onto this PR
Run tests locally and guarantee that everything passes
Ping me with everything ready so that I can merge and release and update the dep here in one go

daviddias · 2018-02-15T09:32:22Z

README.md

- [repo](https://github.com/ipfs/interface-ipfs-core/tree/master/SPEC/)
-  - `ipfs.repo.init`
-  - `ipfs.repo.version`
-  - `ipfs.repo.gc` (not implemented, yet!)


Why move these to the Graph section??

I would agree if moved to the Node Management Section

It's on the Node Management section

nvm, I got fooled twice by GH interface. all good :)

daviddias · 2018-02-15T09:33:18Z

src/cli/commands/bitswap/stat.js

+  wantlist [${stats.wantlist.length} keys]
+    ${stats.wantlist.join('\n    ')}
+  partners [${stats.peers.length}]
+    ${stats.peers.join('\n    ')}`)
    })
  }
 }


After Rebase, enable the CLI Bitswap test stat

Done and it works!!

daviddias · 2018-02-15T09:34:15Z

src/core/components/bitswap.js

      if (!self.isOnline()) {
-        throw new Error(OFFLINE_ERROR)
+        callback(new Error(OFFLINE_ERROR))


This needs two things:

Be wrapped with a setImmediate from the async module

Have a return, otherwise the rest of the function will be executed.

daviddias · 2018-02-15T09:35:51Z

src/core/components/start.js

-        self._peerInfoBook
+        Object.assign({
+          statsEnabled: true
+        }, self._peerInfoBook)


This is weird. Are you saying that in order to enable stats on Bitswap there is a variable on PeerBook that needs to be true? This Object Assign is also probably messing with the fact that PeerBook is a Object from Class and not an Object Literal. //cc @pgte

We need to have the stats enabled and this was the best way I found to do that.

@pgte could you comment here given that you implemented stats on Bitswap

I see. the thing is that bitswap doesn't need access to the PeerInfoBook at all. You can just pass the object options.

hacdias · 2018-02-15T10:09:22Z

@diasdavid the related tests to this PR seem to be working here 😄

daviddias

Thank you @hacdias <3

ghost assigned hacdias Jan 29, 2018

ghost added the status/in-progress In progress label Jan 29, 2018

hacdias force-pushed the stats-api branch from a3ddcbb to 6d4962d Compare January 29, 2018 20:27

hacdias changed the title ~~Implement Stats API~~ [WIP] Implement Stats API Jan 29, 2018

hacdias force-pushed the stats-api branch from 5ef74f5 to d6fb931 Compare January 30, 2018 08:36

hacdias mentioned this pull request Jan 30, 2018

Implement Stats API #1197

Closed

hacdias force-pushed the stats-api branch from 9665936 to a2560bf Compare January 30, 2018 18:51

This was referenced Jan 31, 2018

feat(breaking change): stats.bwReadableStream and stats.bwPullStream ipfs-inactive/interface-js-ipfs-core#211

Merged

feat(breaking change): use stream on stats.bw ipfs-inactive/js-ipfs-http-client#686

Merged

pgte mentioned this pull request Feb 1, 2018

per-peer stats ipfs/js-ipfs-bitswap#165

Closed

hacdias mentioned this pull request Feb 3, 2018

Implement statistics to support Connection Management libp2p/js-libp2p#157

Closed

daviddias mentioned this pull request Feb 14, 2018

Update js-ipfs to use latest libp2p and all the other deps #1216

Merged

daviddias changed the title ~~Implement Stats API (stats.bitswap and stats.repo)~~ feat: stats API (stats.bitswap and stats.repo) Feb 15, 2018

daviddias suggested changes Feb 15, 2018

View reviewed changes

hacdias force-pushed the stats-api branch 3 times, most recently from b8cd672 to bb5eece Compare February 15, 2018 10:02

hacdias and others added 3 commits February 15, 2018 11:39

feat: implement stats.vwbitswap and stats.repo

e0d5024

interface-ipfs-core commit

201e2d4

fix: only pass options to bitswap as last arg

4069995

daviddias force-pushed the stats-api branch from a168dcc to 4069995 Compare February 15, 2018 10:40

ghost assigned daviddias Feb 15, 2018

daviddias previously approved these changes Feb 15, 2018

View reviewed changes

test: fix command count

0625786

daviddias dismissed their stale review via 0625786 February 15, 2018 10:47

daviddias approved these changes Feb 15, 2018

View reviewed changes

daviddias merged commit 905bdc0 into master Feb 15, 2018

daviddias deleted the stats-api branch February 15, 2018 11:02

ghost removed the status/in-progress In progress label Feb 15, 2018

JonKrone pushed a commit that referenced this pull request Feb 16, 2018

feat: stats API (stats.bitswap and stats.repo) (#1198)

58ef77b

daviddias mentioned this pull request Feb 17, 2018

Stats API errors #1225

Closed

JonKrone pushed a commit to JonKrone/js-ipfs that referenced this pull request Feb 19, 2018

feat: stats API (stats.bitswap and stats.repo) (ipfs#1198)

d882238

This was referenced Feb 20, 2018

feat: .stats.bw* - Bandwidth Stats #1230

Merged

Expose new bitswap stats #1150

Closed

dryajov pushed a commit that referenced this pull request Feb 28, 2018

feat: stats API (stats.bitswap and stats.repo) (#1198)

19adbce

feat: stats API (stats.bitswap and stats.repo) #1198

feat: stats API (stats.bitswap and stats.repo) #1198

Conversation

hacdias commented Jan 29, 2018 • edited Loading

Prerequisites and related PRs

ToDos

Not Fundamental, but also done in this PR

hacdias commented Jan 30, 2018 • edited Loading

pgte commented Jan 30, 2018 • edited Loading

hacdias commented Jan 30, 2018

hacdias commented Jan 30, 2018

pgte commented Jan 30, 2018 via email

hacdias commented Jan 30, 2018

pgte commented Jan 31, 2018

hacdias commented Jan 31, 2018

pgte commented Jan 31, 2018

pgte commented Jan 31, 2018

hacdias commented Jan 31, 2018 • edited Loading

pgte commented Feb 1, 2018

pgte commented Feb 1, 2018

pgte commented Feb 1, 2018

pgte commented Feb 2, 2018

hacdias commented Feb 2, 2018

pgte commented Feb 2, 2018

pgte commented Feb 2, 2018

hacdias commented Feb 7, 2018

daviddias commented Feb 14, 2018

hacdias commented Feb 14, 2018

daviddias commented Feb 14, 2018

hacdias commented Feb 14, 2018

daviddias commented Feb 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacdias commented Feb 15, 2018

daviddias left a comment

Choose a reason for hiding this comment

hacdias commented Jan 29, 2018 •

edited

Loading

hacdias commented Jan 30, 2018 •

edited

Loading

pgte commented Jan 30, 2018 •

edited

Loading

hacdias commented Jan 31, 2018 •

edited

Loading