Skip to content

Commit

Permalink
Call host parser only once when building light metricsets (elastic#20149
Browse files Browse the repository at this point in the history
)

When using light modules, host parser is called twice. First by the actual implementation
of the metricset, and second after adding the configuration defined in the light module
manifest. Second call might be missing data as the original host is modified after the first
call, causing problems.

This change disables host parser in the registration created for light modules, and makes
only the "second call" inside the factory.

It also removes previous fix for URLs as it shouldn't be needed anymore.
  • Loading branch information
jsoriano authored Jul 23, 2020
1 parent 7579d51 commit 6c82292
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-developer.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ The list below covers the major changes between 7.0.0-rc2 and master only.
==== Bugfixes

- Stop using `mage:import` in community beats. This was ignoring the vendorized beats directory for some mage targets, using the code available in GOPATH, this causes inconsistencies and compilation problems if the version of the code in the GOPATH is different to the vendored one. Use of `mage:import` will continue to be unsupported in custom beats till beats is migrated to go modules, or mage supports vendored dependencies. {issue}13998[13998] {pull}14162[14162]
- Metricbeat module builders call host parser only once when instantiating light modules. {pull}20149[20149]

==== Added

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

import (
"fmt"
"net/url"

"github.com/pkg/errors"

"github.com/elastic/beats/v7/libbeat/common"
Expand Down Expand Up @@ -55,13 +52,17 @@ func (m *LightMetricSet) Registration(r *Register) (MetricSetRegistration, error
originalFactory := registration.Factory
registration.IsDefault = m.Default

// Disable the host parser, we will call it as part of the factory so the original
// host in the base module is not modified.
originalHostParser := registration.HostParser
registration.HostParser = nil

// Light modules factory has to override defaults and reproduce builder
// functionality with the resulting configuration, it does:
// - Override defaults
// - Call module factory if registered (it wouldn't have been called
// if light module is really a registered mixed module)
// - Call host parser if defined (it would have already been called
// without the light module defaults)
// - Call host parser if there was one defined
// - Finally, call the original factory for the registered metricset
registration.Factory = func(base BaseMetricSet) (MetricSet, error) {
// Override default config on base module and metricset
Expand All @@ -83,11 +84,9 @@ func (m *LightMetricSet) Registration(r *Register) (MetricSetRegistration, error
base.module = module
}

// At this point host parser was already run, we need to run this again
// with the overriden defaults
if registration.HostParser != nil {
host := m.useHostURISchemeIfPossible(base.host, base.hostData.URI)
base.hostData, err = registration.HostParser(base.module, host)
// Run the host parser if there was anyone defined
if originalHostParser != nil {
base.hostData, err = originalHostParser(base.module, base.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 @@ -100,18 +99,6 @@ 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

0 comments on commit 6c82292

Please sign in to comment.