-
Notifications
You must be signed in to change notification settings - Fork 819
fix: make pod_name and container_name optional in LoggingContext; add ContainerIdentifier to TailLogsResponse #7341
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.
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
528c10e
f62e8de
e115523
62c161c
799e1b9
c28e11b
37455e2
bb6141a
b127084
969bbb5
ff91693
c216761
0bf459c
6c7c456
c45e2e7
42d8c94
03ce2e7
03bd31b
fb3990a
ff1ef8a
f606cfc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,8 +203,15 @@ message TailLogsRequest { | |
| // The attempt number. | ||
| uint32 attempt = 2 [(buf.validate.field).uint32.gt = 0]; | ||
|
|
||
| // The pod name to tail logs from. If not provided, attempt to find the primary pod, else assume the first pod. | ||
| string pod_name = 3; | ||
| // Selects which pod(s) to tail. If unset, defaults to all_pods. | ||
| oneof pod_selector { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe we can call this pod_filter and then we can drop the all_pods option altogether since we default to it when no filter is set
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated it |
||
| // Tail logs from the primary pod only. | ||
| bool primary_pod = 3; | ||
| // Tail logs from all pods. This is the default when pod_selector is unset. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a departure from the currently checked in behavior - will it cause any issues for older UI clients?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry for the confusion. The default is primary pod (same as before). I just updated the comment |
||
| bool all_pods = 4; | ||
| // Tail logs from the named pod only. | ||
|
pingsutw marked this conversation as resolved.
Outdated
|
||
| string pod_name = 5; | ||
|
pingsutw marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
|
||
| // Reponse message for tailing logs. | ||
|
|
@@ -213,6 +220,8 @@ message TailLogsResponse { | |
| message Logs { | ||
| // Structured log lines. | ||
| repeated flyteidl2.logs.dataplane.LogLine lines = 1; | ||
| // Which container these lines are from. | ||
|
pingsutw marked this conversation as resolved.
|
||
| flyteidl2.logs.dataplane.ContainerIdentifier container = 2; | ||
|
pingsutw marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // One or more batches of logs. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.