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

refactor(log): adjust log level and macro names #1200

Merged
merged 4 commits into from
Oct 25, 2022

Conversation

acelyc111
Copy link
Member

@acelyc111 acelyc111 commented Oct 23, 2022

#1199

For historical reasons, the log levels of Pegasus is a bit of confused and not consistent with other well known and wildly used logging systems. This patch aim to resolve this issuse, includes the following changes:

  • Re-order log levels to: DEBUG,INFO,WARNING,ERROR,FATAL, see src/runtime/task/task_spec.h, src/utils/api_utilities.h,
  • Change the log relationship between libevent and Pegasus, see src/reporter/pegasus_counter_reporter.cpp
  • Change the log relationship between sasl and Pegasus, see src/runtime/security/sasl_init.cpp
  • Simplify LOG_LEVEL_INFORMATION to LOG_LEVEL_INFO
  • Change logging macros:
    • dinfo -> LOG_DEBUG
    • ddebug -> LOG_INFO
    • dwarn -> LOG_WARNING
    • derror -> LOG_ERROR
    • dfatal -> LOG_FATAL
  • Update related tests
  • Update related config files
  • Update all places using the macros above

Of course, this patch will take effect on thousands of source files, I used IDE to replace the macros, and I've checked these files one by one manually. Reviewers could check the following files to save time:

  1. src/reporter/pegasus_counter_reporter.cpp
  2. src/runtime/security/sasl_init.cpp
  3. src/runtime/task/task_spec.h
  4. src/utils/api_utilities.h
  5. src/utils/fmt_logging.h
  6. src/utils/test/logging.cpp

TODO: log levels using fmt (e.g. dinfo_f, dwarn_f) is left as before and will be refactored in the next patch.

Compatibility

Log levels of DEBUG and INFO are exchanged, should be mentioned in release note.

@github-actions github-actions bot added the cpp label Oct 23, 2022
@acelyc111 acelyc111 marked this pull request as ready for review October 24, 2022 03:14
@empiredan empiredan merged commit fc46688 into apache:master Oct 25, 2022
@acelyc111 acelyc111 mentioned this pull request Oct 25, 2022
3 tasks
@empiredan empiredan mentioned this pull request Aug 17, 2023
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.

4 participants