Enhancing Prometheus metrics for BW consumption

[staheri14]

As part of celestiaorg/celestia-app#2197, in order to be able to calculate the breakdown of BW consumption per chain ID and reactor, we would like to use to the existing Prometheus metrics in cekestia-core. However, it is not clear whether the current metrics are sufficient and whether we may require additional metrics to accomplish the task.
This issue intends to track the progress of this effort.

[staheri14]

Currently, Prometheus collects the sent and received BW, however, they lack two major labels: Channel id and the reactor. So we need to integrate these two.

[staheri14]

In the existing implementation, there is a single reactor associated with each channel ID. Therefore, we can deduce the reactors based solely on the channel ID label. Given this, our approach will involve appending only the channel ID label to each metric.
References:

celestia-core/node/node.go

Lines 633 to 637 in 2f93fc8

	sw.AddReactor("MEMPOOL", mempoolReactor)
	sw.AddReactor("BLOCKCHAIN", bcReactor)
	sw.AddReactor("CONSENSUS", consensusReactor)
	sw.AddReactor("EVIDENCE", evidenceReactor)
	sw.AddReactor("STATESYNC", stateSyncReactor)

celestia-core/node/node.go

Line 690 in 2f93fc8

sw.AddReactor("PEX", pexReactor)

celestia-core/node/node.go

Lines 153 to 158 in 2f93fc8

	// - MEMPOOL
	// - BLOCKCHAIN
	// - CONSENSUS
	// - EVIDENCE
	// - PEX
	// - STATESYNC

[rootulp]

In the existing implementation, there is a single reactor associated with each channel ID.

Is this guaranteed for future modifications to CometBFT? Is there a downside to including two labels?

[staheri14]

Is this guaranteed for future modifications to CometBFT?

The 1:1 correspondence between reactors and channel IDs is not strictly enforced by the current implementation, as custom reactors can be added to switches on any channel ID. (please refer to #1077 (comment)). However, it is probable, based on past trends, that the one-to-one mapping will continue to be maintained in the future (although not guaranteed with 100% certainty).

Is there a downside to including two labels?

Thank you for your suggestion. Our current implementation, which tracks channel ID, adequately addresses the requirements of this issue. Adding an extra label could introduce redundancy and complexity without a significant improvement in observability.
Our initial goal was to minimize modifications to the existing Prometheus metrics. Currently, we monitor sent and received bytes at the peer level, the central point of communication with peers. Extending this observability to specific reactors would require changes at higher levels, increasing the scope significantly. The reason is that in the peer level there is no observability into the reactors to which/from which the message sent. Identifying the exact reactor needs to be done in higher level components, and the surface would much larger.

While enhancing observability by identifying reactors is possible, the current approach was opted to maintain simplicity and efficiency while meeting the objectives of this issue.

[staheri14]

apparently, no two reactors can share the same channel ID as suggested in

celestia-core/p2p/switch.go

Line 166 in 3f3b7cc

// No two reactors can share the same channel.

[evan-forbes]

if I recall correctly, I've been able to determine the bandwidth used before via channel ID, but would have to dig through the metrics. Will hopefully be able to do that later today depending on if I can get the network tests working again

[staheri14]

if I recall correctly, I've been able to determine the bandwidth used before via channel ID, but would have to dig through the metrics. Will hopefully be able to do that later today depending on if I can get the network tests working again

[Reiterating the findings from our discussion in #1078], it has been clarified that the metrics in question are peer_send_bytes_total and peer_receive_bytes_total. Although these metrics include channel IDs, they are presently missing a message type label, rendering them insufficient for providing the desired level of granularity. Nevertheless, the decision reached that we could use them to cross-check results extracted from message_receive_bytes_total and message_send_bytes_total.

[staheri14]

closed by #1091