This repository was archived by the owner on Feb 12, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: implement ipfs ping
flags #928
#1202
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
'use strict' | ||
|
||
const print = require('../utils').print | ||
|
||
module.exports = { | ||
command: 'ping <peerId>', | ||
|
||
describe: 'Measure the latency of a connection', | ||
|
||
builder: { | ||
count: { | ||
alias: 'n', | ||
type: 'integer', | ||
default: 10 | ||
} | ||
}, | ||
|
||
handler (argv) { | ||
const peerId = argv.peerId | ||
const count = argv.count || 10 | ||
|
||
print('PING ' + peerId) | ||
|
||
let noOfTimes = 0 | ||
let totalTime = 0 | ||
|
||
const pingCb = (err, p) => { | ||
if (err) { | ||
throw err | ||
} | ||
let time = p.Time | ||
totalTime = totalTime + time | ||
noOfTimes = noOfTimes + 1 | ||
print('Pong received: time=' + time + ' ms') | ||
if (noOfTimes === count) { | ||
print('Average latency: ' + totalTime / count + 'ms') | ||
} | ||
} | ||
|
||
for (let i = 0; i < count; i++) { | ||
argv.ipfs.ping(peerId, pingCb) | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,35 @@ | ||
'use strict' | ||
|
||
const promisify = require('promisify-es6') | ||
const OFFLINE_ERROR = require('../utils').OFFLINE_ERROR | ||
const PeerId = require('peer-id') | ||
const PeerInfo = require('peer-info') | ||
var Readable = require('stream').Readable | ||
|
||
module.exports = function ping (self) { | ||
return promisify((callback) => { | ||
callback(new Error('Not implemented')) | ||
return promisify((peerId, cb) => { | ||
if (!self.isOnline()) { | ||
return cb(new Error(OFFLINE_ERROR)) | ||
} | ||
|
||
var outputStream = new Readable() | ||
outputStream._read = function (size) { | ||
} | ||
|
||
let peer | ||
try { | ||
peer = self._libp2pNode.peerBook.get(peerId) | ||
} catch (err) { | ||
peer = new PeerInfo(PeerId.createFromB58String(peerId)) | ||
} | ||
|
||
self._libp2pNode.ping(peer, (err, p) => { | ||
p.once('ping', (time) => { | ||
outputStream.push(JSON.stringify([{}, { Success: true, Time: time }, { Text: 'Average latency: ' + time + ' ms' }])) | ||
outputStream.push(null) | ||
p.stop() | ||
cb(err, outputStream) | ||
}) | ||
}) | ||
}) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
'use strict' | ||
|
||
const boom = require('boom') | ||
|
||
exports = module.exports | ||
|
||
exports.get = (request, reply) => { | ||
const ipfs = request.server.app.ipfs | ||
const peerId = request.query.arg | ||
|
||
ipfs.ping(peerId, (err, outputStream) => { | ||
if (err) { | ||
return reply(boom.badRequest(err)) | ||
} | ||
|
||
return reply(outputStream).type('application/json').header('x-chunked-output', '1') | ||
}) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
'use strict' | ||
|
||
const resources = require('./../resources') | ||
|
||
module.exports = (server) => { | ||
const api = server.select('API') | ||
|
||
api.route({ | ||
method: '*', | ||
path: '/api/v0/ping', | ||
config: { | ||
payload: { | ||
parse: false, | ||
output: 'stream' | ||
}, | ||
handler: resources.ping.get | ||
} | ||
}) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A ping with count 10 is a ping with 10 packets, not 10 ping calls. You need to count the number of pings that come in p.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that 10 ping calls is wrong. I started refactoring the code and was able to use the ping object and stream the times to the api.
But I have run into a problem. In js-ipfs-api the response assumes there will be one ping request.
So even if I stream multiple ping responses only the first one is returned.
My understanding of the flow when someone types jsipfs ping is
The problem is js-ipfs-api is expecting only one response ping object. Is there a workaround I'm missing or should I create an issue inn js-ipfs-api ping and fix that. I just want to confirm whether you its an issue or not. :)
Also, irrespective of the api I was able to get the curl outputs for both the go-ipfs http api and js-ipfs http api to be the same . I have attached images for the same. But again I find one inconsistency . Since we are using new Date() in js-libp2p-ping we are not getting the same unit as go-ipfs. go-ipfs returns in nanosecond and js-ipfs returns the time in milliseconds. Let me know whether I should create a issue and fix that. I would be glad to do it. I just want to confirm whether its an issue or not.
Curl response to js-ipfs

Curl response to go-ipfs

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like it needs to be fixed in js-ipfs-api then. Just go for it too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that @ngotchac noticed a misbehavior a while ago too ipfs-inactive/js-ipfs-http-client#556