-
Notifications
You must be signed in to change notification settings - Fork 82
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
Prevent mapping explosion on logs #4181
Conversation
This commit remove some JSON objects that were added to the logs, thus preventing mapping explosion when those logs are ingested into Elasticsearch. Some entries are converted to strings and others just removed to keep the log within a reasonable size.
This pull request does not have a backport label. Could you fix it @belimawr? 🙏
|
|
case !reflect.DeepEqual(curCfg.Inputs[0].Server, newCfg.Inputs[0].Server): | ||
zlog.Info(). | ||
Interface("old", curCfg.Redact()). |
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 we log the new/old config as strings on the debug level instead? (but keep the info level messages when something has changed)
internal/pkg/api/handleCheckin.go
Outdated
Msg("local components data is not equal") | ||
|
||
zlog.Info(). | ||
RawJSON("req.Components", *req.Components). | ||
Str("req.Components", string(reqComponentsJSON)). |
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 there a reason for us to log the components JSON for every components change? Successful checkins will allow it to be queried from the .fleet-agents index at any time. This seems like something that would only be valuable to log whe there is an error.
At minimum this could be moved to the debug level, it doesn't make sense to log this at info by the unhealthy_reason at debug below, the unhealthy reason is much more interesting, but again it can be queried from .fleet-agents at any time.
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 there a reason for us to log the components JSON for every components change?
I'm not sure, honestly, I was just focusing on the mapping explosion. Someone from @elastic/fleet might be able to answer that.
The lines before already log the old and new components at trace level, so just removing them from this entry seems to be the best option, which will also avoid logging the same thing multiple times.
Quality Gate passedIssues Measures |
This commit remove some JSON objects that were added to the logs, thus preventing mapping explosion when those logs are ingested into Elasticsearch. Some entries are converted to strings, others are fully removed to keep the log within a reasonable size and some are kept as string at trace level. (cherry picked from commit 8ff01e3)
This commit remove some JSON objects that were added to the logs, thus preventing mapping explosion when those logs are ingested into Elasticsearch. Some entries are converted to strings, others are fully removed to keep the log within a reasonable size and some are kept as string at trace level. (cherry picked from commit 8ff01e3)
This commit remove some JSON objects that were added to the logs, thus preventing mapping explosion when those logs are ingested into Elasticsearch. Some entries are converted to strings, others are fully removed to keep the log within a reasonable size and some are kept as string at trace level. (cherry picked from commit 8ff01e3) Co-authored-by: Tiago Queiroz <[email protected]>
This commit remove some JSON objects that were added to the logs, thus preventing mapping explosion when those logs are ingested into Elasticsearch. Some entries are converted to strings, others are fully removed to keep the log within a reasonable size and some are kept as string at trace level. (cherry picked from commit 8ff01e3) Co-authored-by: Tiago Queiroz <[email protected]>
What is the problem this PR solves?
This commit remove some JSON objects that were added to the logs, thus preventing mapping explosion when those logs are ingested into Elasticsearch.
How does this PR solve the problem?
Some entries are converted to strings and others just removed to keep the log within a reasonable size.
How to test this PR locally
Run Fleet-Server, watch the logs
Design Checklist
Checklist
[ ] I have commented my code, particularly in hard-to-understand areas[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added tests that prove my fix is effective or that my feature works[ ] I have added an entry in./changelog/fragments
using the changelog tool## Related issues