-
Notifications
You must be signed in to change notification settings - Fork 249
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.
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
feat(telemetry)_: send peer count metric #5460
Conversation
Jenkins BuildsClick to see older builds (13)
|
96d84bc
to
42e6ef7
Compare
wakuv2/waku.go
Outdated
@@ -1399,16 +1400,21 @@ func (w *Waku) Start() error { | |||
case <-peerCountTimer.C: | |||
peerCountTimer.Reset(3 * time.Second) | |||
newPeerCount := len(w.node.Host().Network().Peers()) | |||
if newPeerCount != peerCount && w.onPeerStats != nil { | |||
if newPeerCount != peerCount && (w.onPeerStats != nil || w.statusTelemetryClient != nil) { |
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.
Why don't we want to set peerCount
is onPeerStats
and statusTelemetryClient
are nil
? IMO you don't need to check for the nil
here since you also do it later - or am I missing anything?
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.
We update the peer count if either one is true. We only call onPeerStats
and/or statusTelemetryClient
if either one 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.
LGTM in general, added one comment
0042450
to
4c51832
Compare
4c51832
to
b0b3064
Compare
When peer count timer is fired and peer count has changed, send a message to telemetry service indicating the node's peer count at the current timestamp.
Also logs decoded error from telemetry service and fixes expected response code.
Important changes:
Related to status-im/telemetry#21