Skip to content

Don't generate autodiscover config when no port matches host hints #12086

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

Merged
jsoriano merged 3 commits into from
May 8, 2019
Merged

Don't generate autodiscover config when no port matches host hints #12086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
675d605
Don't generate autodiscover config when no port matches host hints
jsoriano May 6, 2019
87157d4
Fix changelog
jsoriano May 7, 2019
5339a45
Add tests for the case of multiple hosts
jsoriano May 8, 2019
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 @@ -104,6 +104,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Change diskio metrics retrieval method (only for Windows) from wmi query to DeviceIOControl function using the IOCTL_DISK_PERFORMANCE control code {pull}11635[11635]
- Call GetMetricData api per region instead of per instance. {issue}11820[11820] {pull}11882[11882]
- Update documentation with cloudwatch:ListMetrics permission. {pull}11987[11987]
- Avoid generating hints-based configuration with empty hosts when no exposed port is suitable for the hosts hint. {issue}8264[8264] {pull}12086[12086]

*Packetbeat*

Expand Down
17 changes: 13 additions & 4 deletions metricbeat/autodiscover/builder/hints/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ func (m *metricHints) CreateConfig(event bus.Event) []*common.Config {
return config
}

hsts := m.getHostsWithPort(hints, port)
hosts, ok := m.getHostsWithPort(hints, port)
if !ok {
return config
}

ns := m.getNamespace(hints)
msets := m.getMetricSets(hints, mod)
tout := m.getTimeout(hints)
Expand All @@ -112,7 +116,7 @@ func (m *metricHints) CreateConfig(event bus.Event) []*common.Config {
moduleConfig := common.MapStr{
"module": mod,
"metricsets": msets,
"hosts": hsts,
"hosts": hosts,
"timeout": tout,
"period": ival,
"enabled": true,
Expand Down Expand Up @@ -161,7 +165,7 @@ func (m *metricHints) getMetricSets(hints common.MapStr, module string) []string
return msets
}

func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) []string {
func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string, bool) {
var result []string
thosts := builder.GetHintAsList(hints, m.Key, hosts)

Expand All @@ -175,7 +179,12 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) []string {
}
}

return result
if len(thosts) > 0 && len(result) == 0 {
logp.Debug("hints.builder", "no hosts selected for port %d with hints: %+v", port, thosts)
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng May 7, 2019

Choose a reason for hiding this comment

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

Should we make the switch to m.logger here instead of logp? 😬not sure if putting them in same pr is good or not.

Copy link
Member Author

@jsoriano jsoriano May 7, 2019

Choose a reason for hiding this comment

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

This module doesn't have a logger yet, I'd leave this for a future change.

return nil, false
}

return result, true
}

func (m *metricHints) getNamespace(hints common.MapStr) string {
Expand Down
61 changes: 60 additions & 1 deletion metricbeat/autodiscover/builder/hints/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,65 @@ func TestGenerateHints(t *testing.T) {
len: 0,
result: common.MapStr{},
},
{
message: "Hints without matching port should return nothing",
event: bus.Event{
"host": "1.2.3.4",
"port": 9090,
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng May 7, 2019

Choose a reason for hiding this comment

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

What if no port is assigned to this container?

Copy link
Member Author

@jsoriano jsoriano May 8, 2019

Choose a reason for hiding this comment

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

Then a configuration is returned, the following test cases cover this.

"hints": common.MapStr{
"metrics": common.MapStr{
"module": "mockmoduledefaults",
"hosts": "${data.host}:8888",
},
},
},
len: 0,
result: common.MapStr{},
},
{
message: "Hints with multiple hosts return only the matching one",
event: bus.Event{
"host": "1.2.3.4",
"port": 9090,
"hints": common.MapStr{
"metrics": common.MapStr{
"module": "mockmoduledefaults",
"hosts": "${data.host}:8888,${data.host}:9090",
},
},
},
len: 1,
result: common.MapStr{
"module": "mockmoduledefaults",
"metricsets": []string{"default"},
"timeout": "3s",
"period": "1m",
"enabled": true,
"hosts": []interface{}{"1.2.3.4:9090"},
},
},
{
message: "Hints with multiple hosts return only the one with the template",
event: bus.Event{
"host": "1.2.3.4",
"port": 9090,
"hints": common.MapStr{
"metrics": common.MapStr{
"module": "mockmoduledefaults",
"hosts": "${data.host}:8888,${data.host}:${data.port}",
},
},
},
len: 1,
result: common.MapStr{
"module": "mockmoduledefaults",
"metricsets": []string{"default"},
"timeout": "3s",
"period": "1m",
"enabled": true,
"hosts": []interface{}{"1.2.3.4:9090"},
},
},
{
message: "Only module hint should return all metricsets",
event: bus.Event{
Expand All @@ -87,7 +146,7 @@ func TestGenerateHints(t *testing.T) {
},
},
{
message: "metricsets hint works",
message: "Metricsets hint works",
event: bus.Event{
"host": "1.2.3.4",
"hints": common.MapStr{
Expand Down