-
Notifications
You must be signed in to change notification settings - Fork 143
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
[Feature] Start/stop monitoring server based on monitoring config #3584
Conversation
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
🌐 Coverage report
|
This pull request is now in conflicts. Could you fix it? 🙏
|
I think we need to change the agent configuration used in cloud first, and since we are doing that we should address #2509 which will also need a cloud configuration change. See #2734 (comment) |
this is dependent on work defined here: elastic/kibana#168629
default behavior is either 1st or 3rd with very few exceptions which falls into seconds case. |
all dependent work is done. bringing this alive |
func (sr *ServerReloader) Stop() error { | ||
sr.isServerRunningLock.Lock() | ||
defer sr.isServerRunningLock.Unlock() | ||
|
||
return sr.stop() | ||
} |
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.
[Suggestion]
You could use an atomic.Bool
instead of the mutex. I believe it'd make the code easier to maintain as it's not imediately obvious start()
and stop()
cannot lock the mutex because Reload()
does so and then call them.
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.
good point
failing on some unrelated tests, some of which are already addressed by @belimawr |
This pull request is now in conflicts. Could you fix it? 🙏
|
SonarQube Quality Gate |
due to cloud scenario where they have disabled monitoring but expect server to be running
this needs to be in sync with fleet to distinguish
monitoring.enabled
because we don't want to monitor anything and disabled because server should be downFixes: #2734