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(metrics): add close group shunned metrics #2167

Merged
merged 4 commits into from
Oct 6, 2024

Conversation

RolandSherwin
Copy link
Member

  • adds two new metrics to /metrics_extended endpoint based on the shunned_count:
    • shunned_by_close_group - The number of close group peers that have shunned our node
    • shunned_by_old_close_group - The number of close group peers that have shunned our node. This contains the peers that were once in our close group but have since been evicted.

println!("Metrics server on http://{}/metrics", server.local_addr());

info!("Metrics server on http://{} Available endpoints: /metrics, /metrics_extended, /metadata", server.local_addr());

Check warning

Code scanning / devskim

An HTTP-based URL without TLS was detected. Warning

Insecure URL
@RolandSherwin RolandSherwin force-pushed the metrics_shunned_close branch from 82a7a79 to ddfd4c5 Compare October 1, 2024 13:17
sn_networking/src/event/mod.rs Show resolved Hide resolved
sn_networking/src/metrics/bad_node.rs Outdated Show resolved Hide resolved
sn_networking/src/metrics/bad_node.rs Outdated Show resolved Hide resolved
@RolandSherwin RolandSherwin force-pushed the metrics_shunned_close branch 2 times, most recently from 0b47ce7 to 3329dc8 Compare October 4, 2024 13:32
@RolandSherwin RolandSherwin force-pushed the metrics_shunned_close branch from 3329dc8 to f050a0e Compare October 4, 2024 13:33
self.metric_current_group.inc();
self.close_group_peers_that_have_shunned_us.insert(peer);
self.old_new_group_shunned_list.insert(peer);
Copy link
Member

@maqi maqi Oct 4, 2024

Choose a reason for hiding this comment

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

this can be outside of the inner if clause but within the outer if clause
so that only one code line of insertion will be enough.

of you can even do if self.old_new_group_shunned_list.insert(peer).is_none() { ... } to avoid separated check then insert

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 can be outside of the inner if clause but within the outer if clause
so that only one code line of insertion will be enough.

Do you mean within this clause? !self.old_new_group_shunned_list.contains(&peer)
But that would contain ALL peers that are observed by us. I only want the ones that are in the close group (either old / new).

@maqi maqi enabled auto-merge October 4, 2024 19:36
@maqi maqi added this pull request to the merge queue Oct 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 4, 2024
@RolandSherwin RolandSherwin added this pull request to the merge queue Oct 6, 2024
Merged via the queue into maidsafe:main with commit c717713 Oct 6, 2024
41 checks passed
@RolandSherwin RolandSherwin deleted the metrics_shunned_close branch October 6, 2024 13:39
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.

2 participants