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

Miscellaneous improvements about kvstore logging #28843

Merged
merged 6 commits into from
Nov 2, 2023

Conversation

giorio94
Copy link
Member

Please review commit by commit, and refer to the corresponding descriptions for additional information.

The logging level can be configured through a dedicated flag (and
environment variable). Let's drop this legacy ldflags variable,
as not specific to the kvstore package only.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
This alias appeared to be used only in a unit test. Hence, let's just
get rid of it.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. sig/kvstore Impacts the KVStore package interactions. labels Oct 27, 2023
In all usages, the name is somewhat related to the watched prefix.
Hence, let's just drop it, and consistently use the prefix in messages
for additional clarity.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Detailed logs concerning each key/value event received by every watcher
can be extremely verbose even at low scales, causing other useful
kvstore related debug messages to be missed. Hence, let's output them
only when verbose kvstore logs are enabled. This is also consistent
with all other kvstore operations.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94
Copy link
Member Author

/test

@giorio94 giorio94 marked this pull request as ready for review October 27, 2023 17:08
@giorio94 giorio94 requested review from a team as code owners October 27, 2023 17:08
Avoid recreating a new logger instance for every operation, as it is an
expensive operation in terms of allocations.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Only a type conversion, nothing useful to log there (even at trace
level).

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94
Copy link
Member Author

Fixed unit-test failure.

@giorio94
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 2, 2023
@tklauser tklauser merged commit f8c96d6 into cilium:main Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/kvstore Impacts the KVStore package interactions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants