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: peers table enhancements #1616

Merged
merged 19 commits into from
Mar 29, 2021
Merged

feat: peers table enhancements #1616

merged 19 commits into from
Mar 29, 2021

Conversation

jessicaschilling
Copy link
Contributor

@jessicaschilling jessicaschilling commented Sep 8, 2020

Closes #1602

Summary

This PR stubs out but doesn't finish #1602. All elements are on the page, with dummy values and visual styling as outlined in #1602, but the following still need to be wired up:

"In/Out" column:

"Peer ID" column:

  • Make Peer IDs clickable, with an action of copying that Peer ID to clipboard
  • Upon copy, feedback using green snackbar: "Peer ID copied to clipboard."

"Connection" column:

  • Make row content clickable, with an action of copying that multiaddr to clipboard
  • Upon copy, feedback using green snackbar: "Connection address copied to clipboard."
  • Append direction to the hover text as such: /multi/addr (outbound) (note this doesn't appear if using js-ipfs)

"Agent" column:

  • Real value for agent in peer-locations.js based on AgentVersion (via ipfs id <PeerId>)
  • Sortability (A-Z should be fine here)
  • Include hover state indicating what type of activity occurred, i.e. DHT, Bitswap (note this doesn't appear if using js-ipfs)

Screenshot

image

@jessicaschilling
Copy link
Contributor Author

@lidel and/or @rafaelramalho19 -- I realize it's a stretch to get this into 2.11, but wanted to at least stub out something that would be easier to wire up in 2.12. Happy to plug away at any guidance you might want to give yourselves if that's easier than just asking you to do it. 😊

@lidel
Copy link
Member

lidel commented Oct 13, 2020

Sidenote: when we ship these improvements we should also update geoip database (idea how to do this faster: ipfs-shipyard/ipfs-geoip#68 (comment)), because the moment we bring people's focus to the Peers screen, they will notice faster-than-lings pings ;)
It does not need to be part of this PR, but should land in the same release for the biggest "improvement" impact.

@lidel
Copy link
Member

lidel commented Oct 30, 2020

Wired up direction and streams + added geoip update that already landed in master:

2020-10-30--18-31-53
2020-10-30--18-30-57

Reading bandwidth stats and agent ids requires two additional HTTP requests per every peer, so we need to ensure it happens only for visible ones.

Will wire up copying first.

@lidel lidel changed the title Feat/peers table enhancements feat: peers table enhancements Oct 30, 2020
@lidel
Copy link
Member

lidel commented Nov 2, 2020

Wired up copying:

I realized showing the green snackbar was not the best UI for providing feedback, especially when details behind each cell can be copied separately.

After some trial&error I implemented something different, a form of in-place confirmation:

copying-ui

IMO its less noisy, and more intuitive for this specific UI, but lmk your thoughts @jessicaschilling

@jessicaschilling
Copy link
Contributor Author

@lidel - great idea!

@jessicaschilling
Copy link
Contributor Author

@lidel One quick thought - could we shorten the dwelltime of the phrase "Copied!"? 2sec should be enough.

@lidel lidel added the P2 Medium: Good to have, but can wait until someone steps up label Feb 25, 2021
@lidel
Copy link
Member

lidel commented Mar 15, 2021

Quick note on unblocking 80% of this PR:

  • comment out IN/OUT and AGENT columns for now + fill issue to follow-up:
    • getting this data is too expensive atm, some additional caching would need to happen
    • mixing active and totals may be too confusing
  • restore NOTES column in place of AGENT one
  • make sure geoip works

Note: a better course of action may be to close this PR without merging, and open a new one, porting only the minimum set of features that work.

@lidel lidel self-assigned this Mar 26, 2021
Sadly I was not able to make things performant enough:

- in/out adds 1 HTTP request PER peer
  (and cant be cached as it is dynamic)
- agent also adds one (but could be cached)

We need better API than HTTP, maybe RPC over websockets or pubsub.

To salvage this PR I replaced mentioned columns with open libp2p
streams, which does not require any additional requests, but is pretty
useful, gives us insight if peer was used only for kad DHT, bitswap,
pubsub etc.
@lidel lidel force-pushed the feat/peers-table-enhancements branch from d57d150 to 8510d39 Compare March 29, 2021 13:30
@lidel
Copy link
Member

lidel commented Mar 29, 2021

@jessicaschilling sadly I was not able to make things performant enough to run decently on low-power machines:

  • in/out adds 1 HTTP request PER peer
    (and cant be cached as it is dynamic)
  • agent also adds one (but could be cached)

We need better API than HTTP, maybe RPC over websockets or pubsub.

To salvage this PR I replaced mentioned columns with "open libp2p streams" column, which does not require any additional requests, but is pretty useful, gives us insight if peer was used only for kad DHT, bitswap, pubsub etc:

Screenshot_2021-03-29 Peers IPFS

Mind doing quick 👀?

@lidel lidel self-requested a review March 29, 2021 15:06
@jessicaschilling
Copy link
Contributor Author

@lidel Works for me. Two thoughts:

  • Is there any point in having all cell values in this table click-to-copy? For example, copying the latency value doesn't really seem to have a use case. Might be worth limiting this just to the Peer ID and Connection columns.
  • The 🤔 in the Open Streams column is charming, but could be more useful if the hover text replaced the 🤔 with what that actually means.
    Thanks!

@lidel
Copy link
Member

lidel commented Mar 29, 2021

Thanks! Should be ready:

  • Removed interaction from Latency column, I kept copying for everything else as I find them useful
  • Hover text for 🤔 is [unnamed]

@lidel lidel marked this pull request as ready for review March 29, 2021 18:06
@lidel lidel merged commit 4560981 into master Mar 29, 2021
@lidel lidel deleted the feat/peers-table-enhancements branch March 29, 2021 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Peers page table enhancements
2 participants