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.

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: includes peer_id as a label in the MessageSendBytesTotal and MessageReceiveBytesTotal Prometheus metrics #1086

Merged
merged 6 commits into from
Sep 22, 2023

Conversation

staheri14
Copy link
Collaborator

@staheri14 staheri14 commented Sep 19, 2023

Inline with #1077

Our past experimentation showed that none of the current Prometheus traffic related metrics encompass all the information regarding the message type, peer ID and channel ID. This deficiency can be addressed by incorporating peer IDs for message_receive_bytes_total and message_send_bytes_total. This PR provides this feature.

I tested it by executing a local validator node and examining the Prometheus metrics endpoint. Here's an example of the output:

cometbft_p2p_message_send_bytes_total{chID="0x40", chain_id="mocha-4", message_type="blockchain_StatusResponse", peer_id="cb7adca6fbaabc5336c5eebbc1312390a2bb9d2d", version="1.0.0-rc0-278-g4c452c5f4"} 8

@staheri14 staheri14 changed the base branch from main to v0.34.x-celestia September 19, 2023 23:03
@staheri14 staheri14 self-assigned this Sep 19, 2023
@staheri14 staheri14 changed the title adds peer_id as label to MessageSendBytesTotal and MessageReceiveBytesTotal feat: includes peer_id as a label in the MessageSendBytesTotal and MessageReceiveBytesTotal Prometheus metrics Sep 20, 2023
@staheri14 staheri14 marked this pull request as ready for review September 20, 2023 20:55
@cmwaters
Copy link
Contributor

Do you think the bandwidth may vary per peer?

@staheri14
Copy link
Collaborator Author

Do you think the bandwidth may vary per peer?

Good question, Indeed, it can and it does, especially in scenarios involving request-reply protocols (like in the CAT mempool), or when a connected peer is lagging behind and attempting to synchronize, as this often results in a substantial portion of network traffic being consumed by that peer.

@staheri14 staheri14 merged commit ad660fe into v0.34.x-celestia Sep 22, 2023
@staheri14 staheri14 deleted the sanaz/adds-missing-labels branch September 22, 2023 20:23
@faddat faddat mentioned this pull request Feb 22, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants