Skip to content

Commit

Permalink
Deprecate support for directly accessing logger (#1690)
Browse files Browse the repository at this point in the history
Contributes to rapidsai/build-planning#104




This PR removes support for accessing rmm's underlying spdlog logger directly.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Lawrence Mitchell (https://github.com/wence-)
  - Mark Harris (https://github.com/harrism)

URL: #1690
  • Loading branch information
vyasr authored Oct 16, 2024
1 parent 1b70ffd commit de42f57
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 15 deletions.
25 changes: 16 additions & 9 deletions include/rmm/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ struct bytes {
}
};

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

/**
Expand All @@ -107,23 +112,25 @@ struct bytes {
*
* @return spdlog::logger& The logger.
*/
RMM_EXPORT inline spdlog::logger& logger()
[[deprecated(
"Support for direct access to spdlog loggers in rmm is planned for "
"removal")]] RMM_EXPORT inline spdlog::logger&
logger()
{
static detail::logger_wrapper wrapped{};
return wrapped.logger_;
return detail::logger();
}

//! @cond Doxygen_Suppress
//
// The default is INFO, but it should be used sparingly, so that by default a log file is only
// output if there is important information, warnings, errors, and critical failures
// Log messages that require computation should only be used at level TRACE and DEBUG
#define RMM_LOG_TRACE(...) SPDLOG_LOGGER_TRACE(&rmm::logger(), __VA_ARGS__)
#define RMM_LOG_DEBUG(...) SPDLOG_LOGGER_DEBUG(&rmm::logger(), __VA_ARGS__)
#define RMM_LOG_INFO(...) SPDLOG_LOGGER_INFO(&rmm::logger(), __VA_ARGS__)
#define RMM_LOG_WARN(...) SPDLOG_LOGGER_WARN(&rmm::logger(), __VA_ARGS__)
#define RMM_LOG_ERROR(...) SPDLOG_LOGGER_ERROR(&rmm::logger(), __VA_ARGS__)
#define RMM_LOG_CRITICAL(...) SPDLOG_LOGGER_CRITICAL(&rmm::logger(), __VA_ARGS__)
#define RMM_LOG_TRACE(...) SPDLOG_LOGGER_TRACE(&rmm::detail::logger(), __VA_ARGS__)
#define RMM_LOG_DEBUG(...) SPDLOG_LOGGER_DEBUG(&rmm::detail::logger(), __VA_ARGS__)
#define RMM_LOG_INFO(...) SPDLOG_LOGGER_INFO(&rmm::detail::logger(), __VA_ARGS__)
#define RMM_LOG_WARN(...) SPDLOG_LOGGER_WARN(&rmm::detail::logger(), __VA_ARGS__)
#define RMM_LOG_ERROR(...) SPDLOG_LOGGER_ERROR(&rmm::detail::logger(), __VA_ARGS__)
#define RMM_LOG_CRITICAL(...) SPDLOG_LOGGER_CRITICAL(&rmm::detail::logger(), __VA_ARGS__)

//! @endcond

Expand Down
2 changes: 1 addition & 1 deletion python/rmm/rmm/librmm/_logger.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,5 @@ cdef extern from "spdlog/spdlog.h" namespace "spdlog" nogil:
bool should_log(logging_level msg_level)


cdef extern from "rmm/logger.hpp" namespace "rmm" nogil:
cdef extern from "rmm/logger.hpp" namespace "rmm::detail" nogil:
cdef spdlog_logger& logger() except +
10 changes: 5 additions & 5 deletions tests/mr/device/tracking_mr_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,16 @@ TEST(TrackingTest, LogOutstandingAllocations)
{
std::ostringstream oss;
auto oss_sink = std::make_shared<spdlog::sinks::ostream_sink_st>(oss);
rmm::logger().sinks().push_back(oss_sink);
auto old_level = rmm::logger().level();
rmm::detail::logger().sinks().push_back(oss_sink);
auto old_level = rmm::detail::logger().level();

tracking_adaptor mr{rmm::mr::get_current_device_resource_ref()};
std::vector<void*> allocations;
for (std::size_t i = 0; i < num_allocations; ++i) {
allocations.push_back(mr.allocate(ten_MiB));
}

rmm::logger().set_level(spdlog::level::debug);
rmm::detail::logger().set_level(spdlog::level::debug);
EXPECT_NO_THROW(mr.log_outstanding_allocations());

#if SPDLOG_ACTIVE_LEVEL <= SPDLOG_LEVEL_DEBUG
Expand All @@ -224,8 +224,8 @@ TEST(TrackingTest, LogOutstandingAllocations)
mr.deallocate(allocation, ten_MiB);
}

rmm::logger().set_level(old_level);
rmm::logger().sinks().pop_back();
rmm::detail::logger().set_level(old_level);
rmm::detail::logger().sinks().pop_back();
}

} // namespace
Expand Down

0 comments on commit de42f57

Please sign in to comment.