-
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
Recognize log level in Elasticsearch JVM logs #34159
Conversation
Elasticsearch will add a log level to its JVM logs to allow users to help them detect errors / warnings more easily. With this commit we detect this new field if present and continue to recognize the prior log format without a log level. Relates elastic/elasticsearch#92382 Closes elastic#34054
This pull request doesn't have a |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
There is one thing where I'm specifically looking for guidance from reviewers. In line with |
|
Lets make sure, any change we add here also makes it into the integrations package: https://github.com/elastic/integrations/tree/main/packages/elasticsearch/data_stream/gc |
Lee has already opened an issue for that: elastic/integrations#4842. :) |
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 think log.level
is the way to go, otherwise everything looks great.
Great, thanks for having a look @leehinman . Is this as simple as just changing the grok pattern from
to
or am I missing something? |
I've pushed the rename now in 24cf0bb. @leehinman can you please take another look? |
@danielmitterdorfer When will that work be done and by whom? My team is tagged for it but it's not planned on our part. |
@miltonhultgren I would hope it's not much effort once this change is in (basically copy & paste what's done here)? I'd like to see this change being released with 8.7.0. I can attempt to do this myself but will require some guidance. |
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
Let me know if you want help. |
@danielmitterdorfer @leehinman Okay, I had to confirm with our expert about what would be affected but it seems like a small change so I think we can handle that, I'm putting it into our queue! |
@miltonhultgren thanks for taking care of the changes on the integrations side! @leehinman The changes are now approved and all PR checks are green but I am not authorized to merge the PR. Could you please merge the PR on my behalf? Thank you! |
* Recognize log level in Elasticsearch JVM logs Elasticsearch will add a log level to its JVM logs to allow users to help them detect errors / warnings more easily. With this commit we detect this new field if present and continue to recognize the prior log format without a log level. Relates elastic/elasticsearch#92382 Closes #34054
What does this PR do?
Elasticsearch will add a log level to its JVM logs to allow users to help them detect errors / warnings more easily in elastic/elasticsearch#92382. With this commit we detect this new field if present and continue to recognize the prior log format which does not contain a log level.
Why is it important?
This change is required to keep compatibility with Elasticsearch's JVM log output.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues
Relates elastic/elasticsearch#92382
Closes #34054