-
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
[Agent] Change monitoring defaults #18226
Conversation
Pinging @elastic/ingest-management (Team:Ingest Management) |
@@ -35,7 +41,7 @@ func NewMonitor(downloadConfig *artifact.Config) *Monitor { | |||
return &Monitor{ | |||
operatingSystem: downloadConfig.OS(), | |||
installPath: downloadConfig.InstallPath, | |||
config: &monitoringConfig.MonitoringConfig{}, | |||
config: &defaultMonitoringConfig, |
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.
@michalpristas As this introduces defaults, I wonder if we still need it to be a reference on line 35?
❕ Build Aborted
Expand to view the summary
Build stats
Log outputExpand to view the last 100 lines of log output
|
💔 Build FailedExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
@ruflin Is this PR still valid? |
@ph I think yes, as we still rely on the defaults in config files instead of in code. @michalpristas @blakerouse Could one of you take this over? |
i will take a look but we are in most cases using baked-in defaults to allow empty configs (this is a recent change) will align it with proposal here |
Created new PR here: #18927 |
WIP