-
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
Adding JSON paths to FB ES module docs #12008
Conversation
Pinging @elastic/stack-monitoring |
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.
Made some minor suggestions. I made them to all instances (including generated) so that you can accept them all though GitHub, if you want, but I'm not sure this will pass CI. You might need to pull down the changes and run make update
again.
Co-Authored-By: ycombinator <[email protected]>
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.
Wait until docs CI test is green before merging, but the changes LGTM.
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.
Thank you for this, @ycombinator. LGTM, I left just two questions below.
@@ -70,8 +75,14 @@ Example config: | |||
---- | |||
audit: | |||
var.paths: | |||
- /var/log/elasticsearch/*_audit.json | |||
- /var/log/elasticsearch/*_access.log # Plain text logs | |||
- /var/log/elasticsearch/*_audit.log # JSON logs |
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 really in JSON format a the comment suffix suggests?
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, this was the case in 6.7. Starting 7.0 the JSON audit logs were put into *_audit.json
. I think, to avoid confusion and complexity, I'll just remove this line altogether.
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.
Removed in f062d3e.
@@ -65,8 +70,14 @@ Example config: | |||
---- | |||
audit: | |||
var.paths: | |||
- /var/log/elasticsearch/*_audit.json | |||
- /var/log/elasticsearch/*_access.log # Plain text logs | |||
- /var/log/elasticsearch/*_audit.log # JSON logs |
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.
Same as above - does this file contain JSON-formatted logs?
I addressed your review feedback, @weltenwort. This is ready for re-review, when you get a chance. Thanks! |
* Adding JSON paths to FB ES module docs * Adding note about ES version * Apply suggestions from code review Co-Authored-By: ycombinator <[email protected]> * Remove mention of *_audit.log to avoid confusion
* Adding JSON paths to FB ES module docs * Adding note about ES version * Apply suggestions from code review Co-Authored-By: ycombinator <[email protected]> * Remove mention of *_audit.log to avoid confusion
…#12030) * Adding JSON paths to FB ES module docs * Adding note about ES version * Apply suggestions from code review Co-Authored-By: ycombinator <[email protected]> * Remove mention of *_audit.log to avoid confusion
…#12031) * Adding JSON paths to FB ES module docs * Adding note about ES version * Apply suggestions from code review Co-Authored-By: ycombinator <[email protected]> * Remove mention of *_audit.log to avoid confusion
Resolves #12000.
Clarifies what log file paths to point to for the Filebeat Elasticsearch module.