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

[Heartbeat] Fix bug where enabled: false is ignored. #27615

Merged
merged 1 commit into from
Aug 26, 2021
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
11 changes: 5 additions & 6 deletions heartbeat/beater/heartbeat.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@
package beater

import (
"errors"
"fmt"
"time"

"github.com/pkg/errors"

"github.com/elastic/beats/v7/heartbeat/config"
"github.com/elastic/beats/v7/heartbeat/hbregistry"
"github.com/elastic/beats/v7/heartbeat/monitors"
"github.com/elastic/beats/v7/heartbeat/monitors/stdfields"
"github.com/elastic/beats/v7/heartbeat/scheduler"
"github.com/elastic/beats/v7/libbeat/autodiscover"
"github.com/elastic/beats/v7/libbeat/beat"
Expand Down Expand Up @@ -132,11 +130,12 @@ func (bt *Heartbeat) RunStaticMonitors(b *beat.Beat) (stop func(), err error) {
for _, cfg := range bt.config.Monitors {
created, err := factory.Create(b.Publisher, cfg)
if err != nil {
if err == stdfields.ErrPluginDisabled {
if errors.Is(err, monitors.ErrMonitorDisabled) {
logp.Info("skipping disabled monitor: %s", err)
continue // don't stop loading monitors just because they're disabled
}

return nil, errors.Wrap(err, "could not create monitor")
return nil, fmt.Errorf("could not create monitor: %w", err)
}

created.Start()
Expand All @@ -163,7 +162,7 @@ func (bt *Heartbeat) RunCentralMgmtMonitors(b *beat.Beat) {
func (bt *Heartbeat) RunReloadableMonitors(b *beat.Beat) (err error) {
// Check monitor configs
if err := bt.monitorReloader.Check(bt.dynamicFactory); err != nil {
logp.Error(errors.Wrap(err, "error loading reloadable monitors"))
logp.Error(fmt.Errorf("error loading reloadable monitors: %w", err))
}

// Execute the monitor
Expand Down
7 changes: 7 additions & 0 deletions heartbeat/monitors/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ import (
"github.com/elastic/beats/v7/libbeat/logp"
)

// ErrMonitorDisabled is returned when the monitor plugin is marked as disabled.
var ErrMonitorDisabled = errors.New("monitor not loaded, plugin is disabled")

// Monitor represents a configured recurring monitoring configuredJob loaded from a config file. Starting it
// will cause it to run with the given scheduler until Stop() is called.
type Monitor struct {
Expand Down Expand Up @@ -114,6 +117,10 @@ func newMonitorUnsafe(
return nil, err
}

if !config.Enabled() {
return nil, fmt.Errorf("monitor '%s' with id '%s' skipped: %w", standardFields.Name, standardFields.ID, ErrMonitorDisabled)
}

pluginFactory, found := registrar.Get(standardFields.Type)
if !found {
return nil, fmt.Errorf("monitor type %v does not exist, valid types are %v", standardFields.Type, registrar.MonitorNames())
Expand Down
7 changes: 0 additions & 7 deletions heartbeat/monitors/stdfields/stdfields.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ import (
"github.com/elastic/beats/v7/libbeat/common"
)

// ErrPluginDisabled is returned when the monitor plugin is marked as disabled.
var ErrPluginDisabled = errors.New("monitor not loaded, plugin is disabled")

type ServiceFields struct {
Name string `config:"name"`
}
Expand Down Expand Up @@ -60,9 +57,5 @@ func ConfigToStdMonitorFields(config *common.Config) (StdMonitorFields, error) {
}
}

if !mpi.Enabled {
return mpi, ErrPluginDisabled
}

return mpi, nil
}
4 changes: 4 additions & 0 deletions heartbeat/tests/system/config/heartbeat.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ heartbeat.monitors:
timeout: {{monitor.timeout}}
{% endif -%}

{%- if monitor.enabled is defined %}
enabled: {{monitor.enabled}}
{% endif -%}

{%- if monitor.tags is defined %}
tags:
{% for tag in monitor.tags -%}
Expand Down
3 changes: 2 additions & 1 deletion heartbeat/tests/system/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from elasticsearch import Elasticsearch
from beat.beat import INTEGRATION_TESTS
from beat import common_tests
from time import sleep


class Test(BaseTest, common_tests.TestExportsMixin):
Expand Down Expand Up @@ -53,7 +54,7 @@ def test_disabled(self):
)

heartbeat_proc = self.start_beat()
self.wait_until(lambda: self.log_contains("heartbeat is running"))
self.wait_until(lambda: self.log_contains("skipping disabled monitor"))
heartbeat_proc.check_kill_and_wait()

def test_fields_under_root(self):
Expand Down