-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
src/http-api/routes/debug.js
Outdated
const api = server.select('API') | ||
const gauge = new client.Gauge({ name: 'number_of_peers', help: 'the_number_of_currently_connected_peers' }) | ||
|
||
setInterval(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this approach. Guess we have two options, 1) (which is implemented here) is to once every X seconds, check the number of peers and save it or 2) when a request happens at /debug/metrics/prometheus
, then check the number of peers and respond.
@diasdavid what you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's calculate per request
Missing:
|
src/http-api/index.js
Outdated
// Setup debug metrics collection | ||
const collectDefaultMetrics = prometheusClient.collectDefaultMetrics | ||
collectDefaultMetrics({ timeout: 5000 }) | ||
prometheusGcStats(prometheusClient.register)() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that every js-ipfs node will "phone home"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diasdavid "phone home" as in send the data somewhere? No, it'll only be in the local node, available to collect when hitting the endpoint. The data is not sent somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it. Still, we should not add stuff that is not necessary by default. Can these be in a separate file that doesn't get loaded unless a env var is set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that makes sense, although it makes the usage different from go-ipfs who exposes it by default. But that makes sense, I'll move the dependencies to optionalDependencies as well then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! :)
src/http-api/routes/debug.js
Outdated
const api = server.select('API') | ||
const gauge = new client.Gauge({ name: 'number_of_peers', help: 'the_number_of_currently_connected_peers' }) | ||
|
||
setInterval(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's calculate per request
@victorbjelkholm how close are we from this? Any update? |
@diasdavid just resolved a conflict, once CI is passing, is ready to be merged 👍 |
rebase master onto this branch. it will solve the issues you are seeing. |
For basic metrics about running process
7eb8369
to
5abb83d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it deployed? How can I see some metrics already?
Sets up collectors which exposes metrics at
$API/debug/metrics/prometheus
(as in go-ipfs) that can be consumed by Prometheus.Ref: #804