feat: Add ZeroMQ support for DFTracer#267
Draft
izzet wants to merge 39 commits into
Draft
Conversation
izzet
commented
May 27, 2025
Collaborator
- Updated CMakeLists.txt to include ZeroMQ and cppzmq dependencies based on the writer type.
- Introduced DFTRACER_WRITER_TYPE_ENV constant for environment variable configuration.
- Refactored metadata handling to use MetadataMap type instead of std::unordered_map.
- Enhanced DFTracer and DFTLogger classes to accommodate new metadata structure.
- Implemented ZeroMQWriter class for sending log messages over ZeroMQ.
- Created a base WriterBase class to standardize logging interfaces for different writers.
- Updated ChromeWriter to utilize the new base class and refactored JSON conversion methods.
- Added configuration management for writer type selection via environment variables.
- Updated CMakeLists.txt to include ZeroMQ and cppzmq dependencies based on the writer type. - Introduced DFTRACER_WRITER_TYPE_ENV constant for environment variable configuration. - Refactored metadata handling to use MetadataMap type instead of std::unordered_map. - Enhanced DFTracer and DFTLogger classes to accommodate new metadata structure. - Implemented ZeroMQWriter class for sending log messages over ZeroMQ. - Created a base WriterBase class to standardize logging interfaces for different writers. - Updated ChromeWriter to utilize the new base class and refactored JSON conversion methods. - Added configuration management for writer type selection via environment variables.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces ZeroMQ support for DFTracer along with a refactoring of logging writers to inherit from a new WriterBase interface and use a unified MetadataMap type for metadata management. Key changes include the addition of the ZeroMQWriter class, updates to configuration management and build files for ZeroMQ dependencies, and refactoring of ChromeWriter, DFLogger, and DFTracerCore to use the new metadata and writer type configuration.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/dftracer/writer/zeromq_writer.cpp | Adds ZeroMQWriter implementation and integrates ZeroMQ logging functionality. |
| src/dftracer/writer/writer_base.h | Introduces a common base class for logging writers. |
| src/dftracer/writer/writer_base.cpp | Implements shared JSON conversion functions for events and metadata. |
| src/dftracer/writer/chrome_writer.{h,cpp} | Refactors ChromeWriter to inherit from WriterBase and adapt to MetadataMap. |
| src/dftracer/utils/configuration_manager.{h,cpp} | Adds writer_type configuration via environment variables. |
| src/dftracer/{dftracer.cpp,df_logger.h,core/dftracer_main.{h,cpp}} | Updates to use MetadataMap for metadata handling. |
| include/dftracer/core/typedef.h | Defines MetadataMap as a type alias for metadata handling. |
| CMakeLists.txt and related cmake modules/configure files | Update dependencies and build configuration for ZeroMQ support. |
Comments suppressed due to low confidence (1)
src/dftracer/writer/chrome_writer.cpp:60
- [nitpick] The initialization and subsequent update of 'is_first_write' seems inconsistent with its intended use for formatting; please verify that the starting value and update logic correctly reflect the expected behavior for the first log entry.
is_first_write = false;
Collaborator
Author
|
Addresses #78 |
- Introduced `PerfettoChromeFileWriter` for logging events in Perfetto format. - Added `PerfettoProtoFileWriter` to handle ProtoBuf-based logging. - Implemented `PerfettoProtoZMQWriter` for ZeroMQ integration with Perfetto. - Removed the legacy `ZeroMQWriter` and `writer_base` implementations. - Updated test cases to utilize the new `PerfettoChromeFileWriter`. - Adjusted CMake configuration to conditionally compile tests based on writer type. - Enhanced logging and error handling in the new writers.
…s and improve clarity
- Updated CMakeLists.txt to include new writer type options for PERFETTO_CHROME_ZMQ. - Modified configuration header files to define the new writer type. - Adjusted dependency management to handle ZeroMQ when the new writer type is selected. - Enhanced dftracer core to initialize and manage the new PERFETTO_CHROME_ZMQ writer. - Created new writer class PerfettoChromeZMQWriter that inherits from PerfettoChromeWriterBase. - Implemented buffer flushing and message sending logic for the new writer using ZeroMQ. - Updated logging and metadata handling in the new writer class. - Refactored existing PerfettoChromeFileWriter to share common functionality with the new writer base class.
…n CMake configuration
e885d47 to
c3b1992
Compare
- Add automatic fabric protocol detection (CXI/TCP) for mofka server startup - Implement PID-based process tracking for reliable server lifecycle management - Update CMake to use Spack environment paths for LD_LIBRARY_PATH and PYTHONPATH - Consolidate environment variable settings in set_common_properties function
Key changes: - Add ZMQWriter class with non-blocking PUSH socket implementation - Handle fork() safely with automatic socket reconnection in child processes - Add CMake configuration for ZMQ dependencies (libzmq, cppzmq) - Add test infrastructure with ZMQ sink process for integration testing
…or Mofka and ZMQ writers
Detect fork via pid mismatch and rebuild Mofka driver/producer state in child processes. This allows PyTorch DataLoader workers (forked from the main process) to send POSIX I/O events through Mofka mid-run. Previously, forked children inherited stale Mofka connections that silently failed, so only main-process events were captured.
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.