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

Fix: don't miss address scheme #16205

Merged
merged 4 commits into from
Feb 11, 2020
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.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,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 {
jsoriano marked this conversation as resolved.
Show resolved Hide resolved
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) {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this test!

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