-
Notifications
You must be signed in to change notification settings - Fork 203
Remove config parsing from monitoring manager #11466
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
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # internal/pkg/agent/application/monitoring/component/v1_monitor.go # internal/pkg/agent/application/monitoring/component/v1_monitor_test.go
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
|
Did you mean to check in the |
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.
Cleanup LGTM. Left a couple of nitpicks and a question about committing the testsignal binary.
Co-authored-by: Shaunak Kashyap <[email protected]>
It's already there. I think it got into this commit during a complicated rebase, I removed it. |
What does this PR do?
Drops the policy parsing code from the monitoring manager. This configuration is reloaded via the
Reloadfunction and the manager has access to it in a structured format.Why is it important?
The parsing code is ugly, difficult to understand, and unnecessary.
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added an entry in./changelog/fragmentsusing the changelog tool[ ] I have added an integration test or an E2E testHow to test this PR locally
Build agent locally, start it, and try changing the monitoring settings. In particular, this PR affects the
failure_thresholdandmetrics_periodsettings.