Add cluster connections list#28157
Conversation
graham-rp
requested review from
a team,
kbatuigas and
r-vasquez
as code owners
Retry command for Build#74729please wait until all jobs are finished before running the slash command |
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
2 times, most recently
from
a047685 to
5009dd7
Compare
Retry command for Build#74793please wait until all jobs are finished before running the slash command |
| // TODO: add guardrails and define a proper field mapping | ||
| fset.StringVar(&orderBy, "order-by", "", "Order the results by their values. See Examples above") | ||
|
|
||
| // TODO: establish a limit | ||
| fset.Int32Var(&limit, "limit", defaultPageSize, "Limit how many records can be returned") |
There was a problem hiding this comment.
What's left so we can finish these TODOs?
There was a problem hiding this comment.
The second one is scale testing from core, but the first is more involved. We'd have to figure out how to map things from our end back to the proto labels, and then document it in a way that makes sense to users. I think that's realistically a new ticket.
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
from
5009dd7 to
e1de2d8
Compare
| var tableHeaders = []string{ | ||
| "NODE", | ||
| "SHARD", | ||
| "UID", | ||
| "CLIENT-ID", | ||
| "IP:PORT", | ||
| "OPEN-TIME", | ||
| "IDLE", | ||
| "PROD-TPUT", | ||
| "FETCH-TPUT", | ||
| "REQS/MIN", | ||
| "STATE", | ||
| } |
There was a problem hiding this comment.
Let's add a USER column for sure, and I did a bit of opinionated reordering of the columns here:
| var tableHeaders = []string{ | |
| "NODE", | |
| "SHARD", | |
| "UID", | |
| "CLIENT-ID", | |
| "IP:PORT", | |
| "OPEN-TIME", | |
| "IDLE", | |
| "PROD-TPUT", | |
| "FETCH-TPUT", | |
| "REQS/MIN", | |
| "STATE", | |
| } | |
| var tableHeaders = []string{ | |
| "UID", | |
| "STATE", | |
| "USER" | |
| "CLIENT-ID", | |
| "IP:PORT", | |
| "NODE", | |
| "SHARD", | |
| "OPEN-TIME", | |
| "IDLE", | |
| "PROD-TPUT", | |
| "FETCH-TPUT", | |
| "REQS/MIN", | |
| } |
Then the order of importance for more fields is this IMO:
"AVG BATCH",
"GROUP-ID",
"AUTH-MECHANISM",
"LISTENER",
"TRANSACTIONAL-ID",
"GROUP-INSTANCE-ID",
"GROUP-MEMBER-ID",
Ideally I would add basically all of the above data, but I guess it would quickly become too wide and wrap around. I think you mentioned at some point that we could provide a flexible set of columns here. I think that could be super valuable. But that could definitely be a follow up.
There was a problem hiding this comment.
Sounds good - I've got ~150 chars on the table as is after adding the user, so I don't think I want to add any more just yet (they're available with --format=json). Ideally we'd have a --columns=... or a more interactive TUI output
There was a problem hiding this comment.
We also have --format=wide that I think is currently the same as --format=text. Should we add more columns when --format=wide and in that case accept that the text might wrap around if you don't have enough screen space?
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
4 times, most recently
from
da011ee to
93fa3fe
Compare
graham-rp
left a comment
graham-rp
left a comment
There was a problem hiding this comment.
update based on feedback
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
3 times, most recently
from
2f6a679 to
9e2d68c
Compare
| var tableHeaders = []string{ | ||
| "NODE", | ||
| "SHARD", | ||
| "UID", | ||
| "CLIENT-ID", | ||
| "IP:PORT", | ||
| "OPEN-TIME", | ||
| "IDLE", | ||
| "PROD-TPUT", | ||
| "FETCH-TPUT", | ||
| "REQS/MIN", | ||
| "STATE", | ||
| } |
There was a problem hiding this comment.
We also have --format=wide that I think is currently the same as --format=text. Should we add more columns when --format=wide and in that case accept that the text might wrap around if you don't have enough screen space?
| fset.StringVar(&filterRaw, "filter-raw", "", "Filter connections based on a raw query (overrides other filters)") | ||
|
|
||
| // TODO: add guardrails and define a proper field mapping | ||
| fset.StringVar(&orderBy, "order-by", "", "Order the results by their values. See Examples above") |
There was a problem hiding this comment.
What do you think about defining a set of shorthands for ordering as well, similar to how filtering works? I think these would make sense:
--top-producers # order by produce throughput desc
--top-consumers # order by fetch throughput desc
--most-idle # order by idle_duration desc
--least-idle # order by idle_duration asc
--oldest # order by open_time asc
--newest # order by open_time desc
There was a problem hiding this comment.
eventually that probably makes sense - for the time being I think the first thing is documenting a mapping between the API vals and what adminv2 is expecting
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
2 times, most recently
from
00f4b9a to
ab83506
Compare
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
3 times, most recently
from
23d1295 to
83b0ca7
Compare
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
from
c0bc030 to
c8a968e
Compare
Retry command for Build#75054please wait until all jobs are finished before running the slash command |
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
from
c8a968e to
a234d3b
Compare
graham-rp
force-pushed
the
UX-358/add-list-connections
branch
from
a234d3b to
c8a968e
Compare
r-vasquez
force-pushed
the
UX-358/add-list-connections
branch
from
c8a968e to
41f9426
Compare
This adds a basic `cluster connections list` command that uses the adminv2 API to list open and recently closed client connections inside the cluster. Co-authored-by: Gellért Peresztegi-Nagy <pereszteginagy.gellert@gmail.com>
r-vasquez
force-pushed
the
UX-358/add-list-connections
branch
from
41f9426 to
38230d0
Compare
Retry command for Build#75073please wait until all jobs are finished before running the slash command |
4 participants
This adds a basic
cluster connections listcommand that uses the adminv2 API to list open and recently closed client connections inside th cluster.Example output
rpk cluster connection list --helprpk cluster connection listrpk cluster connection list --client-id=rpk --order-by "state, open_time asc"Backports Required
Release Notes
Features
cluster connections listcommand