From d4049bf74b9c72c6efe76c66e3c5a88dcc9e5805 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 11 Feb 2020 17:06:21 +0100 Subject: [PATCH 1/2] Fix: don't miss address scheme (#16205) * Fix: don't miss address scheme * Add unit test * Adjust source after code review * Add comment to method (cherry picked from commit 9c2064abb1b790c08826ead27dc2587c6cb75d7d) --- CHANGELOG.next.asciidoc | 2 + metricbeat/mb/lightmetricset.go | 18 +++- metricbeat/mb/lightmodules_test.go | 91 +++++++++++++++++++ .../httpextended/extends/manifest.yml | 4 + .../lightmodules/httpextended/module.yml | 3 + 5 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 metricbeat/mb/testdata/lightmodules/httpextended/extends/manifest.yml create mode 100644 metricbeat/mb/testdata/lightmodules/httpextended/module.yml diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index c4cd32c4378..7d7492877bb 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -114,6 +114,8 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Add dedot for tags in ec2 metricset and cloudwatch metricset. {issue}15843[15843] {pull}15844[15844] - Use RFC3339 format for timestamps collected using the SQL module. {pull}15847[15847] - 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 eb0ec074918..2a83d61c6be 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" @@ -81,7 +84,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) } @@ -94,6 +98,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) { diff --git a/metricbeat/mb/lightmodules_test.go b/metricbeat/mb/lightmodules_test.go index 4278adc049b..f33318ffe19 100644 --- a/metricbeat/mb/lightmodules_test.go +++ b/metricbeat/mb/lightmodules_test.go @@ -20,6 +20,7 @@ package mb import ( + "net/url" "testing" "time" @@ -280,6 +281,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() 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 9e185ab32496c1af8de0fbee5504f4c82a12b7fb Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 11 Feb 2020 17:16:49 +0100 Subject: [PATCH 2/2] Fix: changelog --- CHANGELOG.next.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 7d7492877bb..5e5c4e50ece 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -114,7 +114,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Add dedot for tags in ec2 metricset and cloudwatch metricset. {issue}15843[15843] {pull}15844[15844] - Use RFC3339 format for timestamps collected using the SQL module. {pull}15847[15847] - 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*