waku-org / nwaku

Waku node and protocol.
Other
198 stars 50 forks source link

bug(waku-discv5): Metric waku_discv5_discovered is not updated correctly #1288

Open alrevuelta opened 1 year ago

alrevuelta commented 1 year ago

Problem

The metric waku_discv5_discovered is not updated correctly, see ref. It is increased .inc every time we queryRandom() but this call can return duplicated peers that were already considered, so we end up having a number of discovered peers that don't make sense. For example, the number of discovered peers in one wakuv2.prod node was 20.000+.

Impact

Low impact, just monitoring metrics.

To reproduce

Expected behavior

Metric waku_discv5_discovered should reflect the numbers of discovered peers without including duplicates.

Screenshots/logs

image

(Note that this dashboard graph doesn't exist in our grafana, I added it to showcase the issue but didn't save it)

nwaku version/commit hash

Relase 2022-10-06 v0.12.0

Additional context

Related to #1010

alrevuelta commented 1 year ago

One possible solution would be to remove waku_discv5_discovered and rely on routing_table_nodes metric from eth/p2p/discoveryv5/routing_table.nim, which ultimately should be the source of truth: see metric.

wdyt @jm-clius ?

jm-clius commented 1 year ago

Ah, this comment slipped through the cracks. Yes, I don't think it makes sense to keep this metric if the number of nodes in the routing table is what we really care about.