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

Check for removed settings on startup #4726

Merged
merged 1 commit into from
Jul 24, 2017
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
3 changes: 2 additions & 1 deletion auditbeat/module/audit/file/metricset.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/pkg/errors"

"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/metricbeat/mb"
"github.com/elastic/beats/metricbeat/mb/parse"
Expand Down Expand Up @@ -45,7 +46,7 @@ type MetricSet struct {
}

func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
logp.Beta("The %v metricset is an beta feature", metricsetName)
cfgwarn.Experimental("The %v metricset is an experimental feature", metricsetName)

config := defaultConfig
if err := base.Module().UnpackConfig(&config); err != nil {
Expand Down
3 changes: 2 additions & 1 deletion auditbeat/module/audit/kernel/audit_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/pkg/errors"

"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/monitoring"
"github.com/elastic/beats/metricbeat/mb"
Expand Down Expand Up @@ -47,7 +48,7 @@ type MetricSet struct {

// New constructs a new MetricSet.
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
logp.Beta("The %v metricset is a beta feature", metricsetName)
cfgwarn.Experimental("The %v metricset is a beta feature", metricsetName)

config := defaultConfig
if err := base.Module().UnpackConfig(&config); err != nil {
Expand Down
6 changes: 6 additions & 0 deletions filebeat/beater/filebeat.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/elastic/beats/libbeat/beat"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/monitoring"
"github.com/elastic/beats/libbeat/outputs/elasticsearch"
Expand Down Expand Up @@ -46,6 +47,11 @@ func New(b *beat.Beat, rawConfig *common.Config) (beat.Beater, error) {
return nil, fmt.Errorf("Error reading config file: %v", err)
}

err := cfgwarn.CheckRemoved5xSettings(rawConfig, "spool_size", "publish_async", "idle_timeout")
if err != nil {
return nil, err
}

moduleRegistry, err := fileset.NewModuleRegistry(config.Modules, b.Info.Version)
if err != nil {
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions filebeat/crawler/crawler.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/elastic/beats/filebeat/registrar"
"github.com/elastic/beats/libbeat/cfgfile"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
)

Expand Down Expand Up @@ -51,7 +52,7 @@ func (c *Crawler) Start(r *registrar.Registrar, configProspectors *common.Config
}

if configProspectors.Enabled() {
logp.Beta("Loading separate prospectors is enabled.")
cfgwarn.Beta("Loading separate prospectors is enabled.")

c.reloader = cfgfile.NewReloader(configProspectors)
factory := prospector.NewFactory(c.out, r, c.beatDone)
Expand All @@ -61,7 +62,7 @@ func (c *Crawler) Start(r *registrar.Registrar, configProspectors *common.Config
}

if configModules.Enabled() {
logp.Beta("Loading separate modules is enabled.")
cfgwarn.Beta("Loading separate modules is enabled.")

c.reloader = cfgfile.NewReloader(configModules)
factory := fileset.NewFactory(c.out, r, c.beatVersion, pipelineLoaderFactory, c.beatDone)
Expand Down
4 changes: 2 additions & 2 deletions filebeat/prospector/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"time"

cfg "github.com/elastic/beats/filebeat/config"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/common/cfgwarn"
)

var (
Expand All @@ -22,7 +22,7 @@ type prospectorConfig struct {

func (c *prospectorConfig) Validate() error {
if c.InputType != "" {
logp.Deprecate("6.0.0", "input_type prospector config is deprecated. Use type instead.")
cfgwarn.Deprecate("6.0.0", "input_type prospector config is deprecated. Use type instead.")
c.Type = c.InputType
}
return nil
Expand Down
3 changes: 2 additions & 1 deletion filebeat/prospector/log/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/elastic/beats/filebeat/harvester"
"github.com/elastic/beats/filebeat/harvester/reader"
"github.com/elastic/beats/filebeat/input/file"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/common/match"
"github.com/elastic/beats/libbeat/logp"
)
Expand Down Expand Up @@ -151,7 +152,7 @@ func (c *config) Validate() error {
}

if c.ScanSort != "" {
logp.Experimental("scan_sort is used.")
cfgwarn.Experimental("scan_sort is used.")

// Check input type
if _, ok := ValidScanSort[c.ScanSort]; !ok {
Expand Down
4 changes: 3 additions & 1 deletion filebeat/prospector/redis/prospector.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/elastic/beats/filebeat/harvester"
"github.com/elastic/beats/filebeat/input/file"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
)

Expand All @@ -23,7 +24,8 @@ type Prospector struct {

// NewProspector creates a new redis prospector
func NewProspector(cfg *common.Config, outletFactory channel.OutleterFactory) (*Prospector, error) {
logp.Experimental("Redis slowlog prospector is enabled.")
cfgwarn.Experimental("Redis slowlog prospector is enabled.")

config := defaultConfig

err := cfg.Unpack(&config)
Expand Down
3 changes: 2 additions & 1 deletion filebeat/prospector/udp/prospector.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/elastic/beats/filebeat/channel"
"github.com/elastic/beats/filebeat/harvester"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
)

Expand All @@ -14,7 +15,7 @@ type Prospector struct {
}

func NewProspector(cfg *common.Config, outlet channel.OutleterFactory) (*Prospector, error) {
logp.Experimental("UDP prospector type is used")
cfgwarn.Experimental("UDP prospector type is used")

out, err := outlet(cfg)
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion filebeat/tests/system/config/filebeat_modules.yml.j2
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
filebeat.idle_timeout: 0.5s
filebeat.registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default("registry")}}

output.elasticsearch.hosts: ["{{ elasticsearch_url }}"]
Expand Down
1 change: 0 additions & 1 deletion filebeat/tests/system/config/filebeat_prospectors.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ filebeat.prospectors:
scan_frequency: 0.5s
encoding: {{prospector.encoding | default("plain") }}
{% endfor %}
filebeat.idle_timeout: 0.5s
filebeat.registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default("registry")}}

output.file:
Expand Down
20 changes: 20 additions & 0 deletions filebeat/tests/system/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,23 @@ def test_base(self):
output = self.read_output()[0]
assert "@timestamp" in output
assert "prospector.type" in output

def test_invalid_config_with_removed_settings(self):
"""
Checks if filebeat fails to load if removed settings have been used:
"""
self.render_config_template(console={"pretty": "false"})

exit_code = self.run_beat(extra_args=[
"-E", "filebeat.spool_size=2048",
"-E", "filebeat.publish_async=true",
"-E", "filebeat.idle_timeout=1s",
])

assert exit_code == 1
assert self.log_contains("setting 'filebeat.spool_size'"
" has been removed")
assert self.log_contains("setting 'filebeat.publish_async'"
" has been removed")
assert self.log_contains("setting 'filebeat.idle_timeout'"
" has been removed")
3 changes: 2 additions & 1 deletion libbeat/api/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import (
"strconv"

"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/monitoring"
)

// Start starts the metrics api endpoint on the configured host and port
func Start(cfg *common.Config, info common.BeatInfo) {
logp.Beta("Metrics endpoint is enabled.")
cfgwarn.Beta("Metrics endpoint is enabled.")
config := DefaultConfig
cfg.Unpack(&config)

Expand Down
12 changes: 9 additions & 3 deletions libbeat/beat/beat.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"github.com/elastic/beats/libbeat/api"
"github.com/elastic/beats/libbeat/cfgfile"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/common/file"
"github.com/elastic/beats/libbeat/dashboards"
"github.com/elastic/beats/libbeat/logp"
Expand Down Expand Up @@ -290,7 +291,7 @@ func (b *Beat) launch(bt Creator) error {

// If -configtest was specified, exit now prior to run.
if cfgfile.IsTestConfig() {
logp.Deprecate("6.0", "-configtest flag has been deprecated, use configtest subcommand")
cfgwarn.Deprecate("6.0", "-configtest flag has been deprecated, use configtest subcommand")
fmt.Println("Config OK")
return GracefulExit
}
Expand All @@ -299,7 +300,7 @@ func (b *Beat) launch(bt Creator) error {

// TODO Deprecate this in favor of setup subcommand (7.0)
if *setup {
logp.Deprecate("6.0", "-setup flag has been deprectad, use setup subcommand")
cfgwarn.Deprecate("6.0", "-setup flag has been deprectad, use setup subcommand")
}
err = b.loadDashboards(false)
if err != nil {
Expand Down Expand Up @@ -419,7 +420,7 @@ func (b *Beat) handleFlags() error {
flag.Parse()

if *printVersion {
logp.Deprecate("6.0", "-version flag has been deprectad, use version subcommand")
cfgwarn.Deprecate("6.0", "-version flag has been deprectad, use version subcommand")
fmt.Printf("%s version %s (%s), libbeat %s\n",
b.Info.Beat, b.Info.Version, runtime.GOARCH, version.GetDefaultVersion())
return GracefulExit
Expand Down Expand Up @@ -452,6 +453,11 @@ func (b *Beat) configure() error {
return fmt.Errorf("error unpacking config data: %v", err)
}

err = cfgwarn.CheckRemoved5xSettings(cfg, "queue_size", "bulk_queue_size")
if err != nil {
return err
}

if name := b.Config.Shipper.Name; name != "" {
b.Info.Name = name
}
Expand Down
24 changes: 24 additions & 0 deletions libbeat/common/cfgwarn/cfgwarn.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package cfgwarn

import (
"fmt"

"github.com/elastic/beats/libbeat/logp"
)

// Beta logs the usage of an beta feature.
func Beta(format string, v ...interface{}) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems to me this part would be more belong into logp?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with both locations, but I'm also curious why it got moved.

Copy link
Author

Choose a reason for hiding this comment

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

I introduced package cfgwarn to combine all configuration related warning messages in. Plus I find logp kind of messy right now.

logp.Warn("BETA: "+format, v...)
}

// Deprecate logs a deprecation message.
// The version string contains the version when the future will be removed
func Deprecate(version string, format string, v ...interface{}) {
postfix := fmt.Sprintf(" Will be removed in version: %s", version)
logp.Warn("DEPRECATED: "+format+postfix, v...)
}

// Experimental logs the usage of an experimental feature.
func Experimental(format string, v ...interface{}) {
logp.Warn("EXPERIMENTAL: "+format, v...)
}
49 changes: 49 additions & 0 deletions libbeat/common/cfgwarn/removed.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package cfgwarn

import (
"fmt"
"strings"

"github.com/joeshaw/multierror"

"github.com/elastic/beats/libbeat/common"
)

func CheckRemoved5xSettings(cfg *common.Config, settings ...string) error {
var errs multierror.Errors
for _, setting := range settings {
if err := CheckRemoved5xSetting(cfg, setting); err != nil {
errs = append(errs, err)
}
}

return errs.Err()
}

// CheckRemoved5xSetting prints a warning if the obsolete setting is used.
func CheckRemoved5xSetting(cfg *common.Config, setting string) error {
segments := strings.Split(setting, ".")

L := len(segments)
name := segments[L-1]
path := segments[:L-1]

current := cfg
for _, p := range path {
current, _ := current.Child(p, -1)
if current == nil {
break
}
}

// full path to setting not available -> setting not found
if current == nil {
return nil
}

if !current.HasField(name) {
return nil
}

return fmt.Errorf("setting '%v' has been removed", current.PathOf(name))
}
17 changes: 0 additions & 17 deletions libbeat/logp/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,23 +163,6 @@ func Critical(format string, v ...interface{}) {
msg(LOG_CRIT, "CRIT", format, v...)
}

// Deprecate logs a deprecation message.
// The version string contains the version when the future will be removed
func Deprecate(version string, format string, v ...interface{}) {
postfix := fmt.Sprintf(" Will be removed in version: %s", version)
Warn("DEPRECATED: "+format+postfix, v...)
}

// Experimental logs the usage of an experimental feature.
func Experimental(format string, v ...interface{}) {
Warn("EXPERIMENTAL: "+format, v...)
}

// Beta logs the usage of an beta feature.
func Beta(format string, v ...interface{}) {
Warn("BETA: "+format, v...)
}

// WTF prints the message at CRIT level and panics immediately with the same
// message
func WTF(format string, v ...interface{}) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/processors"
"github.com/elastic/beats/libbeat/processors/actions"
Expand All @@ -26,7 +27,7 @@ func newDockerMetadataProcessor(cfg *common.Config) (processors.Processor, error
}

func buildDockerMetadataProcessor(cfg *common.Config, watcherConstructor WatcherConstructor) (processors.Processor, error) {
logp.Beta("The add_docker_metadata processor is beta")
cfgwarn.Beta("The add_docker_metadata processor is beta")

config := defaultConfig()

Expand Down
3 changes: 2 additions & 1 deletion libbeat/processors/add_kubernetes_metadata/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/cfgwarn"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/processors"
"github.com/elastic/beats/libbeat/publisher/beat"
Expand Down Expand Up @@ -40,7 +41,7 @@ func init() {
}

func newKubernetesAnnotator(cfg *common.Config) (processors.Processor, error) {
logp.Beta("The kubernetes processor is beta")
cfgwarn.Beta("The kubernetes processor is beta")

config := defaultKuberentesAnnotatorConfig()

Expand Down
Loading