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.

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

Set default metricset in windows module to service #6675

Merged
merged 1 commit into from
Mar 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
- Set default metricsets in vSphere module. {pull}6676[6676]
- Set `status` as default metricset in Apache module. {pull}6673[6673]
- Set `namespace` as default metricset in Aerospike module. {pull}6669[6669]
- Set `service` as default metricset in Windows module. {pull}6675[6675]

*Packetbeat*

Expand Down
9 changes: 2 additions & 7 deletions metricbeat/docs/modules/windows.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ This file is generated! See scripts/docs_collector.py

beta[]

This is the Windows module.
This is the Windows module. It collects metrics from Windows systems,
by default metricset `service` is enabled.


[float]
Expand All @@ -20,12 +21,6 @@ in <<configuration-metricbeat>>. Here is an example configuration:
----
metricbeat.modules:
- module: windows
metricsets: ["perfmon"]
period: 10s
perfmon.counters:

- module: windows
metricsets: ["service"]
period: 60s
----

Expand Down
7 changes: 7 additions & 0 deletions metricbeat/metricbeat.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -493,11 +493,18 @@ metricbeat.modules:
#------------------------------- Windows Module ------------------------------
- module: windows
metricsets: ["perfmon"]
enabled: true
period: 10s
perfmon.ignore_non_existent_counters: true
perfmon.counters:
# - instance_label: processor.name
# instance_name: total
# measurement_label: processor.time.total.pct
# query: '\Processor Information(_Total)\% Processor Time'

- module: windows
metricsets: ["service"]
enabled: true
period: 60s
Copy link
Member

Choose a reason for hiding this comment

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

Now it becomes interesting. In the reference config we should use defaults and only in config.ymloverwrite the defaults. At least that is what we did so far.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how periods were in the reference config before this PR :)

I think that supporting different default periods can make a lot of sense (this module is a good example), but this gives for a bigger discussion out of the scope of this PR, wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

That we had different periods in the reference config was more an accident as the values were taken from the short config. Now that we have a reference file we could use the default one.

There are a lot of interesting discussion popping up with our changes here. We can take these offline. Will merge as is for now.


#------------------------------ ZooKeeper Module -----------------------------
Expand Down
15 changes: 15 additions & 0 deletions metricbeat/module/windows/_meta/config.reference.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- module: windows
metricsets: ["perfmon"]
enabled: true
period: 10s
perfmon.ignore_non_existent_counters: true
perfmon.counters:
# - instance_label: processor.name
# instance_name: total
# measurement_label: processor.time.total.pct
# query: '\Processor Information(_Total)\% Processor Time'

- module: windows
metricsets: ["service"]
enabled: true
period: 60s
6 changes: 0 additions & 6 deletions metricbeat/module/windows/_meta/config.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,2 @@
- module: windows
metricsets: ["perfmon"]
period: 10s
perfmon.counters:

- module: windows
metricsets: ["service"]
period: 60s
3 changes: 2 additions & 1 deletion metricbeat/module/windows/_meta/docs.asciidoc
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
This is the Windows module.
This is the Windows module. It collects metrics from Windows systems,
by default metricset `service` is enabled.
4 changes: 1 addition & 3 deletions metricbeat/module/windows/perfmon/perfmon.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ type Config struct {
}

func init() {
if err := mb.Registry.AddMetricSet("windows", "perfmon", New); err != nil {
panic(err)
}
mb.Registry.MustAddMetricSet("windows", "perfmon", New)
}

type MetricSet struct {
Expand Down
6 changes: 3 additions & 3 deletions metricbeat/module/windows/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
// init registers the MetricSet with the central registry.
// The New method will be called after the setup of the module and before starting to fetch data
func init() {
if err := mb.Registry.AddMetricSet("windows", "service", New); err != nil {
panic(err)
}
mb.Registry.MustAddMetricSet("windows", "service", New,
mb.DefaultMetricSet(),
)
}

// MetricSet type defines all fields of the MetricSet
Expand Down
6 changes: 0 additions & 6 deletions metricbeat/modules.d/windows.yml.disabled
Original file line number Diff line number Diff line change
@@ -1,8 +1,2 @@
- module: windows
metricsets: ["perfmon"]
period: 10s
perfmon.counters:

- module: windows
metricsets: ["service"]
period: 60s