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(lightnode,libp2p): add prometheus metrics #1979

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

mrekucci
Copy link
Contributor

@mrekucci mrekucci commented Jun 3, 2021

Adds the following metrics:

  • number of currently connected peers
  • number of currently disconnected peers
  • total number of kicked out peers

This change is Reviewable

@mrekucci mrekucci added the ready for review The PR is ready to be reviewed label Jun 3, 2021
@mrekucci mrekucci force-pushed the lightnodes-prometheus-metrics branch 2 times, most recently from 2671613 to eb7f40e Compare June 3, 2021 17:20
Copy link
Member

@acud acud left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aloknerurkar, @anatollupacescu, and @mrekucci)


pkg/topology/lightnode/container.go, line 63 at r1 (raw file):

func (c *Container) KickOut(_ p2p.Peer) {
	c.metrics.TotalKickedPeers.Inc()

this metric can be inside libp2p, it doesn't really matter to have it on the light node container. also, if the peer info is not used, why do we need it?

@mrekucci mrekucci force-pushed the lightnodes-prometheus-metrics branch from eb7f40e to 2a8c128 Compare June 4, 2021 07:27
@mrekucci mrekucci requested a review from acud June 4, 2021 07:28
Copy link
Contributor Author

@mrekucci mrekucci left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on @acud, @aloknerurkar, and @anatollupacescu)


pkg/topology/lightnode/container.go, line 63 at r1 (raw file):

Previously, acud (acud) wrote…

this metric can be inside libp2p, it doesn't really matter to have it on the light node container. also, if the peer info is not used, why do we need it?

Done.

@mrekucci mrekucci changed the title feat(lightnode): add prometheus metrics feat(lightnode,libp2p): add prometheus metrics Jun 4, 2021
@mrekucci mrekucci force-pushed the lightnodes-prometheus-metrics branch from 2a8c128 to 7a87cd7 Compare June 4, 2021 08:50
Copy link
Member

@acud acud left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aloknerurkar)

@mrekucci mrekucci merged commit 1169c8c into master Jun 4, 2021
@mrekucci mrekucci deleted the lightnodes-prometheus-metrics branch June 4, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull-request ready for review The PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants