Skip to content

Commit

Permalink
Fix: don't miss address scheme (elastic#16205)
Browse files Browse the repository at this point in the history
* Fix: don't miss address scheme

* Add unit test

* Adjust source after code review

* Add comment to method
  • Loading branch information
mtojek authored Feb 11, 2020
1 parent 4dd40a4 commit 9c2064a
Show file tree
Hide file tree
Showing 5 changed files with 116 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 @@ -87,6 +87,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Change lookup_fields from metricset.host to service.address {pull}15883[15883]
- Add dedot for cloudwatch metric name. {issue}15916[15916] {pull}15917[15917]
- Fixed issue `logstash-xpack` module suddenly ceasing to monitor Logstash. {issue}15974[15974] {pull}16044[16044]
- Fix skipping protocol scheme by light modules. {pull}16205[pull]

*Packetbeat*

Expand Down
18 changes: 17 additions & 1 deletion metricbeat/mb/lightmetricset.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
package mb

import (
"fmt"
"net/url"

"github.com/pkg/errors"

"github.com/elastic/beats/libbeat/common"
Expand Down Expand Up @@ -83,7 +86,8 @@ func (m *LightMetricSet) Registration(r *Register) (MetricSetRegistration, error
// At this point host parser was already run, we need to run this again
// with the overriden defaults
if registration.HostParser != nil {
base.hostData, err = registration.HostParser(base.module, base.host)
host := m.useHostURISchemeIfPossible(base.host, base.hostData.URI)
base.hostData, err = registration.HostParser(base.module, host)
if err != nil {
return nil, errors.Wrapf(err, "host parser failed on light metricset factory for '%s/%s'", m.Module, m.Name)
}
Expand All @@ -96,6 +100,18 @@ func (m *LightMetricSet) Registration(r *Register) (MetricSetRegistration, error
return registration, nil
}

// useHostURISchemeIfPossible method parses given URI to extract protocol scheme and prepend it to the host.
// It prevents from skipping protocol scheme (e.g. https) while executing HostParser.
func (m *LightMetricSet) useHostURISchemeIfPossible(host, uri string) string {
u, err := url.ParseRequestURI(uri)
if err == nil {
if u.Scheme != "" {
return fmt.Sprintf("%s://%s", u.Scheme, u.Host)
}
}
return host
}

// baseModule does the configuration overrides in the base module configuration
// taking into account the light metric set default configurations
func (m *LightMetricSet) baseModule(from Module) (*BaseModule, error) {
Expand Down
91 changes: 91 additions & 0 deletions metricbeat/mb/lightmodules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package mb

import (
"net/url"
"testing"
"time"

Expand Down Expand Up @@ -287,6 +288,96 @@ func TestNewModuleFromConfig(t *testing.T) {
}
}

func TestLightMetricSet_VerifyHostDataURI(t *testing.T) {
const hostEndpoint = "ceph-restful:8003"
const sampleHttpsEndpoint = "https://" + hostEndpoint

r := NewRegister()
r.MustAddMetricSet("http", "json", newMetricSetWithOption,
WithHostParser(func(module Module, host string) (HostData, error) {
u, err := url.Parse(host)
if err != nil {
return HostData{}, err
}
return HostData{
Host: u.Host,
URI: host,
}, nil
}))
r.SetSecondarySource(NewLightModulesSource("testdata/lightmodules"))

config, err := common.NewConfigFrom(
common.MapStr{
"module": "httpextended",
"metricsets": []string{"extends"},
"hosts": []string{sampleHttpsEndpoint},
})
require.NoError(t, err)

_, metricSets, err := NewModule(config, r)
require.NoError(t, err)
require.Len(t, metricSets, 1)

assert.Equal(t, hostEndpoint, metricSets[0].Host())
assert.Equal(t, sampleHttpsEndpoint, metricSets[0].HostData().URI)
}

func TestLightMetricSet_WithoutHostParser(t *testing.T) {
const sampleHttpsEndpoint = "https://ceph-restful:8003"

r := NewRegister()
r.MustAddMetricSet("http", "json", newMetricSetWithOption)
r.SetSecondarySource(NewLightModulesSource("testdata/lightmodules"))

config, err := common.NewConfigFrom(
common.MapStr{
"module": "httpextended",
"metricsets": []string{"extends"},
"hosts": []string{sampleHttpsEndpoint},
})
require.NoError(t, err)

_, metricSets, err := NewModule(config, r)
require.NoError(t, err)
require.Len(t, metricSets, 1)

assert.Equal(t, sampleHttpsEndpoint, metricSets[0].Host())
assert.Equal(t, sampleHttpsEndpoint, metricSets[0].HostData().URI)
}

func TestLightMetricSet_VerifyHostDataURI_NonParsableHost(t *testing.T) {
const (
postgresHost = "host1:5432"
postgresEndpoint = "postgres://user1:pass@host1:5432?connect_timeout=2"
postgresParsed = "connect_timeout=3 host=host1 password=pass port=5432 user=user1"
)

r := NewRegister()
r.MustAddMetricSet("http", "json", newMetricSetWithOption,
WithHostParser(func(module Module, host string) (HostData, error) {
return HostData{
Host: postgresHost,
URI: postgresParsed,
}, nil
}))
r.SetSecondarySource(NewLightModulesSource("testdata/lightmodules"))

config, err := common.NewConfigFrom(
common.MapStr{
"module": "httpextended",
"metricsets": []string{"extends"},
"hosts": []string{postgresEndpoint},
})
require.NoError(t, err)

_, metricSets, err := NewModule(config, r)
require.NoError(t, err)
require.Len(t, metricSets, 1)

assert.Equal(t, postgresHost, metricSets[0].Host())
assert.Equal(t, postgresParsed, metricSets[0].HostData().URI)
}

func TestNewModulesCallModuleFactory(t *testing.T) {
logp.TestingSetup()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
default: true
input:
module: http
metricset: json
3 changes: 3 additions & 0 deletions metricbeat/mb/testdata/lightmodules/httpextended/module.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: httpextended
metricsets:
- extends

0 comments on commit 9c2064a

Please sign in to comment.