-
Notifications
You must be signed in to change notification settings - Fork 525
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
Add initial support for beats central management #4228
Conversation
💔 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
|
Some more detailed instructions about testing:
|
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.
Looks good, just a couple of small things.
I suppose we should start looking at shuffling things around to use cfgfile.RunnerList
, like heartbeat?
That would take care of starting/stopping servers in response to config changes. Not needed for now though.
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.
LGTM, just one more small thing
beater/beater.go
Outdated
@@ -136,10 +114,40 @@ type beater struct { | |||
stopped bool | |||
} | |||
|
|||
var once sync.Once |
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.
Sorry, I missed one thing: can this be moved into the Run function, so it's close to where it's used?
var reloadOnce sync.Once
var reloadable = reload.ReloadableFunc(func(ucfg *reload.ConfigWithMeta) error {
...
}
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.
ah yes, didn't know I could do that 👍
Seeing as elastic/beats#21225 is not in the 7.10 branch, I'm not sure if or how we can test this with 7.10-BC1. I could cherry-pick into the elastic-agent 7.10 branch, but I think that would defeat the purpose of manual testing. I propose we push testing this back to 7.11, when the required elastic-agent bits are available. |
ah didn't think of that, sgtm |
Elasticsearch and Kibana setup like in #4542 (comment), with Elastic Agent running outside of Docker so I could modify I edited the APM integration to set
Because of #4572 the secret token is literally set to the string "false".
|
Motivation/summary
Registers a reloadable object in Central Management so that when APM Server runs under Elastic Agent, it can receive and apply configuration from it.
In practice APM Server will only expect configuration once. This is enough for the standalone mode.
Checklist
I have considered changes for:
How to test these changes
Pull in elastic/beats#21225 and build the Agent binary
Add an
apm
input toelastic-agent.yml
with a well known config, eg:Check that apm-server is running with RUM enabled
Related issues
Required for #4004