-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Refactor: Renaming prospector to input #6078
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,15 +24,15 @@ type clientEventer struct { | |
wgEvents eventCounter | ||
} | ||
|
||
// prospectorOutletConfig defines common prospector settings | ||
// inputOutletConfig defines common input settings | ||
// for the publisher pipline. | ||
type prospectorOutletConfig struct { | ||
type inputOutletConfig struct { | ||
// event processing | ||
common.EventMetadata `config:",inline"` // Fields and tags to add to events. | ||
Processors processors.PluginConfig `config:"processors"` | ||
|
||
// implicit event fields | ||
Type string `config:"type"` // prospector.type | ||
Type string `config:"type"` // input.type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case the comment is correct, this is a breaking change of the data format I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why its a breaking changes? The name of the private struct changed and the only the comment changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only indirectly breaking if the comment is correct. I assume when looking at it that you removed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not remove the |
||
|
||
// hidden filebeat modules settings | ||
Module string `config:"_module_name"` // hidden setting | ||
|
@@ -44,7 +44,7 @@ type prospectorOutletConfig struct { | |
} | ||
|
||
// NewOutletFactory creates a new outlet factory for | ||
// connecting a prospector to the publisher pipeline. | ||
// connecting an input to the publisher pipeline. | ||
func NewOutletFactory( | ||
done <-chan struct{}, | ||
pipeline beat.Pipeline, | ||
|
@@ -63,12 +63,12 @@ func NewOutletFactory( | |
return o | ||
} | ||
|
||
// Create builds a new Outleter, while applying common prospector settings. | ||
// Prospectors and all harvesters use the same pipeline client instance. | ||
// Create builds a new Outleter, while applying common input settings. | ||
// Inputs and all harvesters use the same pipeline client instance. | ||
// This guarantees ordering between events as required by the registrar for | ||
// file.State updates | ||
func (f *OutletFactory) Create(cfg *common.Config, dynFields *common.MapStrPointer) (Outleter, error) { | ||
config := prospectorOutletConfig{} | ||
config := inputOutletConfig{} | ||
if err := cfg.Unpack(&config); err != nil { | ||
return nil, err | ||
} | ||
|
@@ -99,6 +99,9 @@ func (f *OutletFactory) Create(cfg *common.Config, dynFields *common.MapStrPoint | |
fields["prospector"] = common.MapStr{ | ||
"type": config.Type, | ||
} | ||
fields["event"] = common.MapStr{ | ||
"type": config.Type, | ||
} | ||
} | ||
|
||
client, err := f.pipeline.ConnectWith(beat.ClientConfig{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,15 +21,16 @@ const ( | |
) | ||
|
||
type Config struct { | ||
Prospectors []*common.Config `config:"prospectors"` | ||
RegistryFile string `config:"registry_file"` | ||
RegistryFlush time.Duration `config:"registry_flush"` | ||
ConfigDir string `config:"config_dir"` | ||
ShutdownTimeout time.Duration `config:"shutdown_timeout"` | ||
Modules []*common.Config `config:"modules"` | ||
ConfigProspector *common.Config `config:"config.prospectors"` | ||
ConfigModules *common.Config `config:"config.modules"` | ||
Autodiscover *autodiscover.Config `config:"autodiscover"` | ||
Inputs []*common.Config `config:"inputs"` | ||
Prospectors []*common.Config `config:"prospectors"` | ||
RegistryFile string `config:"registry_file"` | ||
RegistryFlush time.Duration `config:"registry_flush"` | ||
ConfigDir string `config:"config_dir"` | ||
ShutdownTimeout time.Duration `config:"shutdown_timeout"` | ||
Modules []*common.Config `config:"modules"` | ||
ConfigInput *common.Config `config:"config.prospectors"` | ||
ConfigModules *common.Config `config:"config.modules"` | ||
Autodiscover *autodiscover.Config `config:"autodiscover"` | ||
} | ||
|
||
var ( | ||
|
@@ -82,7 +83,15 @@ func mergeConfigFiles(configFiles []string, config *Config) error { | |
return fmt.Errorf("Failed to read %s: %s", file, err) | ||
} | ||
|
||
config.Prospectors = append(config.Prospectors, tmpConfig.Filebeat.Prospectors...) | ||
if len(tmpConfig.Filebeat.Prospectors) > 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some system tests for this would also be nice. |
||
cfgwarn.Deprecate("7.0.0", "prospectors are deprecated, Use `inputs` instead.") | ||
if len(tmpConfig.Filebeat.Inputs) > 0 { | ||
return fmt.Errorf("prospectors and inputs used in the configuration file, define only inputs not both") | ||
} | ||
tmpConfig.Filebeat.Inputs = append(tmpConfig.Filebeat.Inputs, tmpConfig.Filebeat.Prospectors...) | ||
} | ||
|
||
config.Inputs = append(config.Inputs, tmpConfig.Filebeat.Inputs...) | ||
} | ||
|
||
return nil | ||
|
@@ -97,7 +106,7 @@ func (config *Config) FetchConfigs() error { | |
return nil | ||
} | ||
|
||
cfgwarn.Deprecate("7.0.0", "config_dir is deprecated. Use `filebeat.config.prospectors` instead.") | ||
cfgwarn.Deprecate("7.0.0", "config_dir is deprecated. Use `filebeat.config.inputs` instead.") | ||
|
||
// If configDir is relative, consider it relative to the config path | ||
configDir = paths.Resolve(paths.Config, configDir) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some system tests with the old config options to make sure this works as expected? We have done this for previous renamings and I think it's pretty useful to make sure it actually works and be remembered that we break something, when we remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, I was planning to do it in a dedicated PR, because right now all the tests use the old config options :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. Didn't realise it's all on the old config. Good to know.