Add logging abstraction layer for glog deprecation#614
Open
yurekami wants to merge 2 commits intouccl-project:mainfrom
Open
Add logging abstraction layer for glog deprecation#614yurekami wants to merge 2 commits intouccl-project:mainfrom
yurekami wants to merge 2 commits intouccl-project:mainfrom
Conversation
Introduces include/util/logging.h that provides glog-compatible macros (LOG, VLOG, CHECK, DCHECK) with a lightweight fallback implementation. This enables gradual migration away from glog dependency: - When USE_GLOG is defined: uses glog as before - Otherwise: uses minimal stderr-based logging Updated key headers to use the new abstraction: - include/util/util.h - include/util/timer.h - include/util/net.h - include/util/shared_pool.h Addresses uccl-project#295 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Member
|
Hi @yurekami , thank you for the contribution! We actually have some plans on this and would like a similar interface to NCCL's one: https://github.com/NVIDIA/nccl/blob/master/src/debug.cc. Would you like to convert to NCCL's APIs and implementation? |
Contributor
Author
|
Thanks for the pointer to NCCL's debug.cc! I'll study that implementation and revise this PR to align with NCCL's logging interface. This would provide better consistency for users familiar with NCCL debugging. |
Revise logging abstraction to match NCCL's logging interface per maintainer feedback. The new implementation: - Uses ncclDebugLogger_t function pointer passed via pluginInit() - Provides INFO(flags, fmt, ...), WARN(fmt, ...), TRACE(flags, ...) macros - Defines subsystem flags (UCCL_INIT, UCCL_NET, UCCL_P2P, UCCL_ALL) - Includes fallback logger for stderr when NCCL logger unavailable - Updates RDMA plugin as reference implementation This aligns with NCCL's debug.cc interface as requested: https://github.com/NVIDIA/nccl/blob/master/src/debug.cc 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Author
|
Thanks for the feedback @YangZhou1997! I've revised the implementation to use NCCL-style logging API. Changes
ncclResult_t pluginInit(ncclDebugLogger_t logFunction) {
uccl::logging::initLogger(logFunction);
INFO(UCCL_INIT, "UCCL RDMA plugin initialized, PID: %d", getpid());
...
}The EFA and afxdp plugins can be migrated similarly in follow-up PRs. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
include/util/logging.hthat provides glog-compatible macros (LOG,VLOG,CHECK,DCHECK,CHECK_NOTNULL) with a lightweight fallback implementationUSE_GLOGis defined: uses glog as beforeUCCL_VLOG_LEVELenv var for verbose level controlUpdated key headers to use the new abstraction:
include/util/util.hinclude/util/timer.hinclude/util/net.hinclude/util/shared_pool.hTest plan
USE_GLOGdefined - should use glog as beforeUSE_GLOG- should use lightweight fallbackUCCL_VLOG_LEVELenvironment variable controls verbose loggingAddresses #295
🤖 Generated with Claude Code