Skip to content
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

rpk/transform: introduce logs command #16923

Merged
merged 10 commits into from
Mar 16, 2024

Conversation

rockwotj
Copy link
Contributor

@rockwotj rockwotj commented Mar 6, 2024

Fixes: https://github.com/redpanda-data/core-internal/issues/1009, https://github.com/redpanda-data/core-internal/issues/1079

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x

Release Notes

Features

  • Introduce rpk transform logs NAME to view logs for a transform

@rockwotj rockwotj force-pushed the rpk-transform-logs branch 2 times, most recently from ec477f5 to 778bb82 Compare March 7, 2024 15:19
@github-actions github-actions bot added area/redpanda area/wasm WASM Data Transforms labels Mar 7, 2024
@rockwotj rockwotj force-pushed the rpk-transform-logs branch 2 times, most recently from 632bd49 to 858fca7 Compare March 7, 2024 16:58
@rockwotj rockwotj requested a review from oleiman March 7, 2024 16:59
@twmb twmb removed their request for review March 7, 2024 16:59
@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 7, 2024

Force push: fixes from manual testing and fixed broker output to be more user friendly

@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 7, 2024

Force push: Another round of manual testing. Should be good to go now.

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Mar 7, 2024

new failures in https://buildkite.com/redpanda/redpanda/builds/45815#018e1a64-7c19-4dd5-a4e8-eb67b3e215ad:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45815#018e1a64-7c1f-4e7d-a3c2-0cccc43d6567:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_cc_escaping"

new failures in https://buildkite.com/redpanda/redpanda/builds/45815#018e1a64-7c1c-4ffb-9d6b-90807df1fb35:

"rptest.tests.consumer_group_balancing_test.ConsumerGroupBalancingTest.test_coordinator_nodes_balance"

new failures in https://buildkite.com/redpanda/redpanda/builds/45833#018e1b23-5c87-4a2a-b94d-739b8ff197cb:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45833#018e1b4b-56c1-4197-8d0c-448cce43e921:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45880#018e1edd-8c9a-4c73-bdcf-8a28e4b531d2:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45880#018e1eee-bcb1-4d46-92ee-e22b8e303806:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45906#018e1ff9-078a-4ea7-aa9a-6f91333529d2:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45906#018e2020-de1c-4276-9a5b-785fcb7cb20f:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/45906#018e2020-de19-4cec-8861-37248fcc06e5:

"rptest.tests.tx_coordinator_migration_test.TxCoordinatorMigrationTest.test_migrating_tx_manager_coordinator.with_failures=True.upgrade=True"

new failures in https://buildkite.com/redpanda/redpanda/builds/45906#018e2328-4fb4-40e8-b028-4f4d09f0d4f5:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

new failures in https://buildkite.com/redpanda/redpanda/builds/46062#018e33b7-aac8-4489-af46-b2e2baddfa2f:

"rptest.tests.delete_records_test.DeleteRecordsTest.test_delete_records_concurrent_truncations.cloud_storage_enabled=True"

new failures in https://buildkite.com/redpanda/redpanda/builds/46135#018e3ae9-3cb6-4547-994f-c7d028a1c73e:

"rptest.tests.data_transforms_test.DataTransformsLoggingTest.test_logs_volume"

@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 7, 2024

Force push: fix tests

@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 7, 2024

Force push: remove some files I didn't mean to add

Copy link
Contributor

@r-vasquez r-vasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

src/go/rpk/pkg/cli/transform/logs.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/transform/logs.go Show resolved Hide resolved
src/go/rpk/pkg/cli/transform/logs.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/transform/logs.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/transform/logs.go Show resolved Hide resolved
src/go/rpk/pkg/cli/transform/logs.go Show resolved Hide resolved
src/go/rpk/pkg/cli/transform/logs.go Show resolved Hide resolved
@rockwotj rockwotj requested a review from a team March 7, 2024 22:03
@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 7, 2024

@redpanda-data/documentation asking for a review of the help text for the new rpk transform logs NAME command.

@rockwotj rockwotj requested a review from r-vasquez March 7, 2024 22:05
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

core bits lgtm. I'll defer to @r-vasquez for signoff.

(I miss the Gerrit +1)

Comment on lines 516 to 517
self.body = self._rep.get(self.BODY_FIELD, None)
body_object = self._rep.get(self.BODY_FIELD, None)
if isinstance(body_object, dict):
self.body = body_object.get("stringValue", None)
Copy link
Member

@oleiman oleiman Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun. I knew I should have tried parsing this into the real protobuf. Oh well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh yeah I missed this in the reviews too 🤷

for (const iovec_t& vec : iovecs) {
try {
ffi::array<char> data = mem->translate_array<char>(
auto level = fd == 1 ? ss::log_level::info : ss::log_level::warn;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rockwotj - Thanks for sorting this out btw. I spent a few hours staring at it while you were out and couldn't quite visualize where things ought to go.

r-vasquez
r-vasquez previously approved these changes Mar 7, 2024
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rockwotj rockwotj requested a review from oleiman March 12, 2024 21:31
oleiman
oleiman previously approved these changes Mar 12, 2024
kbatuigas
kbatuigas previously approved these changes Mar 13, 2024
src/go/rpk/pkg/cli/transform/logs.go Outdated Show resolved Hide resolved
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
Copy link
Contributor Author

Force push: docs fixes

oleiman
oleiman previously approved these changes Mar 13, 2024
@rockwotj
Copy link
Contributor Author

Force push: make logs test more robust now that we produce exactly the correct number of logs (see slack thread)

@rockwotj rockwotj requested a review from oleiman March 15, 2024 15:56
tests/rptest/util.py Outdated Show resolved Hide resolved
@rockwotj rockwotj requested a review from oleiman March 15, 2024 16:03
oleiman
oleiman previously approved these changes Mar 15, 2024
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

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>
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>
@rockwotj rockwotj merged commit 3772047 into redpanda-data:dev Mar 16, 2024
24 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-16923-v23.3.x-71 remotes/upstream/v23.3.x
git cherry-pick -x 4bb172bb759a7ecbe8ab63c2f9c03e7c6de54d73 f15257eb1eb0403d4413620256e76ba0d0d34a11 6458d3e51ca5f37268d11ea6664fa60c7e061b65 74d0af91d1c8fd24918cd2cb35228aa124ad95ba 2e648a04d969d5191afdc9cdc8dadad36d20a606 c9a0889ac563201e93b082df8a843f2149e12ce0 080964b4170752ab96687163c740dd54b1d735be 74de39c8adfa699103a6faba6d327e94a8afaa63 bd6ed7fa7c5ad908fa29d584bf6171f6d8e0dec7 d03086c7a94c79b31dbbc7b5707e92e7b9f6262e

Workflow run logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants