Skip to content

Commit

Permalink
Merge #1477
Browse files Browse the repository at this point in the history
1477: Backport HotStuff view metric fixes from `v0.22-mainnet` r=jordanschalm a=jordanschalm

Fixes two issues with HotStuff view metric from Mainnet deployment branch:
* `HotstuffViewFun` unset caused panic on non-consensus nodes
* `hotstuffCurView` metric missing `role` label

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Kay-Zee <kan@axiomzen.co>
Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com>
Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
  • Loading branch information
4 people committed Oct 15, 2021
2 parents 674122b + 2a554df commit 8521505
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 7 deletions.
6 changes: 6 additions & 0 deletions cmd/scaffold.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ func (fnb *FlowNodeBuilder) EnqueueNetworkInit() {
}
return head.Height, nil
},
HotstuffViewFun: nil, // set in next code block, depending on role
}

// only consensus roles will need to report hotstuff view
Expand All @@ -191,6 +192,11 @@ func (fnb *FlowNodeBuilder) EnqueueNetworkInit() {

return curView, nil
}
} else {
// non-consensus will not report any hotstuff view
pingProvider.HotstuffViewFun = func() (uint64, error) {
return 0, fmt.Errorf("non-consensus nodes do not report hotstuff view in ping")
}
}

streamFactory, err := p2p.LibP2PStreamCompressorFactoryFunc(fnb.BaseConfig.LibP2PStreamCompression)
Expand Down
21 changes: 14 additions & 7 deletions module/metrics/ping.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func NewPingCollector() *PingCollector {
Namespace: namespaceNetwork,
Subsystem: subsystemGossip,
Help: "the hotstuff current view",
}, []string{LabelNodeID, LabelNodeAddress, LabelNodeRole, LabelNodeInfo}),
}, []string{LabelNodeID, LabelNodeAddress, LabelNodeInfo}),
}

return pc
Expand Down Expand Up @@ -65,10 +65,17 @@ func (pc *PingCollector) NodeInfo(node *flow.Identity, nodeInfo string, version
LabelNodeVersion: version}).
Set(float64(sealedHeight))

pc.hotstuffCurView.With(prometheus.Labels{
LabelNodeID: node.NodeID.String(),
LabelNodeAddress: node.Address,
LabelNodeInfo: nodeInfo,
}).
Set(float64(hotstuffCurView))
// we only need this metrics from consensus nodes.
// since non-consensus nodes will report this metrics as well, and the value will always be 0,
// we can exclude metrics from non-consensus nodes by checking if the value is above 0.
// consensus nodes will start this metrics value with 0 as well, and won't report, but that's
// OK, because their hotstuff view will quickly go up and start reporting.
if hotstuffCurView > 0 {
pc.hotstuffCurView.With(prometheus.Labels{
LabelNodeID: node.NodeID.String(),
LabelNodeAddress: node.Address,
LabelNodeInfo: nodeInfo,
}).
Set(float64(hotstuffCurView))
}
}

0 comments on commit 8521505

Please sign in to comment.