-
Notifications
You must be signed in to change notification settings - Fork 75
Extend child-support #1133
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
Extend child-support #1133
Conversation
Schamper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a unit test?
And @JSCU-CNI not sure if the # key name breaks anything?
|
I think this could primarily cause trouble in processing dissect records further down the line. For example, Elasticsearch should work with special characters as field names (it is discouraged: https://www.elastic.co/docs/reference/ecs/ecs-guidelines), however I am not certain if those fields can then be easily queried using Elastic DSL or ES|QL.
It seems like Splunk does not allow special characters in field names: https://docs.splunk.com/Documentation/Splunk/latest/Data/Configureindex-timefieldextraction.
I think renaming |
|
The |
|
To expand on this topic, it would be nice if child information output (either using |
|
So i'm struggling a bit on how to do this in target-info. |
|
Latest commit adds "--list-children" and "--list-children-recursive" to
Example output for --list-children-recursive: Additional the
I currently do not have a working setup for all hypervisors so not al code has been tested to work on real (live) installations/systems. But all tox tests are currently passing. Todo:
|
|
@Zawadidone |
|
I don't have a test environment for Virtuozzo, but I think using the Thanks for the fix for Colima. |
|
Requesting review. |
CodSpeed Performance ReportMerging #1133 will not alter performanceComparing Summary
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1133 +/- ##
========================================
Coverage 80.63% 80.63%
========================================
Files 374 375 +1
Lines 33060 33221 +161
========================================
+ Hits 26658 26788 +130
- Misses 6402 6433 +31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@JSCU-CNI Any input on this would still be valuable. |
|
Thanks for the ping @lhaagsma. What input would you like from us specifically? Personally - from an end-user point of view - I would expect |
| ("datetime", "finished"), | ||
| ("string[]", "ports"), | ||
| ("string", "names"), | ||
| ("string", "name"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JSCU-CNI I changed this from names to name, seemed to be an oversight in naming since it's seemingly always a singular name, but let me know if I'm overlooking anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentionally called names since that is how Docker, Podman (and iirc other) container platforms tag their container name (see for example output of docker ps -a or podman ps -a).
https://docs.docker.com/reference/cli/docker/container/ls/#format
https://docs.podman.io/en/v5.0.2/markdown/podman-ps.1.html#format-format
From the dissect perspective I guess it makes sense to make the field singular if it is not a string[] field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that, but I think that's silly 😄. In the linked example of Docker there's a redis,webapp/db container names though, do you know how that occurs, and if we support that already?
c58556b to
2600cd2
Compare
2600cd2 to
cef20da
Compare
This PR adds a --list-children flag to target-query similar to fox-it/acquire#239.
Test output looks like this:
Additionally, added index to the target-info plugin.
Aims to close: #1132 and #1134