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

verbose_logging_timeout_sec_max #15798

Merged
merged 9 commits into from
Dec 21, 2023

Conversation

oleiman
Copy link
Member

@oleiman oleiman commented Dec 20, 2023

This PR introduces verbose_logging_timeout_sec_max, which, if set, causes expiration times included in set_log_level requests to be clamped before timeouts are calculated.

Includes an assortment of minor improvements to the logic around setting log levels and managing timeouts around those settings.

Fixes https://github.com/redpanda-data/core-internal/issues/938
Fixes https://github.com/redpanda-data/core-internal/issues/931
Fixes https://github.com/redpanda-data/core-internal/issues/930

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

Improvements

  • verbose_logging_timeout_max node config, to prevent setting TRACE and DEBUG log level indefinitely

Added option to v1/loggers to provide the log levels in addition
to the names of the loggers.  Use v1/loggers?include-levels=true

Signed-off-by: Michael Boquard <michael@redpanda.com>
Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
@oleiman oleiman self-assigned this Dec 20, 2023
@oleiman oleiman marked this pull request as draft December 20, 2023 07:04
@vbotbuildovich
Copy link
Collaborator

Copy link
Contributor

@michael-redpanda michael-redpanda left a comment

Choose a reason for hiding this comment

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

looks fantastic, just a couple of notes

src/v/redpanda/admin/api-doc/config.def.json Outdated Show resolved Hide resolved
src/v/config/node_config.cc Outdated Show resolved Hide resolved
src/v/config/node_config.h Outdated Show resolved Hide resolved
src/v/redpanda/admin/server.h Outdated Show resolved Hide resolved
@oleiman
Copy link
Member Author

oleiman commented Dec 20, 2023

Added message to set_log_level response that reports previous
level of the logger.

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

oleiman commented Dec 20, 2023

force push contents:

  • rename config to add units
  • config defaults to nullopt to preserve existing behavior
  • add get_log_level to rptest.Admin
  • tests check get_log_level (though it's still useful to monitor logs)
  • nits

@oleiman oleiman marked this pull request as ready for review December 20, 2023 19:18
@michael-redpanda
Copy link
Contributor

/ci-repeat 5
dt-repeat=5
skip-redpanda-build
tests/rptest/tests/log_level_test.py

Copy link
Contributor

@michael-redpanda michael-redpanda left a comment

Choose a reason for hiding this comment

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

couple more questions but looks great!

src/v/config/node_config.cc Outdated Show resolved Hide resolved
src/v/redpanda/admin/server.h Outdated Show resolved Hide resolved
@piyushredpanda piyushredpanda added this to the v23.3.1-rc6 milestone Dec 21, 2023
Max duration for TRACE or DEBUG level logging. If null (default) we
impose no  limit.

Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
Allows setting an expiration that exceeds `verbose_logging_timeout_max`.

Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
- Adds get_log_level
- Improve set_log_level
  - Allows setting expiration=0
  - Adds force query parameter

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

oleiman commented Dec 21, 2023

force push for better operator<(level_reset&, level_reset&) semantics and bounded_property for new config

Copy link
Contributor

@michael-redpanda michael-redpanda left a comment

Choose a reason for hiding this comment

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

lgtm

src/v/redpanda/admin/server.h Outdated Show resolved Hide resolved
This allows us to keep the default level for each logger around
in the level reset map and distinguish between loggers with expiring
settings and those without when the log reset timer fires.

Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
- Clamps timeouts to node config
  - inclusive of 0-valued setting (no timeout)
- Respects force query param to override configured max
- Log the timeout (after clamping) at INFO level

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

oleiman commented Dec 21, 2023

force push condition

@michael-redpanda michael-redpanda merged commit 7b3096e into redpanda-data:dev Dec 21, 2023
21 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.1.x

@vbotbuildovich
Copy link
Collaborator

Oops! Something went wrong.

Workflow run logs.

@vbotbuildovich
Copy link
Collaborator

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

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15798-v23.2.x-896 remotes/upstream/v23.2.x
git cherry-pick -x 2404523d7824bc4b19e2097901dd8da59f11897a cc792b7a5889e8d3d83a4a690d4950747d8f92bb ab79ddc842d1ea5e9d74bd486af4b726cdc8ecdf 194ab70ae968380036ed07f9aaf1a8f30006cc63 d15cd37b20b3d4f7a693c00218710d13349b67c2 88ffdea66e915f02a29f84f32e767ea18d702bbf d1f95563a7964381d17bc55bf453f7ab1efb369c 1cc5583e2899ca07ada60e9a16dd0bb7692a1cb5 4d4c975dc41d7ba047e6afc47fc55be31c0ea219

Workflow run logs.

@vbotbuildovich
Copy link
Collaborator

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

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15798-v23.1.x-301 remotes/upstream/v23.1.x
git cherry-pick -x 2404523d7824bc4b19e2097901dd8da59f11897a cc792b7a5889e8d3d83a4a690d4950747d8f92bb ab79ddc842d1ea5e9d74bd486af4b726cdc8ecdf 194ab70ae968380036ed07f9aaf1a8f30006cc63 d15cd37b20b3d4f7a693c00218710d13349b67c2 88ffdea66e915f02a29f84f32e767ea18d702bbf d1f95563a7964381d17bc55bf453f7ab1efb369c 1cc5583e2899ca07ada60e9a16dd0bb7692a1cb5 4d4c975dc41d7ba047e6afc47fc55be31c0ea219

Workflow run logs.

@oleiman oleiman changed the title verbose_logging_timeout_max verbose_logging_timeout_sec_max Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants