From bd7932135910d46355caaea6f8eeb5dcb7cd651d Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Sat, 8 Feb 2020 23:14:42 +0100 Subject: [PATCH 1/4] Fix: don't miss address scheme --- CHANGELOG.next.asciidoc | 1 + metricbeat/mb/lightmetricset.go | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 67505496c63..f0ff64315e0 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -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* diff --git a/metricbeat/mb/lightmetricset.go b/metricbeat/mb/lightmetricset.go index dee1d0afb7b..16f5d18dc2a 100644 --- a/metricbeat/mb/lightmetricset.go +++ b/metricbeat/mb/lightmetricset.go @@ -18,6 +18,9 @@ package mb import ( + "fmt" + "net/url" + "github.com/pkg/errors" "github.com/elastic/beats/libbeat/common" @@ -83,7 +86,18 @@ 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) + u, err := url.Parse(base.hostData.URI) + if err != nil { + return nil, errors.Wrapf(err, "host data URI parsing failed on light metricset factory for '%s/%s'", m.Module, m.Name) + } + + var host string + if u.Scheme != "" { + host = fmt.Sprintf("%s://%s", u.Scheme, u.Host) + } else { + host = base.host + } + 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) } From e689285f920a54543abe2063248b880a0db6014f Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Sun, 9 Feb 2020 23:05:24 +0100 Subject: [PATCH 2/4] Add unit test --- metricbeat/mb/lightmodules_test.go | 36 +++++++++++++++++++ .../httpextended/extends/manifest.yml | 4 +++ .../lightmodules/httpextended/module.yml | 3 ++ 3 files changed, 43 insertions(+) create mode 100644 metricbeat/mb/testdata/lightmodules/httpextended/extends/manifest.yml create mode 100644 metricbeat/mb/testdata/lightmodules/httpextended/module.yml diff --git a/metricbeat/mb/lightmodules_test.go b/metricbeat/mb/lightmodules_test.go index e7a933cc47a..5b7f059465f 100644 --- a/metricbeat/mb/lightmodules_test.go +++ b/metricbeat/mb/lightmodules_test.go @@ -20,6 +20,7 @@ package mb import ( + "net/url" "testing" "time" @@ -287,6 +288,41 @@ 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, + SanitizedURI: 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 TestNewModulesCallModuleFactory(t *testing.T) { logp.TestingSetup() diff --git a/metricbeat/mb/testdata/lightmodules/httpextended/extends/manifest.yml b/metricbeat/mb/testdata/lightmodules/httpextended/extends/manifest.yml new file mode 100644 index 00000000000..a56f13a3c7d --- /dev/null +++ b/metricbeat/mb/testdata/lightmodules/httpextended/extends/manifest.yml @@ -0,0 +1,4 @@ +default: true +input: + module: http + metricset: json diff --git a/metricbeat/mb/testdata/lightmodules/httpextended/module.yml b/metricbeat/mb/testdata/lightmodules/httpextended/module.yml new file mode 100644 index 00000000000..93ecaa59ba1 --- /dev/null +++ b/metricbeat/mb/testdata/lightmodules/httpextended/module.yml @@ -0,0 +1,3 @@ +name: httpextended +metricsets: +- extends From a117e598ed9c682df5e6083a380e95990a1e3746 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 11 Feb 2020 10:39:17 +0100 Subject: [PATCH 3/4] Adjust source after code review --- metricbeat/mb/lightmetricset.go | 22 +++++------ metricbeat/mb/lightmodules_test.go | 61 ++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/metricbeat/mb/lightmetricset.go b/metricbeat/mb/lightmetricset.go index 16f5d18dc2a..f4a5855958c 100644 --- a/metricbeat/mb/lightmetricset.go +++ b/metricbeat/mb/lightmetricset.go @@ -86,17 +86,7 @@ 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 { - u, err := url.Parse(base.hostData.URI) - if err != nil { - return nil, errors.Wrapf(err, "host data URI parsing failed on light metricset factory for '%s/%s'", m.Module, m.Name) - } - - var host string - if u.Scheme != "" { - host = fmt.Sprintf("%s://%s", u.Scheme, u.Host) - } else { - host = 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) @@ -110,6 +100,16 @@ func (m *LightMetricSet) Registration(r *Register) (MetricSetRegistration, error return registration, nil } +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) { diff --git a/metricbeat/mb/lightmodules_test.go b/metricbeat/mb/lightmodules_test.go index 5b7f059465f..90f81242651 100644 --- a/metricbeat/mb/lightmodules_test.go +++ b/metricbeat/mb/lightmodules_test.go @@ -300,9 +300,8 @@ func TestLightMetricSet_VerifyHostDataURI(t *testing.T) { return HostData{}, err } return HostData{ - Host: u.Host, - URI: host, - SanitizedURI: host, + Host: u.Host, + URI: host, }, nil })) r.SetSecondarySource(NewLightModulesSource("testdata/lightmodules")) @@ -323,6 +322,62 @@ func TestLightMetricSet_VerifyHostDataURI(t *testing.T) { 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() From 8237e9dd54a193710184d0ec9275f98f19fc477d Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 11 Feb 2020 16:46:30 +0100 Subject: [PATCH 4/4] Add comment to method --- metricbeat/mb/lightmetricset.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/metricbeat/mb/lightmetricset.go b/metricbeat/mb/lightmetricset.go index f4a5855958c..11e49cd5c77 100644 --- a/metricbeat/mb/lightmetricset.go +++ b/metricbeat/mb/lightmetricset.go @@ -100,6 +100,8 @@ 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 {