proto: initial definition of debug bundle api #1444
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / push-module (pull_request).
|
bojand
force-pushed
the
proto/debug_bundle_api_initial
branch
2 times, most recently
from
90acf2c to
d0eb4b0
Compare
proto: use timestamps for logs since and until improve comments improve comments and validation add broker id path
bojand
force-pushed
the
proto/debug_bundle_api_initial
branch
from
d0eb4b0 to
f975d49
Compare
|
|
||
| package redpanda.api.console.v1alpha1; | ||
|
|
||
| message CreateDebugBundleRequest { |
There was a problem hiding this comment.
where does auth come into play?
There was a problem hiding this comment.
I don't think we need to expose that to the frontend or UX? The backend will set the parameters in the actual Admin API call based on the Admin API settings within console itself.
There was a problem hiding this comment.
Right now, console must be configured to use a SASL/SCRAM user that is a super user right? What if in the future it no longer has that power? Maybe it can be an optional field and if not provided will use console's creds?
I guess for now it's ok, but in the future we may need to include some level of authn options
There was a problem hiding this comment.
I have added auth parameter in the message.
| int32 cpu_profiler_wait_seconds = 3 [(buf.validate.field).int32 = { | ||
| gte: 15 | ||
| lte: 600 | ||
| }]; |
There was a problem hiding this comment.
quesiton: where did these limits come from?
There was a problem hiding this comment.
The minimum comes from from rpk debug bundle --help
--cpu-profiler-wait duration For how long to collect samples for the CPU profiler (e.g. 30s, 1.5m). Must be higher than 15s (default 30s)Regarding the max... I am not sure I thought I read somewhere or it was said somewhere that max would be 10m or maybe that just seemed like a reasonable max value. But I can't find it now... So I have removed the lte validation.
| // Optional. | ||
| repeated int32 partition_ids = 8; |
There was a problem hiding this comment.
the partition option is of the format:
{namespace/}topic/{partition ids} where namespace is optional and will be replaced with kafka if not provided, and parition ids is comma separated numbers so:
kafka/foo/1,2,3. also there can be multiple of those so
['kafka/foo/1,2,3', 'private/baz/3.4.5']
There was a problem hiding this comment.
Hmm that's good to know... I was assuming it was just the integer partition IDs.
Changed to a repeated string value and added comments.
There was a problem hiding this comment.
Yeah sorry I named this field after the cli option from rpk debug bundle:
-p, --partition stringArray Comma-separated partition IDs; when provided, rpk saves extra admin API requests for those partitions. Check help for extended usage
| // Additional API: | ||
| // GET /api/debug_bundle/{file} | ||
| // GET /api/debug_bundle/{broker_id}/{file} | ||
| // This will download the debug bundle zip file |
| // OIDC Auth settings. | ||
| message OIDCAuth { | ||
| string token = 1; | ||
| } |
There was a problem hiding this comment.
fyi we can probably drop this - rpk doesn't currently support using oidc to authn to a redpanda cluster.
There was a problem hiding this comment.
I'm going to remove it from the API wiki (https://redpandadata.atlassian.net/wiki/spaces/CORE/pages/804225124/API+Design)
There was a problem hiding this comment.
Removed. I have left the overall authentication as oneof to allow easily adding other options in the future.
|
Looks good, the only outstanding question I have is here: #1444 (comment) |
No description provided.