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: add log-metrics command feature #1484

Merged
merged 9 commits into from
Nov 12, 2024
Merged

Conversation

fukusuket
Copy link
Collaborator

@fukusuket fukusuket commented Nov 12, 2024

@fukusuket fukusuket added the enhancement New feature or request label Nov 12, 2024
@fukusuket fukusuket added this to the 2.19.0 milestone Nov 12, 2024
@fukusuket fukusuket self-assigned this Nov 12, 2024
@fukusuket
Copy link
Collaborator Author

fukusuket commented Nov 12, 2024

I created prototype! The following features have not yet been implemented🤔

  • Timeformat
  • Filtering
  • Abbreviation

@YamatoSecurity
I would appreciate your comments when you have time🙏(Sort order of output results, column order, ... etc)

Terminal Output

スクリーンショット 2024-11-12 12 31 51

CSV Output

スクリーンショット 2024-11-12 12 33 31 スクリーンショット 2024-11-12 12 33 26

@fukusuket
Copy link
Collaborator Author

fukusuket commented Nov 12, 2024

The following three issues may be better separated as separate issues and will be addressed in a separate pull request.

@YamatoSecurity
Copy link
Collaborator

@fukusuket Thanks! Looking good. Just two things about the table besides what you mentioned.

  1. Can you rename "Event Count" to just "Events" because this column title is long it is making the column width wider than necessary.
  2. Can you separate the Computers, Channels and Providers by newline instead of commas when there are multiple results? (Make them multi-lined) I think this will be easier to read than separating by commas as well as might make the width narrower.

@YamatoSecurity
Copy link
Collaborator

When writing to CSV file, maybe we should separate by the broken pipe character like we do for Hayabusa results?

@YamatoSecurity
Copy link
Collaborator

One more bug I noticed is that the progress bar does not reach 100% for some reason.
Screenshot 2024-11-12 at 15 51 42

@fukusuket fukusuket marked this pull request as ready for review November 12, 2024 09:53
@fukusuket
Copy link
Collaborator Author

fukusuket commented Nov 12, 2024

@YamatoSecurity
Thank you for comment! I updated following points! Could you check it?🙏

Also, I implemented Timeformat/Filter!

@fukusuket
Copy link
Collaborator Author

I created #1484 (comment) issue as separate one.

@YamatoSecurity
Copy link
Collaborator

@fukusuket Thanks! It seems that --timeline-offset is not working. Can you check this?

@YamatoSecurity
Copy link
Collaborator

Sorry, just a few small things.

  1. Can you change the ¦ separator to ¦ with spaces around it?
  2. Does the abbreviations first check the channel_abbreviations.txt file before generic_abbreviations.txt? I see MS-Win-PwrShell/Op but it should be PwSh/Op. Also, provider abbreviations should first check provider_abbreviations.txt and then after that use generic_abbreviations.txt.
  3. Could you add a -M, --multiline Output event field information in multiple rows under Output that uses newlines instead of ¦ like we do for csv-timeline?

@fukusuket
Copy link
Collaborator Author

fukusuket commented Nov 12, 2024

@YamatoSecurity
Thank you for checking! I fixed following four points! Could you check it?🙏

It seems that --timeline-offset is not working. Can you check this?

  1. Can you change the ¦ separator to ¦ with spaces around it?
  2. Does the abbreviations first check the channel_abbreviations.txt file before generic_abbreviations.txt? I see MS-Win-PwrShell/Op but it should be PwSh/Op. Also, provider abbreviations should first check provider_abbreviations.txt and then after that use generic_abbreviations.txt.
  3. Could you add a -M, --multiline Output event field information in multiple rows under Output that uses newlines instead of ¦ like we do for csv-timeline?

Copy link
Collaborator

@YamatoSecurity YamatoSecurity left a comment

Choose a reason for hiding this comment

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

@fukusuket LGTM! Thanks so much!

@YamatoSecurity YamatoSecurity merged commit 730aec8 into main Nov 12, 2024
9 checks passed
@fukusuket fukusuket deleted the 1474-log-metrics-command branch November 12, 2024 12:31
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.

log-metrics command
2 participants