feat/cmds: hide peers info default in bitswap stat

[overbool]

Fixed: #5763

License: MIT
Signed-off-by: Overbool [email protected]

[Stebalien]

In theory, I completely agree with this. Unfortunately, in practice, it changes a long-standing stable interface.

@whyrusleeping thoughts? At the moment, this command is kind of painful due to this issue.

[whyrusleeping]

I pretty much always pipe this command to head -n10. It would not hurt my feelings if this got changed.

That said, 'bitswap partners' are really supposed to just be peers we are 'actively' trading data with. 'every peer you are connected to' really shouldnt be in that list..

[whyrusleeping]

maybe for now, default verbose to true, that way we can use ipfs bitswap stat -v=false to get the trimmed output.

I'm also not against this change as is, but if we go forward with it, we should do a bit of due dilligence to see if anyone is parsing the output of this command and using it for something

[eingenito]

I'd suggest not changing the default. It seems monitoring/troubleshooting related and therefore the kind of thing that people might well call in a script (like to check the health of their cluster or something). And because it's a little bit arcane it seems like marginal benefit for a change that's hard to estimate the impact of.

[Stebalien]

And because it's a little bit arcane it seems like marginal benefit for a change that's hard to estimate the impact of.

So, as long as we don't change the json output, we should mitigate the damage (although yes, someone is probably parsing the text...).

Maybe a solution is to just fix this by only showing real bitswap peers. We can probably record a simple "active" refcount for each peer.

However, that will have to wait until the current bitswap refactor is done.

[eingenito]

So, as long as we don't change the json output, we should mitigate the damage (although yes, someone is probably parsing the text...).

👍 Interesting.

Maybe a solution is to just fix this by only showing real bitswap peers. We can probably record a simple "active" refcount for each peer.

Flip-flop'ing here but if we're comfortable changing the default behavior in the future, and it has little (no?) worth now, maybe it's actually better to merge this and we'll improve the bitswap peers listing when we get to it. I guess right now this is roughly equivalent to ipfs swarm peers.

I can imagine lots of per-peer bitswap stats that might be neat but that would definitely change the json.

[Stebalien]

I've done some soul searching on this and, really, I don't think anyone will complain about this change. We're changing what is effectively a debugging interface (which not even the webui uses.

Everyone here seems quite happy because ipfs bitswap stat currently sucks so, (yolo).