-
Notifications
You must be signed in to change notification settings - Fork 558
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
[v23.3.x] rpk/transform: introduce logs command #17148
Merged
rockwotj
merged 10 commits into
redpanda-data:v23.3.x
from
rockwotj:vbotbuildovich/backport-16923-v23.3.x-106
Mar 17, 2024
Merged
[v23.3.x] rpk/transform: introduce logs command #17148
rockwotj
merged 10 commits into
redpanda-data:v23.3.x
from
rockwotj:vbotbuildovich/backport-16923-v23.3.x-106
Mar 17, 2024
Conversation
This file contains 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
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 4bb172b)
Add the initial boilerplate for the command, as well as the flags. This includes adding the timequery parsing flags. The parsing code here is very similar to the timequery parsing flags in `rpk topic consume`, except they are simplified a little bit. Tests are added to ensure parsing is correct. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit f15257e)
Implement the logs command for transforms. This command is very similar in general structure to `rpk topic consume`, as they are fundamentally doing similar things. The differences is that we only care about a single partition, so we lookup the partition the transform lives in and then consume all logs within the bounds of our query and print them. There is some awkwardness as we need to buffer log messages to properly emit newlines (otherwise we could get partial log lines split on multiple lines of output, and we split warning logs and info logs on different lines). Otherwise we add timestamps and levels to the output when one does `--format=wide`. Lastly, the `--tail` flag is hard to implement using franz-go. One could make a new client everytime and incrementally read batches of records in reverse, but that seems complex, so we just instead buffer everything then dump the logs at the end. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 6458d3e)
The Open Telemetry body is an AnyValue, which we just happen to always make a string. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 74d0af9)
The `wasm::logger` implementations should handle the validation that was happening here. This existing code was stripping out newlines. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 2e648a0)
Allow logs to have newlines and tabs, but since absl escaping also escapes newlines, that results in a bad UX for users. Just drop if there are actually control characters but valid UTF8. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit c9a0889)
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 080964b)
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 74de39c)
Offsets are 0 indexed, so the last record in the transform should be n - 1. NOTE: It's still possible for this test to be flaky if all the input partitions end up on the same core, as all of the partitions could produce logs at once, overshooting the log buffer. Maybe that is unlikely enough it will never happen. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit bd6ed7f)
This was always recording 0 bytes because there was an unrolled batch, which this method doesn't report. It seems cleaner to just count the bytes that are returned from the builder anyways. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit d03086c)
rockwotj
requested review from
twmb,
r-vasquez and
gene-redpanda
as code owners
March 16, 2024 15:35
only manual conflicts where due to include changes in a deleted test |
oleiman
approved these changes
Mar 17, 2024
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.
lgtm
just the base/units.h -> units.h
in the wasi_logs_test
, right?
Yes and got threw a fit because the file was deleted (easy to resolve tho) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport of PR #16923