-
Notifications
You must be signed in to change notification settings - Fork 915
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
Add logging to libcudf #12637
Merged
Merged
Add logging to libcudf #12637
Changes from 54 commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
60f6157
remove unused function
vuule a888ac8
logger
vuule 5108371
log env var reads
vuule b2cc531
remove repeated env var reads of PREFER_PAGEABLE_TMP_MEMORY
vuule d7ecd33
copyright year
vuule 830ab00
docs
vuule a4c51c0
yaml
vuule d029e1e
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 3d7c2ca
move impl to cpp
vuule cbda9d0
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 87c5d3a
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 0df8e97
anon namespace
vuule 92f53f9
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 36004e5
tests mostly
vuule 0a90a8f
style
vuule 0e95828
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 498ac21
use macro
vuule f0ae3e2
fixture
vuule 680f425
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 6ab194c
separate public logger API from logging macros
vuule 404fa1d
Merge branch 'branch-23.04' into fea-logger
vuule bc12b13
guide start
vuule 38da9b5
Merge branch 'fea-logger' of https://github.com/vuule/cudf into fea-l…
vuule c736164
header docs
vuule b0ec318
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule bae202c
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule df803a4
dev guide
vuule d6b8145
clean up
vuule e813cb9
build changes
vuule b1f0994
style
vuule c1ceb73
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 30fcd28
Apply suggestions from code review
vuule af09276
Merge branch 'fea-logger' of https://github.com/vuule/cudf into fea-l…
vuule 588d6f5
remove GCC workaround
vuule 0c24717
fix string constant
vuule 5f090e2
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 6fea0bf
update default level, sink
vuule c77426d
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule 5581c8a
switch to stderr
vuule 6378bf7
missing doc
vuule 553a57b
docs improvement
vuule 49f3896
remove outdated docs
vuule 0b6b27c
update dev guide
vuule a540928
small fixes
vuule dcd3b29
style
vuule e49f007
Merge branch 'branch-23.04' into fea-logger
vuule fd2636f
Merge branch 'branch-23.04' of https://github.com/rapidsai/cudf into …
vuule c75ff33
cmake changes
vuule a8c3e11
conda changes
vuule 57f6d05
Merge branch 'fea-logger' of https://github.com/vuule/cudf into fea-l…
vuule eb045f7
style
vuule 2280ecf
doc improvement
vuule e7fe0cd
docs that killed timezone refactor
vuule 06893a3
Merge branch 'branch-23.04' into fea-logger
vuule 1d1950c
Update cpp/CMakeLists.txt
vuule 855fcc0
Merge branch 'branch-23.04' into fea-logger
vuule 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 |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# ============================================================================= | ||
# Copyright (c) 2023, NVIDIA CORPORATION. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except | ||
# in compliance with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software distributed under the License | ||
# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express | ||
# or implied. See the License for the specific language governing permissions and limitations under | ||
# the License. | ||
# ============================================================================= | ||
|
||
# Use CPM to find or clone fmt | ||
function(find_and_configure_fmt) | ||
|
||
include(${rapids-cmake-dir}/cpm/fmt.cmake) | ||
rapids_cpm_fmt(INSTALL_EXPORT_SET cudf-exports BUILD_EXPORT_SET cudf-exports) | ||
endfunction() | ||
|
||
find_and_configure_fmt() |
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 |
---|---|---|
@@ -0,0 +1,34 @@ | ||
# ============================================================================= | ||
# Copyright (c) 2023, NVIDIA CORPORATION. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except | ||
# in compliance with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software distributed under the License | ||
# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express | ||
# or implied. See the License for the specific language governing permissions and limitations under | ||
# the License. | ||
# ============================================================================= | ||
|
||
# Use CPM to find or clone speedlog | ||
function(find_and_configure_spdlog) | ||
|
||
include(${rapids-cmake-dir}/cpm/spdlog.cmake) | ||
rapids_cpm_spdlog(FMT_OPTION "EXTERNAL_FMT_HO" INSTALL_EXPORT_SET cudf-exports) | ||
rapids_export_package(BUILD spdlog cudf-exports) | ||
vyasr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if(spdlog_ADDED) | ||
rapids_export( | ||
BUILD spdlog | ||
EXPORT_SET spdlog | ||
GLOBAL_TARGETS spdlog spdlog_header_only | ||
NAMESPACE spdlog:: | ||
) | ||
include("${rapids-cmake-dir}/export/find_package_root.cmake") | ||
rapids_export_find_package_root(BUILD spdlog [=[${CMAKE_CURRENT_LIST_DIR}]=] cudf-exports) | ||
endif() | ||
endfunction() | ||
|
||
find_and_configure_spdlog() |
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 |
---|---|---|
@@ -0,0 +1,27 @@ | ||
/* | ||
* Copyright (c) 2023, NVIDIA CORPORATION. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#pragma once | ||
|
||
#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__) |
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 |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* Copyright (c) 2023, NVIDIA CORPORATION. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#pragma once | ||
|
||
#include <spdlog/spdlog.h> | ||
|
||
namespace cudf { | ||
|
||
/** | ||
* @brief Returns the global logger. | ||
* | ||
* This is a global instance of a spdlog logger. It can be used to configure logging behavior in | ||
* libcudf. | ||
* | ||
* Examples: | ||
* @code{.cpp} | ||
* // Turn off logging at runtime | ||
* cudf::logger().set_level(spdlog::level::off); | ||
* // Add a stdout sink to the logger | ||
* cudf::logger().sinks().push_back(std::make_shared<spdlog::sinks::stdout_sink_mt>()); | ||
* // Replace the default sink | ||
* cudf::logger().sinks() ={std::make_shared<spdlog::sinks::stderr_sink_mt>()}; | ||
* @endcode | ||
* | ||
* Note: Changes to the sinks are not thread safe and should only be done during global | ||
* initialization. | ||
* | ||
* @return spdlog::logger& The logger. | ||
*/ | ||
spdlog::logger& logger(); | ||
|
||
} // namespace cudf |
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
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.
Should logging levels of RMM and cuDF be independently controlled?
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.
Assuming they do - how do we handle setting
SPDLOG_ACTIVE_LEVEL
? RMM sets it based on RMM_LOGGING_LEVEL, and we need to set it based onLIBCUDF_LOGGING_LEVEL
.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.
Ah. Good point. I don't have feelings on this, we can keep it all the same.
I think spdlog may have multiple ways to set the logging level, and compile-time options are only one of the methods. It would be hard to ensure that debug messages are compiled out while retaining runtime control over two different loggers. 😖
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.
Yes, the logging level can be set the runtime. This is what we use to set the level if
LIBCUDF_LOGGING_LEVEL
env var is set.The CMake config var can be seen as minimal logging level that the build includes, since it literally undefines logging macros for level below its value (which is great for security/perf). AFAIK, the logging level for both libcudf will stay at INFO (or whatever is the default) even if CMake config var is TRACE/DEBUG.
I'm wary of having different SPDLOG_ACTIVE_LEVEL values in RMM and libcudf as it feels like possible ODR violation 🤷
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.
I don't think you would run into any issues. RMM only sets this for compiled components i.e. its tests and benchmarks. The main rmm code is header-only so there are no compiled components where
SPDLOG_ACTIVE_LEVEL
would be set to conflict with whatever libcudf sets.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.
Thinking about this more we might need to make this PRIVATE.
If a downstream consumer uses cudf and rmm they can't specify an RMM logging level since we are specifying one already.
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.
Great point, yeah that makes sense. I believe RMM sets this PUBLIC, but again it's only done in tests and benchmarks so it probably doesn't matter.