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(p2p): add native libp2p debug metrics #1795

Merged
merged 22 commits into from
Apr 3, 2023

Conversation

derrandz
Copy link
Contributor

@derrandz derrandz commented Feb 21, 2023

Overview

This PR introduces libp2p metrics. The metrics that are introduced in this PR are plentiful, and it's sufficient to list the libp2p components they cover:

  1. Resource Manager
  2. AutoNat
  3. Identify
  4. EventBus
  5. Swarm

(check the following link)

Usage

Run your node with flag:

$ celestia light start ... --metrics --metrics.endpoint ... --p2p.metrics

Results

Dashboards:

Screen Shot 2023-02-28 at 00 29 58

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@derrandz derrandz marked this pull request as draft February 21, 2023 16:13
@derrandz derrandz changed the title feat: add p2p bandwidth metrics using opentelemetry primitives feat(p2p): add p2p bandwidth metrics using opentelemetry primitives + native libp2p debug metrics Feb 27, 2023
@derrandz derrandz marked this pull request as ready for review February 27, 2023 23:30
@derrandz derrandz requested a review from walldiss as a code owner February 27, 2023 23:30
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Initial quick look

nodebuilder/settings.go Outdated Show resolved Hide resolved
nodebuilder/p2p/module.go Outdated Show resolved Hide resolved
nodebuilder/p2p/module.go Outdated Show resolved Hide resolved
@derrandz derrandz requested a review from Wondertan March 17, 2023 15:07
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2023

Codecov Report

Merging #1795 (c9471d3) into main (f18b1ab) will decrease coverage by 0.33%.
The diff coverage is 14.96%.

@@            Coverage Diff             @@
##             main    #1795      +/-   ##
==========================================
- Coverage   54.95%   54.63%   -0.33%     
==========================================
  Files         216      216              
  Lines       13925    14009      +84     
==========================================
+ Hits         7653     7654       +1     
- Misses       5460     5537      +77     
- Partials      812      818       +6     
Impacted Files Coverage Δ
cmd/celestia/main.go 47.05% <ø> (ø)
nodebuilder/node/admin.go 26.66% <0.00%> (ø)
nodebuilder/settings.go 75.00% <0.00%> (-7.36%) ⬇️
cmd/celestia/rpc.go 7.03% <7.03%> (ø)
nodebuilder/p2p/metrics.go 11.42% <11.42%> (ø)
share/eds/eds.go 67.19% <40.00%> (-1.87%) ⬇️
cmd/flags_misc.go 48.40% <46.66%> (-0.19%) ⬇️
api/rpc/client/client.go 72.72% <100.00%> (-9.63%) ⬇️
core/testing.go 91.86% <100.00%> (+0.09%) ⬆️
nodebuilder/p2p/config.go 81.08% <100.00%> (+0.52%) ⬆️
... and 2 more

... and 6 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@walldiss walldiss left a comment

Choose a reason for hiding this comment

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

Could you please elaborate what is ERROR level metrics and how it would be used? It feel counterintuitive that ERROR level is below DEBUG, when in logging it is the highest.

cmd/flags_misc.go Outdated Show resolved Hide resolved
cmd/flags_misc.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
@derrandz derrandz requested a review from walldiss March 30, 2023 15:33
@derrandz derrandz changed the title feat(p2p): add p2p bandwidth metrics using opentelemetry primitives + native libp2p debug metrics feat(p2p): add native libp2p debug metrics Mar 30, 2023
@derrandz derrandz added the kind:feat Attached to feature PRs label Mar 30, 2023
cmd/flags_misc.go Outdated Show resolved Hide resolved
@derrandz derrandz requested a review from distractedm1nd March 30, 2023 15:59
Wondertan
Wondertan previously approved these changes Mar 30, 2023
Copy link
Member

@Wondertan Wondertan 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

cmd/flags_misc.go Outdated Show resolved Hide resolved
nodebuilder/p2p/config.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/settings.go Outdated Show resolved Hide resolved
nodebuilder/settings.go Show resolved Hide resolved
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

There is a new example for metrics usage in libp2p libp2p/go-libp2p#2232
Let's double-check it to ensure we didn't miss anything.

cmd/flags_misc.go Outdated Show resolved Hide resolved
cmd/flags_misc.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
@derrandz derrandz requested review from renaynay and Wondertan April 3, 2023 16:37
@derrandz
Copy link
Contributor Author

derrandz commented Apr 3, 2023

@Wondertan I've checked on libp2p/go-libp2p#2232 and nothing seems to be new. We should be good.

Wondertan
Wondertan previously approved these changes Apr 3, 2023
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

small nits, should be quick 🙏🏻

cmd/flags_misc.go Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
nodebuilder/p2p/metrics.go Outdated Show resolved Hide resolved
@derrandz derrandz enabled auto-merge (squash) April 3, 2023 17:54
@derrandz derrandz disabled auto-merge April 3, 2023 17:54
@derrandz derrandz enabled auto-merge (squash) April 3, 2023 17:55
@derrandz derrandz dismissed stale reviews from distractedm1nd and walldiss April 3, 2023 17:57

addressed

@derrandz derrandz merged commit f5f5efb into celestiaorg:main Apr 3, 2023
@renaynay renaynay added the kind:break! Attached to breaking PRs label Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Related to measuring/collecting node metrics kind:break! Attached to breaking PRs kind:feat Attached to feature PRs
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants