-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Filebeat: Elasticsearch module: Audit log #7365
Conversation
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.
Left a few minor comments.
Could you update the CHANGELOG and add 1 small test log file to the test directory? It will be picked up by the tests automatically. (see other metricsets). We can add an expected event also later.
type: group | ||
description: > | ||
fields: | ||
- name: timestamp |
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 this timestamp still needed?
fields: | ||
- name: timestamp | ||
description: Please add description | ||
example: Please add example |
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 update descriptions and potential examples if needed.
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.
Left some minor comments. I suggest we get the PR in an open a follow up PR to discuss the field changes.
You have to rebase again.
type: group | ||
description: > | ||
fields: | ||
- name: node_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.
Is this the Elasticsearch node name? If yes, this field should end up under elasticsearch.node.name
. Like this we will be able to correlate it.
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 am investigating here a bit more, because we have several options and more fields can show up. I need to run different configurations to follow up on behaviour and best approach. I will keep in mind that this field could be moved.
- name: origin_address | ||
description: "The IP address from which the request originated" | ||
example: "192.168.1.42" | ||
type: keyword |
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.
Should this be of type ip?
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.
Yeah, this can be an IP.
description: "Where the request originated: rest (request originated from a REST API request), transport (request was received on the transport channel), local_node (the local node issued the request)" | ||
example: "local_node" | ||
type: keyword | ||
- name: origin_address |
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.
So this is the ip of the machine on which for example an Elasticsearch client is running which makes requests?
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.
Yes
description: "The REST endpoint URI" | ||
example: /_xpack/security/_authenticate | ||
type: keyword | ||
- name: request |
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.
request
could be confusing as I would expect more data to be inside. Perhaps this is request_type
?
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 followed the documentation names for all of the fields from Auditing.
8342cf1
to
143384f
Compare
…ct of modules hierarchy
Remove unnecesary timestamp from event Improve Grok pattern and add new field to document Add test file and expected JSON Add details to fields description
26950a7
to
db37ed3
Compare
@ruflin - which versions was this merged into? Just master? Should we update the labels to reflect? |
@skearns64 This ended up in 6.4. At this time master and 6.4 were still identical and is why there are not backports or labels. Not sure it is worth adding labels as it applies to all 6.4 features. Best is to trust the changelog (in most cases ;-) ). |
This is initial PR for Audit logs in Elasticsearch. It successfully index basic events. Following PR's will improve grok patterns and add files for testing. Dashboards will come as bonus.