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

[5.0] Prometheus: Ensure valid unique_conn_node_id #1879

Merged
merged 2 commits into from
Nov 8, 2023

Conversation

heifner
Copy link
Member

@heifner heifner commented Nov 8, 2023

Make sure a valid and unique connection id is provided to prometheus. If not connected, then use a temporary id until the connection is established and a handshake is received.

Resolves #1871

@heifner heifner changed the base branch from main to release/5.0 November 8, 2023 18:17
@heifner heifner added the OCI Work exclusive to OCI team label Nov 8, 2023
@matthewdarwin
Copy link

Is the temporary connection id semi-permanent (eg it is not changing every 30 seconds after re-connect attempt)? If there are multiple temporary connections does each one get a unique name?

c->unique_conn_node_id = fc::sha256::hash(c->p2p_address).str().substr(0, 7);
} else if (!c->remote_endpoint_ip.empty()) {
c->unique_conn_node_id = fc::sha256::hash(c->remote_endpoint_ip).str().substr(0, 7);
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is c->connection_id always not empty in this else condition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It is assigned at connection object construction.

@heifner
Copy link
Member Author

heifner commented Nov 8, 2023

Is the temporary connection id semi-permanent (eg it is not changing every 30 seconds after re-connect attempt)? If there are multiple temporary connections does each one get a unique name?

Yes. It is even permanent on re-start of nodeos. It will change when connection/handshake complete assuming a connection/handshake ever happens.

tests/p2p_sync_throttle_test.py Show resolved Hide resolved
@@ -181,7 +181,7 @@ def extractPrometheusMetric(connID: str, metric: str, text: str):
errorLimit -= 1
continue
Print('Throttled Node Start State')
throttledNodePortMap = {port: id for id, port in connPorts if id != ''}
throttledNodePortMap = {port: id for id, port in connPorts if port != '0'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should 9877 be compared as above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 9877 so that is not needed.

plugins/net_plugin/net_plugin.cpp Show resolved Hide resolved
@heifner heifner merged commit 88eb2fe into release/5.0 Nov 8, 2023
29 checks passed
@heifner heifner deleted the GH-1871-prometheus-5.0 branch November 8, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5.0 prometheus exporter shows stats without a connection id
4 participants