Skip to content
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

Control topic handling. #167

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

Henry-Denny
Copy link
Member

@Henry-Denny Henry-Denny commented May 2, 2024

This work is a continuation of #165 and aims to close #113. It gives the trace-archiver-hdf5 the ability to consume a control topic, and use this to handle when to start and stop the recording of trace data to an HDF file.

Henry-Denny and others added 22 commits May 1, 2024 11:23
Co-authored-by: Dan Nixon <dan@dan-nixon.com>
Co-authored-by: Dan Nixon <dan@dan-nixon.com>
This is so that there is less ambiguity when the `control_topic` command
line argument is added later.
In the case the control topic is not provided, the tool should use the
default behaviour.
Uses the Unix epoch timestamp milliseconds as the filenames. This could
potentially be problematic if two (or more) runs are started less than a
millisecond apart. Should look for a better way of generating filename
UIDs.
@Henry-Denny Henry-Denny marked this pull request as ready for review May 20, 2024 11:15
trace-archiver-hdf5/README.md Outdated Show resolved Hide resolved
trace-archiver-hdf5/src/main.rs Outdated Show resolved Hide resolved
trace-archiver-hdf5/run.hdf5 Outdated Show resolved Hide resolved
trace-archiver-hdf5/README.md Outdated Show resolved Hide resolved
trace-archiver-hdf5/src/main.rs Outdated Show resolved Hide resolved
trace-archiver-hdf5/src/main.rs Outdated Show resolved Hide resolved
trace-archiver-hdf5/src/main.rs Outdated Show resolved Hide resolved
debug!("New run start.");
// Start recording trace data to file.
if file.is_none() {
let filename = generate_filename(msg.timestamp());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timestamp from the run start message should be used here, not the Kafka timestamp.

Henry-Denny and others added 3 commits June 6, 2024 14:05
Co-authored-by: Dan Nixon <dan@dan-nixon.com>
Co-authored-by: Dan Nixon <dan@dan-nixon.com>
This is to combat the issue where the `file` command line argument was
required regardless of whether it was actually needed (for example if a
control topic is provided then the files are just saved under the
timestamp).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trace saving solution
2 participants