Skip to content

chore(log): replace logging calls in cli/* and common/* #2888

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

Merged
merged 2 commits into from
Apr 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/cli/daemon_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@
inline bool SupervisedUpstart() {
const char *upstart_job = getenv("UPSTART_JOB");
if (!upstart_job) {
LOG(WARNING) << "upstart supervision requested, but UPSTART_JOB not found";
warn("upstart supervision requested, but UPSTART_JOB not found");
return false;
}
LOG(INFO) << "supervised by upstart, will stop to signal readiness";
info("supervised by upstart, will stop to signal readiness");
raise(SIGSTOP);
unsetenv("UPSTART_JOB");
return true;
Expand All @@ -46,13 +46,13 @@ inline bool SupervisedUpstart() {
inline bool SupervisedSystemd() {
const char *notify_socket = getenv("NOTIFY_SOCKET");
if (!notify_socket) {
LOG(WARNING) << "systemd supervision requested, but NOTIFY_SOCKET not found";
warn("systemd supervision requested, but NOTIFY_SOCKET not found");
return false;
}

auto fd = UniqueFD(socket(AF_UNIX, SOCK_DGRAM, 0));
if (!fd) {
LOG(WARNING) << "Cannot connect to systemd socket " << notify_socket;
warn("Cannot connect to systemd socket {}", notify_socket);
return false;
}

Expand All @@ -79,7 +79,7 @@ inline bool SupervisedSystemd() {
sendto_flags |= MSG_NOSIGNAL;
#endif
if (sendmsg(*fd, &hdr, sendto_flags) < 0) {
LOG(WARNING) << "Cannot send notification to systemd";
warn("Cannot send notification to systemd");
return false;
}
return true;
Expand All @@ -106,15 +106,15 @@ inline bool IsSupervisedMode(SupervisedMode mode) {
inline void Daemonize() {
pid_t pid = fork();
if (pid < 0) {
LOG(ERROR) << "Failed to fork the process, err: " << strerror(errno);
error("Failed to fork the process, error: ", strerror(errno));
exit(1);
}

if (pid > 0) exit(EXIT_SUCCESS); // parent process
// change the file mode
umask(0);
if (setsid() < 0) {
LOG(ERROR) << "Failed to setsid, err: " << strerror(errno);
error("Failed to setsid, error: ", strerror(errno));
exit(1);
}

Expand Down
14 changes: 7 additions & 7 deletions src/cli/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Server *srv = nullptr;

extern "C" void SignalHandler(int sig) {
if (srv && !srv->IsStopped()) {
LOG(INFO) << "Signal " << strsignal(sig) << " (" << sig << ") received, stopping the server";
info("Signal {} ({}) received, stopping the server", strsignal(sig), sig);
srv->Stop();
}
}
Expand Down Expand Up @@ -80,7 +80,7 @@ static CLIOptions ParseCommandLineOptions(int argc, char **argv) {
if ((argv[i] == "-c"sv || argv[i] == "--config"sv) && i + 1 < argc) {
opts.conf_file = argv[++i];
} else if (argv[i] == "-v"sv || argv[i] == "--version"sv) {
std::cout << "kvrocks " << PrintVersion << std::endl;
std::cout << "kvrocks " << PrintVersion() << std::endl;
std::exit(0);
} else if (argv[i] == "-h"sv || argv[i] == "--help"sv) {
PrintUsage(*argv);
Expand Down Expand Up @@ -167,15 +167,15 @@ int main(int argc, char *argv[]) {
std::cout << "Failed to initialize logging system. Error: " << s.Msg() << std::endl;
return 1;
}
LOG(INFO) << "kvrocks " << PrintVersion;
info("kvrocks {}", PrintVersion());
// Tricky: We don't expect that different instances running on the same port,
// but the server use REUSE_PORT to support the multi listeners. So we connect
// the listen port to check if the port has already listened or not.
if (config.socket_fd == -1 && !config.binds.empty()) {
uint32_t ports[] = {config.port, config.tls_port, 0};
for (uint32_t *port = ports; *port; ++port) {
if (util::IsPortInUse(*port)) {
LOG(ERROR) << "Could not create server TCP since the specified port[" << *port << "] is already in use";
error("Could not create the server since the specified port {} is already in use", *port);
return 1;
}
}
Expand All @@ -184,7 +184,7 @@ int main(int argc, char *argv[]) {
if (config.daemonize && !is_supervised) Daemonize();
s = CreatePidFile(config.pidfile);
if (!s.IsOK()) {
LOG(ERROR) << "Failed to create pidfile: " << s.Msg();
error("Failed to create pidfile: {}", s.Msg());
return 1;
}
auto pidfile_exit = MakeScopeExit([&config] { RemovePidFile(config.pidfile); });
Expand All @@ -199,14 +199,14 @@ int main(int argc, char *argv[]) {
engine::Storage storage(&config);
s = storage.Open();
if (!s.IsOK()) {
LOG(ERROR) << "Failed to open: " << s.Msg();
error("Failed to open the database: {}", s.Msg());
return 1;
}
Server server(&storage, &config);
srv = &server;
s = srv->Start();
if (!s.IsOK()) {
LOG(ERROR) << "Failed to start server: " << s.Msg();
error("Failed to start server: {}", s.Msg());
return 1;
}
srv->Join();
Expand Down
11 changes: 6 additions & 5 deletions src/cli/signal_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,17 @@
#include "version_util.h"

extern "C" inline void SegvHandler(int sig, [[maybe_unused]] siginfo_t *info, [[maybe_unused]] void *secret) {
LOG(ERROR) << "Ooops! Apache Kvrocks " << PrintVersion << " got signal: " << strsignal(sig) << " (" << sig << ")";
error("Ooops! Apache Kvrocks {} got signal: {} ({})", PrintVersion(), strsignal(sig), sig);
auto trace = cpptrace::generate_trace();

std::string trace_str;
std::ostringstream os(trace_str);
trace.print(os);
LOG(ERROR)
<< os.str()
<< "It would be greatly appreciated if you could submit this crash to https://github.com/apache/kvrocks/issues "
"along with the stacktrace above, logs and any relevant information.";

error("{}", os.str());
error(
"It would be greatly appreciated if you could submit this crash to https://github.com/apache/kvrocks/issues "
"along with the stacktrace above, logs and any relevant information.");

struct sigaction act;
/* Make sure we exit with the right signal at the end. So for instance
Expand Down
13 changes: 8 additions & 5 deletions src/cli/version_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,22 @@
#pragma once

#include <iostream>
#include <iterator>

#include "fmt/base.h"
#include "version.h"

inline std::ostream &PrintVersion(std::ostream &os) {
inline std::string PrintVersion() {
std::string result;
if (VERSION != "unstable") {
os << "version ";
result += "version ";
}

os << VERSION;
result += VERSION;

if (!GIT_COMMIT.empty()) {
os << " (commit " << GIT_COMMIT << ")";
fmt::format_to(std::back_inserter(result), " (commit {})", GIT_COMMIT);
}

return os;
return result;
}
22 changes: 11 additions & 11 deletions src/common/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,43 +30,43 @@

// just like std::source_location::current() in C++20 and __builtin_source_location(),
// but works in lower version compilers (GCC and Clang)
inline spdlog::source_loc CurrentLocation(const char *filename = __builtin_FILE(), int lineno = __builtin_LINE(),
const char *funcname = __builtin_FUNCTION()) {
inline constexpr spdlog::source_loc CurrentLocation(const char *filename = __builtin_FILE(),
int lineno = __builtin_LINE(),
const char *funcname = __builtin_FUNCTION()) {
return {filename, lineno, funcname};
}

template <typename... Args>
struct FormatMessageWithLoc {
template <typename T>
FormatMessageWithLoc(T &&v, spdlog::source_loc loc = CurrentLocation()) // NOLINT
: fmt(std::forward<T>(v)), current_loc(loc) {}
constexpr FormatMessageWithLoc(const T &v, spdlog::source_loc loc = CurrentLocation()) // NOLINT
: fmt(v), current_loc(loc) {}

spdlog::format_string_t<Args...> fmt;
std::string_view fmt;
spdlog::source_loc current_loc;
};

template <typename... Args>
inline void debug(FormatMessageWithLoc<Args...> fmt, Args &&...args) { // NOLINT
inline void debug(FormatMessageWithLoc fmt, Args &&...args) { // NOLINT
spdlog::default_logger_raw()->log(fmt.current_loc, spdlog::level::debug, fmt.fmt, std::forward<Args>(args)...);
}

template <typename... Args>
inline void info(FormatMessageWithLoc<Args...> fmt, Args &&...args) { // NOLINT
inline void info(FormatMessageWithLoc fmt, Args &&...args) { // NOLINT
spdlog::default_logger_raw()->log(fmt.current_loc, spdlog::level::info, fmt.fmt, std::forward<Args>(args)...);
}

template <typename... Args>
inline void warn(FormatMessageWithLoc<Args...> fmt, Args &&...args) { // NOLINT
inline void warn(FormatMessageWithLoc fmt, Args &&...args) { // NOLINT
spdlog::default_logger_raw()->log(fmt.current_loc, spdlog::level::warn, fmt.fmt, std::forward<Args>(args)...);
}

template <typename... Args>
inline void error(FormatMessageWithLoc<Args...> fmt, Args &&...args) { // NOLINT
inline void error(FormatMessageWithLoc fmt, Args &&...args) { // NOLINT
spdlog::default_logger_raw()->log(fmt.current_loc, spdlog::level::err, fmt.fmt, std::forward<Args>(args)...);
}

template <typename... Args>
[[noreturn]] inline void fatal(FormatMessageWithLoc<Args...> fmt, Args &&...args) { // NOLINT
[[noreturn]] inline void fatal(FormatMessageWithLoc fmt, Args &&...args) { // NOLINT
spdlog::default_logger_raw()->log(fmt.current_loc, spdlog::level::critical, fmt.fmt, std::forward<Args>(args)...);
std::abort();
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/task_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Status TaskRunner::Join() {

for (auto &thread : threads_) {
if (auto s = util::ThreadJoin(thread); !s) {
LOG(WARNING) << "Failed to join thread: " << s.Msg();
warn("Failed to join thread: {}", s.Msg());
continue;
}
}
Expand Down
4 changes: 2 additions & 2 deletions utils/kvrocks2redis/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ static Options ParseCommandLineOptions(int argc, char **argv) {
break;
}
case 'v':
std::cout << "kvrocks2redis " << PrintVersion << std::endl;
std::cout << "kvrocks2redis " << PrintVersion() << std::endl;
exit(0);
case 'h':
default:
Expand Down Expand Up @@ -112,7 +112,7 @@ int main(int argc, char *argv[]) {
}

InitSpdlog(config);
LOG(INFO) << "kvrocks2redis " << PrintVersion;
LOG(INFO) << "kvrocks2redis " << PrintVersion();

if (config.daemonize) Daemonize();

Expand Down
Loading