Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: add back waku discv5 metrics #2927

Merged
merged 3 commits into from
Jul 26, 2024
Merged

fix: add back waku discv5 metrics #2927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf141f6
add back metrics
SionoiS Jul 23, 2024
c999493
more metrics
SionoiS Jul 23, 2024
58dbea7
Update waku/discovery/waku_discv5.nim
SionoiS Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions waku/discovery/waku_discv5.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ proc findRandomPeers*(
elif wd.predicate.isSome():
discoveredRecords = discoveredRecords.filter(wd.predicate.get())

waku_discv5_discovered.inc(discoveredRecords.len)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only concern that I have is if we receive the same peer twice, we will update this metric both times.

Another alternative is increasing it in addPeer() if the origin is PeerOrigin.Discv5 after passing the duplicate checks

Copy link
Contributor Author

@SionoiS SionoiS Jul 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I thought we had another metric for that... I could add both.

This one say number of nodes discovered so maybe we don't filter?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good :) Thank you!


return discoveredRecords

proc searchLoop(wd: WakuDiscoveryV5) {.async.} =
Expand All @@ -233,6 +235,7 @@ proc searchLoop(wd: WakuDiscoveryV5) {.async.} =
let peerInfo = record.toRemotePeerInfo().valueOr:
## in case of error, we keep track of it for debugging purposes
wrongRecordsReasons.add(($record, $error))
waku_discv5_errors.inc(labelValues = [$error])
continue

discoveredPeers.add(peerInfo)
Expand Down
3 changes: 3 additions & 0 deletions waku/node/peer_manager/peer_manager.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ declarePublicGauge waku_streams_peers,
declarePublicGauge waku_peer_store_size, "Number of peers managed by the peer store"
declarePublicGauge waku_service_peers,
"Service peer protocol and multiaddress ", labels = ["protocol", "peerId"]
declarePublicGauge waku_total_unique_peers, "total number of unique peers"

logScope:
topics = "waku node peer_manager"
Expand Down Expand Up @@ -141,6 +142,8 @@ proc addPeer*(
trace "Adding peer to manager",
peerId = remotePeerInfo.peerId, addresses = remotePeerInfo.addrs

waku_total_unique_peers.inc()

pm.peerStore[AddressBook][remotePeerInfo.peerId] = remotePeerInfo.addrs
pm.peerStore[KeyBook][remotePeerInfo.peerId] = remotePeerInfo.publicKey
pm.peerStore[SourceBook][remotePeerInfo.peerId] = origin
Expand Down
Loading