-
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
Add ECS logging MVP to libbeat #17974
Conversation
Pinging @elastic/integrations (Team:Integrations) |
libbeat/logp/config.go
Outdated
JSON bool `config:"json"` // Write logs as JSON. | ||
Level Level `config:"level"` // Logging level (error, warning, info, debug). | ||
Selectors []string `config:"selectors"` // Selectors for debug level logging. | ||
ECSEnabled bool `config:"ecs"` |
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.
we want to enable this setting by default?
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 would consider changing the default a breaking change, as it changes the log format.
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.
Do you mind adding a comment to describe what this option does?
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.
good point - added a comment
💔 Build FailedExpand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
Enable ECS conformant logging by setting `logging.ecs:true` Adds @timestamp, log.level, message and ecs.version to every log line. If EnableCaller is set add log.origin.file and log.origin.line. Logs error.message when error is given implements elastic#17942
Test failures are unrelated and also observed in beats |
@urso I just updated to the latest master, would appreciate a review. |
💔 Build FailedExpand to view the summary
Build stats
Test stats 🧪
Log outputExpand to view the last 100 lines of log output
|
The run on jenkins was green, but failed when reporting the results. |
Thank you both for the review. |
Enable minimal ECS conformant logging by setting `logging.ecs:true`: * adds @timestamp, log.level, message and ecs.version to every log line * if EnableCaller is set add log.origin.file and log.origin.line. * logs error.message when error is given implements elastic#17942
Enable minimal ECS conformant logging by setting `logging.ecs:true`: * adds @timestamp, log.level, message and ecs.version to every log line * if EnableCaller is set add log.origin.file and log.origin.line. * logs error.message when error is given implements #17942
@urso, @kvch this PR was backported to |
Hm, when do you plan to make it the default? At least when running via Agent we should have ECS enabled by default. |
Why experimental? If we mark the feature experimental, it implies (at least to me) that we might get rid of the feature or change it completely. But if I understand correctly this is the final format, we are just adding more fields in later releases. However, I agree we should flag it somehow. So I would rather go with beta. Unless of course, the this functionality is subject to change. |
My concern is that this adds a minimal set of ECS fields for now. It doesn't ensure that all log messages are ECS compatible, as this is still up to the implementer of the log messages. @kvch agreed, beta would be a better fit. I'd just like to avoid the assumption that as soon as you set |
++ Is it the default already when running under agent? If not, is there a follow-up issue tracking this? |
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.
LGTM for the minimal heartbeat changes
What does this PR do?
Enable ECS conformant logging by setting
logging.ecs:true
Adds @timestamp, log.level, message and ecs.version to every log line.
If EnableCaller is set add log.origin.file and log.origin.line.
Logs error.message when error is given
Why is it important?
Get started with ECS conformant logging
Checklist
- [ ] I have commented my code, particularly in hard-to-understand areasCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
Set different logging options (syslog, to_files, console) and enable
logging.json
andlogging.ecs
and check log lines contain@timestamp, message, log.level, ecs.version
. Logged errors are logged in the format{ "error": { "message": .. } }
. If caller is enabled it is logged aslog.origin.*
.manually tested for
logging.to_files: true
logging.to_syslog: true
logging.to_stderr: true
./beat -e
Related issues
implements #17942
Logs
Example: