Skip to content

Commit bdb7d74

Browse files
andrewvcmergify-bot
andrewvc
authored and
mergify-bot
committed
[Heartbeat] Fix bug where enabled: false is ignored. (#27615)
Fixes #27599 This properly handles skipped monitors, and also fixes the broken test for this. Additionally, this switches to the go1.3+ way of handling error hierarchies in `heartbeat.go` using `errors.Is`. It also cleans up the code by no longer making a disabled config an `error` when parsing stdfields. It's now only an error when detected in `monitor.go` which is cleaner. (cherry picked from commit 9a517a7)
1 parent 9767379 commit bdb7d74

5 files changed

Lines changed: 18 additions & 14 deletions

File tree

‎heartbeat/beater/heartbeat.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,13 @@
1818
package beater
1919

2020
import (
21+
"errors"
2122
"fmt"
2223
"time"
2324

24-
"github.com/pkg/errors"
25-
2625
"github.com/elastic/beats/v7/heartbeat/config"
2726
"github.com/elastic/beats/v7/heartbeat/hbregistry"
2827
"github.com/elastic/beats/v7/heartbeat/monitors"
29-
"github.com/elastic/beats/v7/heartbeat/monitors/stdfields"
3028
"github.com/elastic/beats/v7/heartbeat/scheduler"
3129
"github.com/elastic/beats/v7/libbeat/autodiscover"
3230
"github.com/elastic/beats/v7/libbeat/beat"
@@ -132,11 +130,12 @@ func (bt *Heartbeat) RunStaticMonitors(b *beat.Beat) (stop func(), err error) {
132130
for _, cfg := range bt.config.Monitors {
133131
created, err := factory.Create(b.Publisher, cfg)
134132
if err != nil {
135-
if err == stdfields.ErrPluginDisabled {
133+
if errors.Is(err, monitors.ErrMonitorDisabled) {
134+
logp.Info("skipping disabled monitor: %s", err)
136135
continue // don't stop loading monitors just because they're disabled
137136
}
138137

139-
return nil, errors.Wrap(err, "could not create monitor")
138+
return nil, fmt.Errorf("could not create monitor: %w", err)
140139
}
141140

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

169168
// Execute the monitor

‎heartbeat/monitors/monitor.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ import (
3737
"github.com/elastic/beats/v7/libbeat/logp"
3838
)
3939

40+
// ErrMonitorDisabled is returned when the monitor plugin is marked as disabled.
41+
var ErrMonitorDisabled = errors.New("monitor not loaded, plugin is disabled")
42+
4043
// Monitor represents a configured recurring monitoring configuredJob loaded from a config file. Starting it
4144
// will cause it to run with the given scheduler until Stop() is called.
4245
type Monitor struct {
@@ -126,6 +129,10 @@ func newMonitorUnsafe(
126129
return nil, err
127130
}
128131

132+
if !config.Enabled() {
133+
return nil, fmt.Errorf("monitor '%s' with id '%s' skipped: %w", standardFields.Name, standardFields.ID, ErrMonitorDisabled)
134+
}
135+
129136
pluginFactory, found := registrar.Get(standardFields.Type)
130137
if !found {
131138
return nil, fmt.Errorf("monitor type %v does not exist, valid types are %v", standardFields.Type, registrar.MonitorNames())

‎heartbeat/monitors/stdfields/stdfields.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ import (
2626
"github.com/elastic/beats/v7/libbeat/common"
2727
)
2828

29-
// ErrPluginDisabled is returned when the monitor plugin is marked as disabled.
30-
var ErrPluginDisabled = errors.New("monitor not loaded, plugin is disabled")
31-
3229
type ServiceFields struct {
3330
Name string `config:"name"`
3431
}
@@ -60,9 +57,5 @@ func ConfigToStdMonitorFields(config *common.Config) (StdMonitorFields, error) {
6057
}
6158
}
6259

63-
if !mpi.Enabled {
64-
return mpi, ErrPluginDisabled
65-
}
66-
6760
return mpi, nil
6861
}

‎heartbeat/tests/system/config/heartbeat.yml.j2‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ heartbeat.monitors:
88
timeout: {{monitor.timeout}}
99
{% endif -%}
1010

11+
{%- if monitor.enabled is defined %}
12+
enabled: {{monitor.enabled}}
13+
{% endif -%}
14+
1115
{%- if monitor.tags is defined %}
1216
tags:
1317
{% for tag in monitor.tags -%}

‎heartbeat/tests/system/test_base.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from elasticsearch import Elasticsearch
66
from beat.beat import INTEGRATION_TESTS
77
from beat import common_tests
8+
from time import sleep
89

910

1011
class Test(BaseTest, common_tests.TestExportsMixin):
@@ -53,7 +54,7 @@ def test_disabled(self):
5354
)
5455

5556
heartbeat_proc = self.start_beat()
56-
self.wait_until(lambda: self.log_contains("heartbeat is running"))
57+
self.wait_until(lambda: self.log_contains("skipping disabled monitor"))
5758
heartbeat_proc.check_kill_and_wait()
5859

5960
def test_fields_under_root(self):

0 commit comments

Comments
 (0)