-
Notifications
You must be signed in to change notification settings - Fork 200
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
Create logger wrapper around spdlog that can be easily reused in other libraries #1722
Merged
Merged
Changes from all commits
Commits
Show all changes
122 commits
Select commit
Hold shift + click to select a range
27b6cab
First version of new logger
vyasr aa92779
Make rmm backwards compat layers explicit
vyasr 812b2bc
Move logger to its own directory
vyasr 956fa3f
Set up basics of file generation
vyasr 3a6d49b
Convert to a function
vyasr c9ec22c
Switch to using a target
vyasr 2573e02
Make rmm compatibility and logging support controllable via CMake
vyasr 9173108
Hide impl one level down
vyasr d89bb82
Make rapids_logger a CMake package
vyasr 07dfcc5
Fix install rules
vyasr 8194997
Switch arena to delegating constructor for simplicity
vyasr 8589c53
Clean up some conditional compilation
vyasr f4f8350
Move some functions out of the impl
vyasr 7da8d70
Get everything compiling and running except arena tests with the new …
vyasr 54abd70
Get arena test passing
vyasr 627d369
Support a different approach for custom class formatting
vyasr 39fe1b2
Get rid of redundant fake_impl
vyasr 4a6cc9d
Some cleanup and consolidation of ifdef blocks
vyasr 91fab68
Compile an object library for the tests to avoid recompilation.
vyasr 2f58cc5
Properly enable building without logging support
vyasr 2c021ec
Clean up some todos
vyasr eb93bf2
Add documentation
vyasr 7b449bc
Simplify options handling and remove unnecessary exporting rules
vyasr 9f85459
Set default options expected for rmm
vyasr 946573a
Add documentation and stop defaulting export set
vyasr 19556a6
Add functions required by the Python API
vyasr 4a0dd6b
Get the Python package building with the new logger
vyasr bec1a9f
Get Python package importable
vyasr 0be4979
Get all tests passing
vyasr 0d6b118
Consolidate enum conversion
vyasr adae837
Make logger work with cudf
vyasr b422af6
Go back to compatibility mode for now
vyasr f7ae2be
Fix benchmarks
vyasr 6a3028c
Comment out bits in the Python build that we don't need yet
vyasr afc4dcf
Fix the flag in Python
vyasr 4714322
Temporarily reinstate inline keyword to get rmm working.
vyasr 08cb91b
Make Python target selection safe in all modes
vyasr aaf4b7c
Address first round of feedback
vyasr 7c9134c
Prefix new CMake variables with RMM_ and ensure that SUPPORTS_LOGGING…
vyasr 8fd9036
Get tests and benchmarks fully working when building without compatib…
vyasr e6ec9f4
Fix tests
vyasr 62efe44
Fix handling of configurable inline
vyasr 0c08a3f
Fix up Python builds and tests again for non-legacy case
vyasr ee33b25
Make arena printing work in backwards compatibility mode
vyasr 28e7c21
Add set_pattern to logger
vyasr 55b3926
Remove unnecessary level of indirection.
vyasr 56ab836
Simplify logging_resource_adaptor constructor stack
vyasr a690620
Implement basic support for sinks to get logging_resource_adaptor wor…
vyasr 19c82ef
Fix logger format strings
vyasr 9faf1b3
Fix logger output to match old fmt prints
vyasr b9b14f5
Inline the extra log call, the extra lambda invocation shouldn't be t…
vyasr 39b3da8
Update logger tests for when logging is not supported
vyasr f299fd2
Make compatible with backwards compatibility mode again
vyasr 15cdb0c
Make notes on what changes will be needed to use hide all spdlog symb…
vyasr 3e8e286
Make notes on tests expected to fail when logging is disabled
vyasr 440d7c1
Always hide the visibility of the logger implementation bits.
vyasr 704c8e2
Add all notes on how we want to use spdlog to fully hide symbols in t…
vyasr 5d11768
Make sure the unique_ptr is not default instantiated
vyasr ebdd741
Merge remote-tracking branch 'upstream/branch-24.12' into feat/logger
vyasr b8f22ec
Merge branch 'branch-25.02' into feat/logger
vyasr 1fdee97
Remove all compatibility logic
vyasr b372c69
Resort the file to reduce complexity for the reader
vyasr 6f6603a
Remove now redundant bytes formatting struct
vyasr b102d62
Move implementation of formatted_log from rmm to rapids_logger
vyasr 1b3add3
Inline formatted_log into log
vyasr 6379a72
More consolidation and remove unnecessary init_logger impl
vyasr 4ef279d
Remove use of rmm error-handling macro
vyasr eb6ef9f
Clean up some includes
vyasr 00f5577
Expose the default log filename publicly
vyasr cd71f30
Enable dynamic linking to spdlog
vyasr 073e179
Make logging macros project-specific
vyasr 853b8a1
Add README
vyasr 92d5e21
Define default logging level
vyasr 1ca5e8d
Try removing the exclude-libs
vyasr 7bd2086
Compile spdlog with fpic
vyasr 7928515
Make sure spdlog is available for compiling tests that need it.
vyasr 6d6a4e4
Fix test that needs spdlog
vyasr cc2c1f9
Remove redundant flag
vyasr 0985006
Always set fpic
vyasr 3ca0f10
Also set up linkage for PTDS tests
vyasr 46e650b
Also compile the logger itself with fpic
vyasr b450cfb
Remove unnecessary inline.
vyasr dd906e0
Switch to using bundled fmt
vyasr aed8320
Also set interface_position_independent_code
vyasr f5830c7
Fix title name
vyasr 7020b9b
Don't fail on symbol errors to see if CI passes
vyasr e068335
Don't use CMAKE_INSTALL_PREFIX
vyasr 4520ccb
Add support for callbacks
vyasr 0abd94c
Remove dynamic spdlog
vyasr fe19b2b
Remove support for building without logging support
vyasr 2ffc827
Go back to checking symbols
vyasr 2731a53
Switch back to header-only spdlog
vyasr 0a7b337
Revert "Add support for callbacks"
vyasr 4f6b98c
Revert "Go back to checking symbols"
vyasr 02212f1
Make sure symbols are not exported (required on both compile and link…
vyasr 4e7eba7
Put back symbol check, adapted for new functions
vyasr 2fcfc70
Remove unnecessary link option
vyasr 49368cf
Add notes
vyasr 46786c6
Full PImpl for the logger
vyasr 8f6d03c
Remove now superfluous function supports_logging
vyasr f88e65a
Hide more in anonymous namespace
vyasr 82443bd
Create a sink_impl
vyasr 483bdfc
Implement a sink_vector as the primary intermediary for sinks.
vyasr 316b1f9
Comment
vyasr a6bc3a7
Convert the underlying logger to a stack-allocated one.
vyasr 3cf1074
Remove one unnecessary friend
vyasr 155c934
Remove one more friend
vyasr b175d68
Make things consistent so that the sink_vector is the only friend tha…
vyasr 9619e51
Add documentation
vyasr fa3b95e
Bugfix
vyasr e1fc36a
Inline init_logger
vyasr c5f56fd
Fix some comments
vyasr 15697c7
Fixes for tracking_mr test
vyasr d12bb93
Fix sinks definition
vyasr 2d84f2d
Remove support for mixing in spdlog sinks and update tests accordingly.
vyasr e7b2d92
Remove lingering spdlog include
vyasr b6af231
Update symbol checking script, we should no longer leak any of these …
vyasr 82f925a
Remove visibility macro configurability from public header since we a…
vyasr 6dfc38a
Try using macros
vyasr cae6784
Make sure that internally needed symbols are exported
vyasr 6bbc174
Fix sink_ptr in docs
vyasr a21dedc
Add comments
vyasr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,32 +47,13 @@ for dso_file in ${dso_files}; do | |
echo " * WEAK: $(grep --count -E ' WEAK ' < ${symbol_file})" | ||
echo " * LOCAL: $(grep --count -E ' LOCAL ' < ${symbol_file})" | ||
|
||
# Explanation for '-v' uses here: | ||
# | ||
# * 'format_error' symbols are intentionally exported, that type of error | ||
# can be thrown across library boundaries. See "Problems with C++ exceptions" | ||
# at https://gcc.gnu.org/wiki/Visibility. | ||
echo "checking for 'fmt::' symbols..." | ||
if grep -E 'fmt\:\:' < "${symbol_file}" \ | ||
| grep -v 'format_error' | ||
then | ||
if grep -E 'fmt\:\:' < "${symbol_file}"; then | ||
raise-symbols-found-error 'fmt::' | ||
fi | ||
|
||
# Explanation for '-v' uses here: | ||
# | ||
# * trivially-destructible objects sometimes get an entry in the symbol table | ||
# for a specialization of `std::_Destroy_aux()` called to destroy them. | ||
# There is one for `spdlog::details::log_msg_buffer like that: | ||
# | ||
# 'std::_Destroy_aux<false>::__destroy<spdlog::details::log_msg_buffer*>' | ||
# | ||
# That should be safe to export. | ||
# | ||
echo "checking for 'spdlog::' symbols..." | ||
if grep -E 'spdlog\:\:' < "${symbol_file}" \ | ||
| grep -v 'std\:\:_Destroy_aux' | ||
then | ||
if grep -E 'spdlog\:\:' < "${symbol_file}"; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (reviewing after merge just to be sure I understand what's happening) Love that hiding these symbols also reduced the complexity of these checks! 🎉 |
||
raise-symbols-found-error 'spdlog::' | ||
fi | ||
echo "No symbol visibility issues found" | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since each logger produced by rapids-logger gets its own set of logger macros controls, rmm's logging level is now decoupled from spdlog's.