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

feat(cli): provide human readable reporting in console output #567

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

qkaiser
Copy link
Contributor

@qkaiser qkaiser commented Apr 25, 2023

Provide some information about identified chunks, extracted content, and encountered errors. All the information is obtained from the ProcessResult object.

Example:

poetry run unblob --report /tmp/report.json -f -e /tmp/out sample.img
Extracted files: 3616
Extracted directories: 609
Extracted links: 782
Extraction directory size: 298.19 MB.
Chunks identification ratio: 90.74%
Chunks distribution
	- EXTFS: 150.00 MB (54.17%)
	- ELF32: 73.98 MB (26.72%)
	- UNKNOWN: 25.64 MB (9.26%)
	- FAT: 16.00 MB (5.78%)
	- LZO: 9.28 MB (3.35%)
	- XZ: 1.03 MB (0.37%)
	- TAR: 860.00 KB (0.30%)
	- BZIP2: 93.56 KB (0.03%)
	- GZIP: 24.21 KB (0.01%)	
Encountered errors: 1
	- Severity.WARNING: MaliciousSymlinkRemoved

This code was sitting on my machine for the last month. I figured I would push it to start a discussion 😄

@qkaiser qkaiser marked this pull request as draft April 25, 2023 19:47
@qkaiser qkaiser self-assigned this Apr 25, 2023
@qkaiser qkaiser added the enhancement New feature or request label Apr 25, 2023
Copy link
Contributor

@e3krisztian e3krisztian left a comment

Choose a reason for hiding this comment

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

This is cool, and it can/should be merged!

@qkaiser qkaiser marked this pull request as ready for review April 26, 2023 17:15
@qkaiser
Copy link
Contributor Author

qkaiser commented Apr 26, 2023

I will use it as my default branch over the coming days to see if anything breaks. I think I've seen a division by zero earlier, probably a good thing to write some tests thinking about unhappy paths.

@vlaci
Copy link
Contributor

vlaci commented Apr 26, 2023

Instead of just restricting the default log level, it'd make sens to write a log file next to the extraction with a bit higher verbosity. I'd keep the console as clear as possible. My dream is to add some more intelligible progress reporting to the console via rich and-or promp-poolkit progress bars.

@qkaiser
Copy link
Contributor Author

qkaiser commented Apr 27, 2023

Instead of just restricting the default log level, it'd make sens to write a log file next to the extraction with a bit higher verbosity.

How would you do it ?

My dream is to add some more intelligible progress reporting to the console via rich and-or promp-poolkit progress bars.

Me too !

@vlaci
Copy link
Contributor

vlaci commented Apr 27, 2023

Instead of just restricting the default log level, it'd make sens to write a log file next to the extraction with a bit higher verbosity.

How would you do it ?

Okay, maybe I've missed something, but it seems like it is really not obvious how to do with structlog...

@qkaiser qkaiser force-pushed the 246-human-readable-report branch 3 times, most recently from 7807d84 to 98ebddb Compare May 26, 2023 19:17
@qkaiser
Copy link
Contributor Author

qkaiser commented May 26, 2023

I introduced logging to a file and a progress bar using rich when the -v option is not used.

The progress bar runs in the main thread and updates the total and completed value based on submitted and completed tasks of the worker pool. Yes, the progress bar can go back but that's how it is. We can't predict what we will encounter down the extraction directory.

There are a few open UI/UX questions:

  • how well does rich and logging play together ? It seems it breaks the console when I used the progress bar.
  • how should we name the logging file ?
  • should we always create the logging file ? should it be linked to an option switch ?

@qkaiser qkaiser force-pushed the 246-human-readable-report branch 3 times, most recently from de81242 to 2f5e41a Compare May 30, 2023 08:25
@vlaci
Copy link
Contributor

vlaci commented Jun 7, 2023

If we route structlog logs to standard logging handlers, we can add different levels to different output streams:
https://github.com/onekey-sec/unblob/pull/597/files#diff-9debb99c720bedc5736dc89eab3987d1e9b3c2ebce05997fb6e79f69b0781b79R122
here, an additional handler could be defined which writes to a file, and the console handler's level could be restricted.

@qkaiser qkaiser force-pushed the 246-human-readable-report branch 3 times, most recently from d3058bf to 8365ebf Compare August 8, 2023 10:10
unblob/logging.py Outdated Show resolved Hide resolved
unblob/cli.py Outdated Show resolved Hide resolved
unblob/cli.py Outdated Show resolved Hide resolved
unblob/logging.py Outdated Show resolved Hide resolved
unblob/models.py Outdated Show resolved Hide resolved
@qkaiser qkaiser force-pushed the 246-human-readable-report branch 3 times, most recently from 2edcffd to 1687d18 Compare August 11, 2023 10:37
@e3krisztian e3krisztian self-requested a review August 11, 2023 14:09
Provide some information about identified chunks, extracted content, and
encountered errors. All the information is obtained from the
ProcessResult object.

Example:

----
poetry run unblob --report /tmp/report.json -f -e /tmp/out sample.img
Extracted files: 3616
Extracted directories: 609
Extracted links: 782
Extraction directory size: 298.19 MB.
Chunks identification ratio: 90.74%
Chunks distribution
	- EXTFS: 150.00 MB (54.17%)
	- ELF32: 73.98 MB (26.72%)
	- UNKNOWN: 25.64 MB (9.26%)
	- FAT: 16.00 MB (5.78%)
	- LZO: 9.28 MB (3.35%)
	- XZ: 1.03 MB (0.37%)
	- TAR: 860.00 KB (0.30%)
	- BZIP2: 93.56 KB (0.03%)
	- GZIP: 24.21 KB (0.01%)
Encountered errors: 1
	- Severity.WARNING: MaliciousSymlinkRemoved
----
@qkaiser qkaiser merged commit 5f8687f into main Aug 11, 2023
11 checks passed
@qkaiser qkaiser deleted the 246-human-readable-report branch August 11, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants