Skip to content

Comments

chore: add client version in metrics#670

Merged
mergify[bot] merged 7 commits intosigp:release-v1.0.0from
petarjuki7:add_version_in_metrics
Oct 10, 2025
Merged

chore: add client version in metrics#670
mergify[bot] merged 7 commits intosigp:release-v1.0.0from
petarjuki7:add_version_in_metrics

Conversation

@petarjuki7
Copy link
Member

@petarjuki7 petarjuki7 commented Oct 8, 2025

Issue Addressed

Addresses Issue #652

Proposed Changes

No longer just save Enr in PeerStore, but a new PeerInfo struct which lets us hold its Enr and ClientType (Anchor or go-ssv)
Then, add metric to count how many go-ssv vs Anchor nodes are connected to us.

Additional Info

I expose two fields in the PeerManager struct as public (peer_store and connection_manager).

@petarjuki7 petarjuki7 self-assigned this Oct 8, 2025
@petarjuki7 petarjuki7 changed the title feat(metrics): add client version in metrics chore: add client version in metrics Oct 8, 2025
@AgeManning
Copy link
Member

This is good.

I think with #652 I was hoping to add a single endpoint so that we know the version of our current running anchor node so we can display on our dash. Would be nice to add that in too.

Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

Nice!

@petarjuki7 petarjuki7 marked this pull request as ready for review October 9, 2025 14:11
@petarjuki7 petarjuki7 requested a review from dknopik October 9, 2025 14:11
@petarjuki7 petarjuki7 added metrics v1.0.0 First Mainnet-release ready-for-review This PR is ready to be reviewed labels Oct 9, 2025
Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

Two nitpicks:

@mergify
Copy link

mergify bot commented Oct 10, 2025

This pull request has merge conflicts. Could you please resolve them @petarjuki7? 🙏

@mergify mergify bot added waiting-on-author and removed ready-for-review This PR is ready to be reviewed labels Oct 10, 2025
@mergify mergify bot added ready-for-review This PR is ready to be reviewed and removed waiting-on-author labels Oct 10, 2025
@petarjuki7 petarjuki7 requested a review from dknopik October 10, 2025 08:24
Comment on lines 588 to 634
let client_type = ClientType::from(metadata.node_version.clone());

// Update client type in peer store
if let Some(peer_info) = store_ref.get_custom_data_mut(&peer_id) {
peer_info.set_client_type(client_type);
} else {
// If no peer info yet, create new peer info
store_ref.insert_custom_data(
&peer_id,
PeerInfo {
enr: None,
client_type: Some(client_type),
},
);
}

// Record subnet match count
if let Ok(gauge_vec) = crate::metrics::HANDSHAKE_SUBNET_MATCHES.as_ref() {
let label = &matching_count.to_string();
if let Ok(gauge) = gauge_vec.get_metric_with_label_values(&[label]) {
gauge.inc();
// Count and record matching subnets
if let Some(our_metadata) = &self.node_info.metadata {
let matching_count =
count_matching_subnets(&our_metadata.subnets, &metadata.subnets);

debug!(
%peer_id,
our_subnets = %our_metadata.subnets,
their_subnets = %metadata.subnets,
matching_subnets = matching_count,
"Handshake completed"
);

// Record subnet match count
if let Ok(gauge_vec) = crate::metrics::HANDSHAKE_SUBNET_MATCHES.as_ref() {
let label = &matching_count.to_string();
if let Ok(gauge) = gauge_vec.get_metric_with_label_values(&[label]) {
gauge.inc();
}
}
} else {
debug!(%peer_id, "Handshake completed");
}

// Trigger metric recalculation
let behaviour = self.swarm.behaviour();
let store_ref = behaviour.peer_manager.peer_store.store();
behaviour
.peer_manager
.connection_manager
.update_metrics_if_changed(true, store_ref);
Copy link
Member

Choose a reason for hiding this comment

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

Actually, what do you think about moving everything inside this if into the peer manager and/or the connection manager within it? This would allow us to avoid making the peer_store and conncection_manager pub, for better encapsulation

Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@dknopik dknopik added ready-for-merge and removed ready-for-review This PR is ready to be reviewed labels Oct 10, 2025
@mergify mergify bot merged commit ebee887 into sigp:release-v1.0.0 Oct 10, 2025
15 checks passed
petarjuki7 added a commit to petarjuki7/anchor that referenced this pull request Oct 16, 2025
Addresses Issue sigp#652


  No longer just save `Enr` in `PeerStore`, but a new `PeerInfo` struct which lets us hold its `Enr` and `ClientType` (`Anchor` or `go-ssv`)
Then, add metric to count how many `go-ssv` vs `Anchor` nodes are connected to us.


Co-Authored-By: petarjuki7 <petar.jukic7@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants