Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix unused libbeat.config.module metrics #19168

Merged
merged 3 commits into from
Jun 18, 2020
Merged

Fix unused libbeat.config.module metrics #19168

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
629b379
Fix unused libbeat.config.module metrics
andrewkroh Jun 12, 2020
b74ae15
Add comments about libbeat.config.module.running
andrewkroh Jun 15, 2020
33386de
Merge branch 'master' into bugfix/libbeat/config-module-metrics
andrewkroh Jun 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- The `monitoring.elasticsearch.api_key` value is correctly base64-encoded before being sent to the monitoring Elasticsearch cluster. {issue}18939[18939] {pull}18945[18945]
- Fix kafka topic setting not allowing upper case characters. {pull}18854[18854] {issue}18640[18640]
- Fix redis key setting not allowing upper case characters. {pull}18854[18854] {issue}18640[18640]
- Fix config reload metrics (`libbeat.config.module.start/stops/running`). {pull}19168[19168]

*Auditbeat*

Expand Down
8 changes: 8 additions & 0 deletions libbeat/cfgfile/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (r *RunnerList) Reload(configs []*reload.ConfigWithMeta) error {
r.logger.Debugf("Stopping runner: %s", runner)
delete(r.runners, hash)
go runner.Stop()
moduleStops.Add(1)
}

// Start new runners
Expand All @@ -101,8 +102,15 @@ func (r *RunnerList) Reload(configs []*reload.ConfigWithMeta) error {
r.logger.Debugf("Starting runner: %s", runner)
r.runners[hash] = runner
runner.Start()
moduleStarts.Add(1)
}

// NOTE: This metric tracks the number of modules in the list. The true
// number of modules in the running state may differ because modules can
// stop on their own (i.e. on errors) and also when this stops a module
// above it is done asynchronously.
moduleRunning.Set(int64(len(r.runners)))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment here that the metric is not necessarily correct. The Reload method calls go runner.Stop and immediately removes the runner from the hashtable. But in fact (especially filebeat) the runner/input might still be active for an unknown amount of time (it's a common problem with reconfigured filebeat inputs erroring out on Start).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I guess the original name wasn't necessary the best for this. I added some comments.


return errs.Err()
}

Expand Down
2 changes: 1 addition & 1 deletion libbeat/cfgfile/reload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var (
configReloads = monitoring.NewInt(nil, "libbeat.config.reloads")
moduleStarts = monitoring.NewInt(nil, "libbeat.config.module.starts")
moduleStops = monitoring.NewInt(nil, "libbeat.config.module.stops")
moduleRunning = monitoring.NewInt(nil, "libbeat.config.module.running")
moduleRunning = monitoring.NewInt(nil, "libbeat.config.module.running") // Number of modules in the runner list (not necessarily in the running state).
)

// DynamicConfig loads config files from a given path, allowing to reload new changes
Expand Down