Skip to content

Commit

Permalink
Fix incorrect usage of hints builder when exposed port is a substring…
Browse files Browse the repository at this point in the history
… of the hint (#19052) (#19163)

This PR makes sure that if a user exposes port 80 and has 8080 on the
metric hint, then we don't use that port to do the autodiscover as the port
could be exposed on another container.

(cherry picked from commit 138a23a)

Co-authored-by: Vijay Samuel <vjsamuel@ebay.com>
  • Loading branch information
jsoriano and vjsamuel authored Jun 15, 2020
1 parent 8e8a7a2 commit f8298da
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ field. You can revert this change by configuring tags for the module and omittin
- Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802]
- Fix compute and pubsub dashboard for googlecloud module. {issue}18962[18962] {pull}18980[18980]
- Fix crash on vsphere module when Host information is not available. {issue}18996[18996] {pull}19078[19078]
- Fix incorrect usage of hints builder when exposed port is a substring of the hint {pull}19052[19052]

*Packetbeat*

Expand Down
24 changes: 23 additions & 1 deletion metricbeat/autodiscover/builder/hints/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package hints

import (
"fmt"
"strconv"
"strings"

"github.com/elastic/go-ucfg"
Expand Down Expand Up @@ -187,7 +188,7 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string,
// Only pick hosts that have ${data.port} or the port on current event. This will make
// sure that incorrect meta mapping doesn't happen
for _, h := range thosts {
if strings.Contains(h, "data.port") || strings.Contains(h, fmt.Sprintf(":%d", port)) ||
if strings.Contains(h, "data.port") || m.checkHostPort(h, port) ||
// Use the event that has no port config if there is a ${data.host}:9090 like input
(port == 0 && strings.Contains(h, "data.host")) {
result = append(result, h)
Expand All @@ -202,6 +203,27 @@ func (m *metricHints) getHostsWithPort(hints common.MapStr, port int) ([]string,
return result, true
}

func (m *metricHints) checkHostPort(h string, p int) bool {
port := strconv.Itoa(p)

index := strings.LastIndex(h, ":"+port)
// Check if host contains :port. If not then return false
if index == -1 {
return false
}

// Check if the host ends with :port. Return true if yes
end := index + len(port) + 1
if end == len(h) {
return true
}

// Check if the character immediately after :port. If its not a number then return true.
// This is to avoid adding :80 as a valid host for an event that has port=8080
// Also ensure that port=3306 and hint="tcp(${data.host}:3306)/" is valid
return h[end] < '0' || h[end] > '9'
}

func (m *metricHints) getNamespace(hints common.MapStr) string {
return builder.GetHintString(hints, m.Key, namespace)
}
Expand Down
39 changes: 39 additions & 0 deletions metricbeat/autodiscover/builder/hints/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,45 @@ func TestGenerateHints(t *testing.T) {
"enabled": true,
},
},
{
message: "Module, namespace, host hint shouldn't return when port isn't the same has hint",
event: bus.Event{
"host": "1.2.3.4",
"port": 80,
"hints": common.MapStr{
"metrics": common.MapStr{
"module": "mockmoduledefaults",
"namespace": "test",
"hosts": "${data.host}:8080",
},
},
},
len: 0,
},
{
message: "Non http URLs with valid host port combination should return a valid config",
event: bus.Event{
"host": "1.2.3.4",
"port": 3306,
"hints": common.MapStr{
"metrics": common.MapStr{
"module": "mockmoduledefaults",
"namespace": "test",
"hosts": "tcp(${data.host}:3306)/",
},
},
},
len: 1,
result: common.MapStr{
"module": "mockmoduledefaults",
"namespace": "test",
"metricsets": []string{"default"},
"hosts": []interface{}{"tcp(1.2.3.4:3306)/"},
"timeout": "3s",
"period": "1m",
"enabled": true,
},
},
}
for _, test := range tests {
mockRegister := mb.NewRegister()
Expand Down

0 comments on commit f8298da

Please sign in to comment.