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

Feature: dump current state to logfile #123

Open
imsnif opened this issue Jan 16, 2020 · 7 comments
Open

Feature: dump current state to logfile #123

imsnif opened this issue Jan 16, 2020 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@imsnif
Copy link
Owner

imsnif commented Jan 16, 2020

Sometimes when troubleshooting a network issue, it might be useful to log all the information the tool currently has in a log file for parsing and perhaps a more thorough look than can fit in the UI.

Note that this should work both for when paused and not paused.

A good key binding might be "d", but other ideas are welcome as well.

The format for dumping to a file should be the same one we use for "raw_mode".

@imsnif imsnif added help wanted Extra attention is needed enhancement New feature or request labels Jan 16, 2020
@chobeat
Copy link
Contributor

chobeat commented Jan 26, 2020

I've started looking into it. The first thing I wanted to do was to refactor the conversion of the state to a string and decouple it from the write to terminal. I did it sucesfully here: https://github.com/chobeat/bandwhich/tree/dump_state but the tests fail because there's a newline before every snapshot. My function behaves correctly and contains no newline. Before continuing I would like somebody to help me make sense of this. It's driving me crazy.

@imsnif
Copy link
Owner Author

imsnif commented Jan 26, 2020

Hey, cool that you're looking into this!
I think what's happening is that after this change, we still keep calling write_to_stdout even when the state is empty, which is not what was happening before. I think it can be solved by checking to see whether there are lines in the state and only printing it out if there aren't any.

@chobeat
Copy link
Contributor

chobeat commented Jan 26, 2020

ah, makes sense. Is it like because in the tests, the first state is empty? Gonna try it tomorrow anyway. Thank you.

@chobeat
Copy link
Contributor

chobeat commented Feb 17, 2020

Just to give a ping: It's taking me forever considering how easy the task is, but I'm slowly progressing.

@chobeat
Copy link
Contributor

chobeat commented Mar 4, 2020

I'm a bit stuck.

I've implemented the first draft of the feature (the file management is not really there yet). The code compiles but then it gets stuck here: https://github.com/chobeat/bandwhich/blob/dump_state/src/dump_mode/dump.rs#L36

The application seems to be freezing here and while I can have an intuition on what would be the problem, I don't have enough Rust expertise to know how to troubleshoot this issue.

I would like to ask you to give a partial review and tell me if the general approach makes sense or using the ui object like this is an error. I

@imsnif
Copy link
Owner Author

imsnif commented Mar 5, 2020

Hey @chobeat - I will try to give a quick guess regarding this and provide a suggestion, and if it doesn't make sense (or I'm completely off), let me know and I'll take a closer look in the next few days.

I think what happens here is a deadlock of the Arc. Generally, I try not to pass an Arc around to different parts of the application in order to avoid these deadlocks (or at least to make them simpler to figure out).

I have not looked at the rest of your code, so maybe this doesn't make sense at all, but my suggestion would be to control the unlocking of the Arc in main.rs and not pass it. Instead opting to just pass the state itself (or at worst, a clone of it). That way you don't need to worry about these deadlocks so much. Makes sense?

@chobeat
Copy link
Contributor

chobeat commented Mar 5, 2020

Yeah, that was my understanding too and originally I was passing around just the state, but I wanted to reuse the writing function. I will try to decouple the two things and pass only the state. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants