Skip to content

Conversation

@DebakelOrakel
Copy link
Contributor

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Update the documentation.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • Link this PR to related issues.

@DebakelOrakel DebakelOrakel added the enhancement New feature or request label Aug 20, 2021
@DebakelOrakel DebakelOrakel changed the title Json log parsing Add JSON log parsing Aug 20, 2021
@DebakelOrakel
Copy link
Contributor Author

Depends on #14

@simu
Copy link
Member

simu commented Aug 20, 2021

Please finish and merge #14 first and rebase this afterwards, it's unclear which changes belong to this PR and which belong to #14 currently.

Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good.

Does this also affect logs which are ingested into the default cluster-logging Elasticsearch? That's not very clear from the documentation.

PS: why are there 15 commits in this PR?

@DebakelOrakel
Copy link
Contributor Author

DebakelOrakel commented Aug 20, 2021

Does this also affect logs which are ingested into the default cluster-logging Elasticsearch? That's not very clear from the documentation.
It does affect all logs. It works like this:

  • it tries to parse the message field to json and saves it in a new field structured
  • if the pod (depending on configuration) has a label logformat=apache it creates a new index app-apache-xxxxx where the structured fields are indexed and searchable
  • if it cant parse the message field no structured field is created
  • only application logs are affected

PS: why are there 15 commits in this PR?
I was wondering that either, probably bc i checked out from the other branch...

@simu
Copy link
Member

simu commented Aug 20, 2021

PS: why are there 15 commits in this PR?

I was wondering that either, probably bc i checked out from the other branch...

Looking at the commits in more detail, it's probably because you used GitHub's "Update Branch" which just merges master into the feature branch instead of rebasing the feature branch on the latest master.

Copy link
Contributor

@corvus-ch corvus-ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for the git history. Please rebase.

@DebakelOrakel
Copy link
Contributor Author

WTF... git, are you mad?

@DebakelOrakel DebakelOrakel force-pushed the json-log-parsing branch 3 times, most recently from d5f0900 to 2216429 Compare August 23, 2021 08:44
@DebakelOrakel DebakelOrakel merged commit ef955e9 into master Aug 23, 2021
@DebakelOrakel DebakelOrakel deleted the json-log-parsing branch August 23, 2021 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants