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.

Sign up for GitHub

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

Deprecate support for directly accessing logger #16964

Merged
merged 4 commits into from
Oct 4, 2024
Merged

Deprecate support for directly accessing logger #16964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b23c86
Deprecate support for directly accessing logger
vyasr Oct 1, 2024
28a86b9
Make sure builds don't have warnings and update tests
vyasr Oct 1, 2024
c6bb562
Also export detail symbol for now
vyasr Oct 1, 2024
84b9e5d
Move definition into main namespace block since we need it there anyway
vyasr Oct 1, 2024
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 cpp/include/cudf/detail/utilities/logger.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, NVIDIA CORPORATION.
* Copyright (c) 2023-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,9 +19,9 @@
#include <cudf/utilities/logger.hpp>

// Log messages that require computation should only be used at level TRACE and DEBUG
#define CUDF_LOG_TRACE(...) SPDLOG_LOGGER_TRACE(&cudf::logger(), __VA_ARGS__)
#define CUDF_LOG_DEBUG(...) SPDLOG_LOGGER_DEBUG(&cudf::logger(), __VA_ARGS__)
#define CUDF_LOG_INFO(...) SPDLOG_LOGGER_INFO(&cudf::logger(), __VA_ARGS__)
#define CUDF_LOG_WARN(...) SPDLOG_LOGGER_WARN(&cudf::logger(), __VA_ARGS__)
#define CUDF_LOG_ERROR(...) SPDLOG_LOGGER_ERROR(&cudf::logger(), __VA_ARGS__)
#define CUDF_LOG_CRITICAL(...) SPDLOG_LOGGER_CRITICAL(&cudf::logger(), __VA_ARGS__)
#define CUDF_LOG_TRACE(...) SPDLOG_LOGGER_TRACE(&cudf::detail::logger(), __VA_ARGS__)
#define CUDF_LOG_DEBUG(...) SPDLOG_LOGGER_DEBUG(&cudf::detail::logger(), __VA_ARGS__)
#define CUDF_LOG_INFO(...) SPDLOG_LOGGER_INFO(&cudf::detail::logger(), __VA_ARGS__)
#define CUDF_LOG_WARN(...) SPDLOG_LOGGER_WARN(&cudf::detail::logger(), __VA_ARGS__)
#define CUDF_LOG_ERROR(...) SPDLOG_LOGGER_ERROR(&cudf::detail::logger(), __VA_ARGS__)
#define CUDF_LOG_CRITICAL(...) SPDLOG_LOGGER_CRITICAL(&cudf::detail::logger(), __VA_ARGS__)
8 changes: 7 additions & 1 deletion cpp/include/cudf/utilities/logger.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@

namespace CUDF_EXPORT cudf {

namespace detail {
spdlog::logger& logger();
}

PointKernel marked this conversation as resolved.
Show resolved Hide resolved
/**
* @brief Returns the global logger.
*
Expand All @@ -43,6 +47,8 @@ namespace CUDF_EXPORT cudf {
*
* @return spdlog::logger& The logger.
*/
spdlog::logger& logger();
[[deprecated(
"Support for direct access to spdlog loggers in cudf is planned for removal")]] spdlog::logger&
logger();

} // namespace CUDF_EXPORT cudf
4 changes: 3 additions & 1 deletion cpp/src/utilities/logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ struct logger_wrapper {

} // namespace

spdlog::logger& cudf::logger()
spdlog::logger& cudf::detail::logger()
{
static logger_wrapper wrapped{};
return wrapped.logger_;
}

spdlog::logger& cudf::logger() { return cudf::detail::logger(); }
37 changes: 19 additions & 18 deletions cpp/tests/utilities_tests/logger_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ class LoggerTest : public cudf::test::BaseFixture {
std::vector<spdlog::sink_ptr> prev_sinks;

public:
LoggerTest() : prev_level{cudf::logger().level()}, prev_sinks{cudf::logger().sinks()}
LoggerTest()
: prev_level{cudf::detail::logger().level()}, prev_sinks{cudf::detail::logger().sinks()}
{
cudf::logger().sinks() = {std::make_shared<spdlog::sinks::ostream_sink_mt>(oss)};
cudf::logger().set_formatter(
cudf::detail::logger().sinks() = {std::make_shared<spdlog::sinks::ostream_sink_mt>(oss)};
cudf::detail::logger().set_formatter(
std::unique_ptr<spdlog::formatter>(new spdlog::pattern_formatter("%v")));
}
~LoggerTest() override
{
cudf::logger().set_level(prev_level);
cudf::logger().sinks() = prev_sinks;
cudf::detail::logger().set_level(prev_level);
cudf::detail::logger().sinks() = prev_sinks;
}

void clear_sink() { oss.str(""); }
Expand All @@ -46,32 +47,32 @@ class LoggerTest : public cudf::test::BaseFixture {

TEST_F(LoggerTest, Basic)
{
cudf::logger().critical("crit msg");
cudf::detail::logger().critical("crit msg");
ASSERT_EQ(this->sink_content(), "crit msg\n");
}

TEST_F(LoggerTest, DefaultLevel)
{
cudf::logger().trace("trace");
cudf::logger().debug("debug");
cudf::logger().info("info");
cudf::logger().warn("warn");
cudf::logger().error("error");
cudf::logger().critical("critical");
cudf::detail::logger().trace("trace");
cudf::detail::logger().debug("debug");
cudf::detail::logger().info("info");
cudf::detail::logger().warn("warn");
cudf::detail::logger().error("error");
cudf::detail::logger().critical("critical");
ASSERT_EQ(this->sink_content(), "warn\nerror\ncritical\n");
}

TEST_F(LoggerTest, CustomLevel)
{
cudf::logger().set_level(spdlog::level::warn);
cudf::logger().info("info");
cudf::logger().warn("warn");
cudf::detail::logger().set_level(spdlog::level::warn);
cudf::detail::logger().info("info");
cudf::detail::logger().warn("warn");
ASSERT_EQ(this->sink_content(), "warn\n");

this->clear_sink();

cudf::logger().set_level(spdlog::level::debug);
cudf::logger().trace("trace");
cudf::logger().debug("debug");
cudf::detail::logger().set_level(spdlog::level::debug);
cudf::detail::logger().trace("trace");
cudf::detail::logger().debug("debug");
ASSERT_EQ(this->sink_content(), "debug\n");
}
Loading