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

Prometheus metrics for nodeos_p2p_connections don't use stable identifier in 5.0 #1683

Closed
matthewdarwin opened this issue Sep 27, 2023 · 4 comments · Fixed by #1750 or #1765
Closed
Assignees
Labels
bug Something isn't working 👍 lgtm OCI Work exclusive to OCI team

Comments

@matthewdarwin
Copy link

matthewdarwin commented Sep 27, 2023

Prometheus metrics for nodeos_p2p_connections don't use stable identifier. "59" will change here:

nodeos_p2p_connections{connid_59="bytes_sent"} 41
nodeos_p2p_connections{connid_55="last_available_block"} 100127462
nodeos_p2p_connections{connid_55="accepting_blocks"} 1
nodeos_p2p_connections{connid_54="eosn-jungle4-trxrly28:9876 - 8c5a970"} 0
nodeos_p2p_connections{connid_59="last_bytes_received"} 1.695820203477813e+18

Better to use something like:

nodeos_p2p_connections{connid="8c5a970", type="bytes_sent"} 41
nodeos_p2p_connections{connid="8c5a970", type="last_available_block"} 100127462
nodeos_p2p_connections{connid="8c5a970", type="accepting_blocks"} 1
nodeos_p2p_connections{connid="8c5a970", type="name", name="eosn-jungle4-trxrly28:9876 - 8c5a970"} 0
nodeos_p2p_connections{connid="8c5a970", type="last_bytes_received"} 1.695820203477813e+18
@matthewdarwin matthewdarwin changed the title Prometheus metrics for nodeos_p2p_connections don't use stable identifier Prometheus metrics for nodeos_p2p_connections don't use stable identifier in 5.0 Sep 27, 2023
@bhazzard
Copy link

See #1687 for more information.

@bhazzard bhazzard added bug Something isn't working 👍 lgtm and removed triage labels Sep 28, 2023
@bhazzard bhazzard added this to the Leap v5.0.0-stable milestone Sep 28, 2023
@matthewdarwin
Copy link
Author

Probably a better way to structure this is to create a unique counter for each metric instead of overloading labels

nodeos_p2p_bytes_sent{connid="8c5a970"} 41
nodeos_p2p_last_available_block{connid="8c5a970",} 100127462
nodeos_p2p_accepting_blocks{connid="8c5a970"} 1
nodeos_p2p_name{connid="8c5a970", name="eosn-jungle4-trxrly28:9876 - 8c5a970"} 0
nodeos_p2p_last_bytes_received{connid="8c5a970"} 1.695820203477813e+18

Presumably last_available_block is NOT a counter since block number can decrease? (after a fork?)

@bhazzard
Copy link

bhazzard commented Oct 2, 2023

Presumably last_available_block is NOT a counter since block number can decrease? (after a fork?)

Good point @matthewdarwin. @heifner Given this, does it make sense to change last_available_block to a gauge?

@heifner
Copy link
Member

heifner commented Oct 2, 2023

Presumably last_available_block is NOT a counter since block number can decrease? (after a fork?)

Good point @matthewdarwin. @heifner Given this, does it make sense to change last_available_block to a gauge?

yes.

@heifner heifner self-assigned this Oct 6, 2023
@heifner heifner added the OCI Work exclusive to OCI team label Oct 6, 2023
heifner added a commit that referenced this issue Oct 11, 2023
…. Add mapped_private database-map-mode since none of the tests currently use it.
heifner added a commit that referenced this issue Oct 11, 2023
@heifner heifner linked a pull request Oct 11, 2023 that will close this issue
heifner added a commit that referenced this issue Oct 11, 2023
heifner added a commit that referenced this issue Oct 12, 2023
[5.0] Prometheus: Add stable identifier for P2P connections
heifner added a commit that referenced this issue Oct 12, 2023
[5.0 -> main] Prometheus: Add stable identifier for P2P connections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 👍 lgtm OCI Work exclusive to OCI team
Projects
Status: Done
4 participants