-
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
Allow users to set just monitoring.cluster_uuid #14338
Allow users to set just monitoring.cluster_uuid #14338
Conversation
Pinging @elastic/stack-monitoring (Stack monitoring) |
libbeat/monitoring/monitoring.go
Outdated
default: | ||
return nil, nil, nil | ||
} | ||
} | ||
|
||
func GetMonitoringClusterUUID(monitoringCfg *common.Config) (string, 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.
exported function GetMonitoringClusterUUID should have comment or be unexported
7522cb7
to
88afc59
Compare
0c97867
to
baa4771
Compare
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.
I don't know the libbeat codebase well enough to leave a thorough review on the code changes but I tested this functionally and it looks good to me.
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
baa4771
to
d826553
Compare
@ycombinator Was this a bad rebase? It looks like this suddenly has many unrelated changes. |
@cachedout Thanks, indeed, I did a bad rebase 🤦♂. Fixing now... |
9b5bfd6
to
81d21cd
Compare
Bad rebase is fixed. |
Travis CI is green and Jenkins CI failures are unrelated. Merging. |
* Allow users to set just monitoring.cluster_uuid (#14338) * Allow users to just set monitoring.cluster_uuid * Adding back comment lost in refactoring * Adds godoc comments for functions * Removes stutter in function name * Adding system test * Adding CHANGELOG entry * Fixing formatting * Move defer to right scope * Adding system test * Fixing formatting * Fixing up CHANGELOG
* Allow users to just set monitoring.cluster_uuid * Adding back comment lost in refactoring * Adds godoc comments for functions * Removes stutter in function name * Adding system test * Adding CHANGELOG entry * Fixing formatting * Move defer to right scope * Adding system test * Fixing formatting
…elastic#14633) * Allow users to set just monitoring.cluster_uuid (elastic#14338) * Allow users to just set monitoring.cluster_uuid * Adding back comment lost in refactoring * Adds godoc comments for functions * Removes stutter in function name * Adding system test * Adding CHANGELOG entry * Fixing formatting * Move defer to right scope * Adding system test * Fixing formatting * Fixing up CHANGELOG
Currently there is a bug in libbeat which allows users to set
monitoring.cluster_uuid
only if there is also a monitoring reporter configured (either viaoutput.elasticsearch.*
settings ormonitoring.elasticsearch.*
settings). If there are no monitoring reporter settings (butmonitoring.cluster_uuid
is set), the value ofmonitoring.cluster_uuid
is not exposed via theGET state
API. This further does not allow themonitoring.cluster_uuid
Metricbeat'sbeat
module to monitor a beat that has specified themonitoring.cluster_uuid
setting in it's configuration. Ultimately this results in the monitored beat being shown under a "Standalone Cluster" in the Stack Monitoring UI.Therefore, we should allow users to configure the
monitoring.cluster_uuid
setting even when no other settings under themonitoring
namespace are configured.This PR fixes this bug by:
monitoring.cluster_uuid
setting before the reporter configuration is done;GET state
API to use, so Metricbeat'sbeat
module may be able to fetch it; andmonitoring
iscluster_uuid
, as we expect the other settings to be related to the reporter (monitoring.elasticsearch.*
).This PR also does a bit of cleanup by extracting the monitoring setup code out of
(*Beat).launch
and into its own method.Testing this PR
elasticsearch
output in your Beat and enable some other output.http.enabled=true
andmonitoring.cluster_uuid=<some value>
.monitoring.cluster_uuid
value is reported.