Allow specifying default metricsets #5675
Conversation
|
This will be used by Auditbeat to simplify the configuration. It's part of the work for #5422. |
This is useful for modules containing a single metricset where you don't need the granularity of multiple metricsets. If `metricsets` isn't found in the config then the default MetricSets will be instantiated. If no defaults are defined for a module then an error is shown to the user (as was the case before this change when no `metricsets` were configured).
```
func init() {
mb.Registry.MustAddMetricSet(moduleName, metricsetName, New,
mb.DefaultMetricSet(),
mb.WithHostParser(parse.EmptyHostParser),
)
}
```
andrewkroh
force-pushed
the
feature/mb/default-metricsets
branch
from
aa0db29 to
d910ced
Compare
| @@ -94,7 +115,28 @@ func (r *Register) AddModule(name string, factory ModuleFactory) error { | |||
| // HostParser function for parsing the 'host' configuration data. An error is | |||
| // returned if any parameter is empty or nil or if a factory has already been | |||
| // registered under the name. | |||
| // | |||
| // Use MustAddMetricSet for new code. | |||
There was a problem hiding this comment.
Does that mean we need to update all existing metricsets?
There was a problem hiding this comment.
UPDATE: Just read your comment again in the PR and does not seem to be the case.
|
@andrewkroh Does this need a review label? |
|
@ruflin Yeah, I had forgotten to add it when I opened the PR. |
|
The test failures on jenkins-windows are for filebeat and libbeat and appear to be unrelated. |
| // addMetricSet registers a new MetricSetFactory. An error is returned if any | ||
| // parameter is empty or nil or if a factory has already been registered under | ||
| // the name. | ||
| func (r *Register) addMetricSet(module, name string, factory MetricSetFactory, options ...MetricSetOption) error { |
There was a problem hiding this comment.
I like that we now have options instead of the more specific hostParser 👍
| @@ -146,20 +190,47 @@ func (r *Register) metricSetFactory(module, name string) (MetricSetFactory, Host | |||
| module = strings.ToLower(module) | |||
| name = strings.ToLower(name) | |||
|
|
|||
| modules, exists := r.metricSets[module] | |||
| metricSets, exists := r.metricSets[module] | |||
| func (r *Register) MetricSets(module string) []string { | ||
| r.lock.RLock() | ||
| defer r.lock.RUnlock() | ||
|
|
||
| var metricsets []string | ||
|
|
||
| sets, ok := r.metricSets[module] | ||
| sets, ok := r.metricSets[strings.ToLower(module)] |
There was a problem hiding this comment.
I wonder if we should add a test somewhere to make sure this is not missing in other parts of the code.
That's correct, there can be more than one metricset marked as default. |
This is useful for modules containing a single metricset where you don't need the granularity of multiple metricsets. If
metricsetsisn't found in the config then the default MetricSets will be instantiated. If no defaults are defined for a module then an error is shown to the user (as was the case before this change when nometricsetswere configured).