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.

Sign up for GitHub

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

Jump to bottom

Introduce wasm::logger #16153

Merged
merged 3 commits into from
Jan 19, 2024
Merged

Introduce wasm::logger #16153

merged 3 commits into from
Jan 19, 2024

Conversation

oleiman
Copy link
Member

@oleiman oleiman commented Jan 18, 2024 •
edited

This PR introduces wasm::logger, which is intended to subsume ss::logger for transform logging purposes.

Closes https://github.com/redpanda-data/core-internal/issues/997

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
  • v23.1.x

Release Notes

  • none

@oleiman oleiman self-assigned this Jan 18, 2024
rockwotj
rockwotj reviewed Jan 18, 2024
View reviewed changes
src/v/wasm/logger.h Outdated Show resolved Hide resolved
src/v/wasm/logger.h Outdated Show resolved Hide resolved
src/v/wasm/logger.h Outdated Show resolved Hide resolved
src/v/wasm/logger.h Outdated Show resolved Hide resolved
src/v/wasm/tests/wasm_cache_test.cc Outdated Show resolved Hide resolved
src/v/wasm/wasi.cc Show resolved Hide resolved
@oleiman
wasm: Introduce wasm::logger
0104afe 
Introduces abstract wasm::logger base class exposing a virtual interface
for logging.

Future implementations will encapsulate higher level log management facilities.

Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
@oleiman
Copy link
Member Author

oleiman commented Jan 18, 2024

force push contents:

  • make wasm::logger pure virtual and move to wasm/api.h
  • split out some implementations for testing and transform::service wiring
  • remove some POC cruft

rockwotj
rockwotj previously approved these changes Jan 18, 2024
View reviewed changes
Copy link
Contributor

@rockwotj rockwotj left a comment

Choose a reason for hiding this comment

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

chef's kiss

src/v/transform/transform_logger.h Outdated Show resolved Hide resolved
@oleiman
transform: Introduce transform::logger
4f69748 
Encapsulates a transform name and a ss::logger.

Writes to broker logs for now, but in future will:

- Handle line buffering
- Forward log lines to a higher level log management facility
@oleiman
Copy link
Member Author

oleiman commented Jan 18, 2024

force push to s/ss::sstring/model::transform_name/ in transform_logger

rockwotj
rockwotj approved these changes Jan 18, 2024
View reviewed changes
Copy link
Contributor

@rockwotj rockwotj left a comment

Choose a reason for hiding this comment

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

LGTM

@oleiman oleiman requested a review from rockwotj January 18, 2024 21:56
rockwotj
rockwotj previously approved these changes Jan 18, 2024
View reviewed changes
@oleiman
Copy link
Member Author

oleiman commented Jan 19, 2024

force push missed a unit test running locally

rockwotj
rockwotj previously approved these changes Jan 19, 2024
View reviewed changes
@oleiman
transform: Wire up wasm::logger
d192d01 
The main change here is that, rather than initializing factories
with a `ss::logger*`, we'll initialize _engines_ with an owning
`wasm::logger` reference. The rest is just plumbing.

Also introduces some thin loggers for testing purposes and updates
`transform::service` with `transform::logger`.

Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
@vbotbuildovich
Copy link
Collaborator

new failures in https://buildkite.com/redpanda/redpanda/builds/43923#018d1f96-4bad-412b-8698-dfc41de61919:

"rptest.tests.partition_movement_test.PartitionMovementTest.test_deletion_stops_move.num_to_upgrade=2"

@vbotbuildovich
Copy link
Collaborator

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43923#018d1fa5-81e7-4242-b346-8de6921b35c5

@oleiman
Copy link
Member Author

oleiman commented Jan 19, 2024

CI Failure:

  • build looks to be something with PartitionMovementTest.test_deletion_stops_move and looks new. Probably not related to this diff though, as transforms wouldn't be enabled for that test.

@oleiman oleiman merged commit 0835c3f into redpanda-data:dev Jan 19, 2024
17 checks passed
@oleiman
Copy link
Member Author

oleiman commented Jan 19, 2024

/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-16153-v23.3.x-94 remotes/upstream/v23.3.x
git cherry-pick -x 0104afe1e0c3fb0d461925d24154c919bb0d3290 4f6974873cbe8197afbe87b7c0989ebeddadc121 d192d01376097d4cd2f4f7c6d791864362ee24ae

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request Jan 19, 2024
Closed
@oleiman oleiman mentioned this pull request Jan 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rockwotj rockwotj rockwotj approved these changes

@dotnwat dotnwat Awaiting requested review from dotnwat

Assignees

@oleiman oleiman

Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@oleiman @vbotbuildovich @rockwotj