Skip to content

hive: take Logger as part of Start/Stop or Run#3

Merged
joamaki merged 1 commit intocilium:mainfrom
bimmlerd:pr/bimmlerd/remove-default-logger
Apr 24, 2024
Merged

hive: take Logger as part of Start/Stop or Run#3
joamaki merged 1 commit intocilium:mainfrom
bimmlerd:pr/bimmlerd/remove-default-logger

Conversation

@bimmlerd
Copy link
Copy Markdown
Member

Instead of having to provide a logger already at hive creation time, pass the logger to Start/Stop/Run, which is when the underlying cells actually need it.

Also revert the changes to the Apply interface by passing the logger when the invokes are actually run instead of during apply.

Finally, add hivetest.Logger to create a logger from a testing.TB, so that the logs can be attributed to the correct test when running tests in parallel. Unfortunately, t.Helper() doesn't correctly work in combination with slog - we log the real source location as an attribute as a workaround.

Instead of having to provide a logger already at hive creation time,
pass the logger to Start/Stop/Run, which is when the underlying cells
actually need it.

Also revert the changes to the Apply interface by passing the logger
when the invokes are actually run instead of during apply.

Finally, add hivetest.Logger to create a logger from a testing.TB, so
that the logs can be attributed to the correct test when running tests
in parallel. Unfortunately, t.Helper() doesn't correctly work in
combination with slog - we log the real source location as an attribute
as a workaround.

Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
@bimmlerd bimmlerd force-pushed the pr/bimmlerd/remove-default-logger branch from 7fe45e8 to f70eb37 Compare April 24, 2024 08:01
@joamaki joamaki self-requested a review April 24, 2024 08:02
@joamaki joamaki merged commit d782c60 into cilium:main Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants