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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Add peer info api #4845

Closed
wants to merge 37 commits into from
Closed

[Merged by Bors] - Add peer info api #4845

wants to merge 37 commits into from

Conversation

piersy
Copy link
Contributor

@piersy piersy commented Aug 14, 2023 •
edited
Loading

Motivation

Closes #4778

Note that this is in a draft state because it needs spacemeshos/api#258 to be merged and I guess a release of the API package, before it can be merged. I plan to remove the replace directive for the api in the mod file when, the api has been released.

Changes

Adds a new streaming API endpoint to the node service to return info about connected peers.

Note this PR changes the PostProviderID field to be exported to allow for config to be easily serialised/deserialised in order to perform a deep copy.

Test Plan

Added a test, the test doesn't check whether tags are since that part would make things a bit trickier. However through manual checking I can see that tags are being assigned.

$ grpc_cli call  localhost:9092 spacemesh.v1.NodeService.PeerInfo ''
connecting to localhost:9092
peers {
  id: "12D3KooWRkZMjGNrQfRyeKQC9U58cUwAfyQMtjNsupixkBFag8AY"
  connections {
    address: "/ip4/34.150.215.241/tcp/5000"
    uptime: "29.348584085s"
    outbound: true
  }
  tags: "bootnode"
}
Rpc succeeded with OK status

DevOps Notes

  • This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
  • This PR does not affect public APIs
  • This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
  • This PR does not make changes to log messages (which monitoring infrastructure may rely on)

dshulyak
dshulyak reviewed Aug 15, 2023
View reviewed changes
activation/post.go Outdated Show resolved Hide resolved
node/util_test.go Outdated Show resolved Hide resolved
p2p/dhtdiscovery/discovery.go Outdated Show resolved Hide resolved
p2p/dhtdiscovery/discovery.go Outdated Show resolved Hide resolved
api/grpcserver/node_service.go Outdated Show resolved Hide resolved
api/grpcserver/node_service.go Outdated Show resolved Hide resolved
fasmat
fasmat reviewed Aug 15, 2023
View reviewed changes
node/util_test.go Outdated Show resolved Hide resolved
fasmat
fasmat reviewed Aug 15, 2023
View reviewed changes
node/util_test.go Outdated Show resolved Hide resolved
fasmat
fasmat reviewed Aug 15, 2023
View reviewed changes
node/util_test.go Outdated Show resolved Hide resolved
fasmat
fasmat reviewed Aug 15, 2023
View reviewed changes
node/util_test.go Outdated Show resolved Hide resolved
fasmat
fasmat reviewed Aug 15, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
fasmat
fasmat reviewed Aug 15, 2023
View reviewed changes
api/grpcserver/interface.go Outdated Show resolved Hide resolved
@piersy piersy marked this pull request as draft August 15, 2023 10:22
@codecov
Copy link

codecov bot commented Aug 15, 2023 •
edited
Loading

Codecov Report

Merging #4845 (d99bf92) into develop (b23bc52) will decrease coverage by 0.1%.
The diff coverage is 74.1%.

❗ Current head d99bf92 differs from pull request most recent head 1e8abb9. Consider uploading reports for the commit 1e8abb9 to get more accurate results

@@            Coverage Diff            @@
##           develop   #4845     +/-   ##
=========================================
- Coverage     76.8%   76.8%   -0.1%     
=========================================
  Files          261     261             
  Lines        30160   30234     +74     
=========================================
+ Hits         23189   23229     +40     
- Misses        5476    5505     +29     
- Partials      1495    1500      +5
Files Changed Coverage Δ
p2p/peer.go 100.0% <ø> (ø)
p2p/host.go 38.8% <64.7%> (+1.0%) ⬆️
p2p/upgrade.go 66.4% <75.0%> (+2.2%) ⬆️
api/grpcserver/admin_service.go 66.6% <76.6%> (+3.3%) ⬆️
api/grpcserver/grpc.go 60.6% <100.0%> (-26.9%) ⬇️
node/node.go 64.8% <100.0%> (-0.1%) ⬇️

... and 7 files with indirect coverage changes

fasmat
fasmat reviewed Aug 16, 2023
View reviewed changes
node/node.go Outdated Show resolved Hide resolved
@piersy piersy requested review from dshulyak and fasmat August 16, 2023 14:12
fasmat
fasmat reviewed Aug 16, 2023
View reviewed changes
node/util_test.go Outdated Show resolved Hide resolved
node/util_test.go Outdated Show resolved Hide resolved
@piersy piersy marked this pull request as ready for review August 17, 2023 08:45
dshulyak
dshulyak approved these changes Aug 21, 2023
View reviewed changes
api/grpcserver/admin_service.go Outdated Show resolved Hide resolved
@piersy
Copy link
Contributor Author

piersy commented Aug 21, 2023

bors merge

bors bot pushed a commit that referenced this pull request Aug 21, 2023
@piersy
Add peer info api (#4845)
64bccb6 
## Motivation
Closes #4778

Note that this is in a draft state because it needs spacemeshos/api#258 to be merged and I guess a release of the API package, before it can be merged. I plan to remove the replace directive for the api in the mod file when, the api has been released.

## Changes
Adds a new streaming API endpoint to the node service to return info about connected peers.

Note this PR changes the PostProviderID field to be exported to allow for config to be easily serialised/deserialised in order to perform a deep copy.

## Test Plan
Added a test, the test doesn't check whether tags are since that part would make things a bit trickier. However through manual checking I can see that tags are being assigned.

```shell
$ grpc_cli call  localhost:9092 spacemesh.v1.NodeService.PeerInfo ''
connecting to localhost:9092
peers {
  id: "12D3KooWRkZMjGNrQfRyeKQC9U58cUwAfyQMtjNsupixkBFag8AY"
  connections {
    address: "/ip4/34.150.215.241/tcp/5000"
    uptime: "29.348584085s"
    outbound: true
  }
  tags: "bootnode"
}
Rpc succeeded with OK status
```

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
@bors
Copy link

bors bot commented Aug 21, 2023

Pull request successfully merged into develop.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Add peer info api [Merged by Bors] - Add peer info api Aug 21, 2023
@bors bors bot closed this Aug 21, 2023
@bors bors bot deleted the piersy/add-peer-info-api branch August 21, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fasmat fasmat fasmat approved these changes

@dshulyak dshulyak dshulyak approved these changes

@countvonzero countvonzero Awaiting requested review from countvonzero

@poszu poszu Awaiting requested review from poszu poszu is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

p2p, api: add api to return information about your peers
3 participants
@piersy @dshulyak @fasmat