Merge ~ballot/charm-prometheus-alertmanager/+git/charm-prometheus-alertmanager:log-level into charm-prometheus-alertmanager:master

Proposed by Benjamin Allot
Status: Superseded
Proposed branch: ~ballot/charm-prometheus-alertmanager/+git/charm-prometheus-alertmanager:log-level
Merge into: charm-prometheus-alertmanager:master
Diff against target: 103 lines (+61/-0)
3 files modified
src/config.yaml (+6/-0)
src/reactive/alertmanager.py (+14/-0)
src/tests/unit/test_reactive_alertmanager.py (+41/-0)
Reviewer Review Type Date Requested Status
Drew Freiberger (community) Needs Fixing
BootStack Reviewers Pending
Review via email: mp+395874@code.launchpad.net

This proposal has been superseded by a proposal from 2021-01-27.

Commit message

Add the log-level config item

In order to easily debug, configuring the log-level of alertmanager can be handy.
The old default value of info (if not specified) is kept so an upgrade won't be bothered by the change.

To post a comment you must log in.
Revision history for this message
Drew Freiberger (afreiberger) wrote :

I'd like to see this charm go into a blocked state for an incorrectly configured log level value, and would appreciate seeing the values accepted for log-level in the config.yaml description.

I am also concerned that this change when upgrade-charm runs will restart the service (which is typically frowned upon) even if no configuration change is expected. Maybe the default should be a blank string, which equates to not setting a --log.level, which should keep the config file from being updated and the service from restarting based on service context change.

review: Needs Fixing
6037fcc... by Benjamin Allot

Set status to blocked upon wrong log-level

Addressed some review items:
* Provide the list of valid log-level in the config description
* Set the charm status to block in case of an invalid value
* Adapt the test accordingly

Unmerged commits

6037fcc... by Benjamin Allot

Set status to blocked upon wrong log-level

Addressed some review items:
* Provide the list of valid log-level in the config description
* Set the charm status to block in case of an invalid value
* Adapt the test accordingly

4a4132c... by Benjamin Allot

Add the log-level config item

In order to easily debug, configuring the log-level of alertmanager can
be handy.
The old default value of info (if not specified) is kept so an upgrade
won't be bothered by the change.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/config.yaml b/src/config.yaml
2index 5c7bf30..fe0c1b1 100644
3--- a/src/config.yaml
4+++ b/src/config.yaml
5@@ -9,6 +9,12 @@ options:
6 type: int
7 default: 9093
8 description: "Alertmanager listening port"
9+ log-level:
10+ type: string
11+ default: ""
12+ description: |
13+ Loggging level used for alertmanager.
14+ Valid values are "error", "warn", "info" and "debug".
15 external_url:
16 default: ""
17 type: string
18diff --git a/src/reactive/alertmanager.py b/src/reactive/alertmanager.py
19index e67f0e0..ef36c5d 100644
20--- a/src/reactive/alertmanager.py
21+++ b/src/reactive/alertmanager.py
22@@ -18,6 +18,7 @@ ALERTMANAGER_YML = "/var/snap/prometheus-alertmanager/current/alertmanager.yml"
23 ALERTMANAGER_DEF = "/var/snap/prometheus-alertmanager/current/daemon_arguments"
24 ALERTMANAGER_YML_TMPL = "alertmanager.yml.j2"
25 ALERTMANAGER_DEF_TMPL = "daemon_arguments.j2"
26+LOG_LEVELS = ["debug", "error", "info", "warn"]
27
28
29 def templates_changed(tmpl_list):
30@@ -121,8 +122,21 @@ def write_alertmanager_config_yml():
31 def write_alertmanager_config_def():
32 config = hookenv.config()
33 port = config.get("port", "9093")
34+ log_level = config.get("log-level")
35+ if log_level:
36+ if log_level in LOG_LEVELS:
37+ runtime_args("--log.level", "{}".format(log_level))
38+ else:
39+ hookenv.status_set(
40+ "blocked",
41+ "log-level {} is unknown."
42+ " Pick one of the following value: {}".format(
43+ log_level, ",".join(LOG_LEVELS))
44+ )
45+ return
46 check_ports(port)
47 runtime_args("--web.listen-address", ":{}".format(port))
48+
49 if config.get("external_url", False):
50 vars = {
51 "private_address": hookenv.unit_get("private-address"),
52diff --git a/src/tests/unit/test_reactive_alertmanager.py b/src/tests/unit/test_reactive_alertmanager.py
53index 98ff2f1..3655c0c 100644
54--- a/src/tests/unit/test_reactive_alertmanager.py
55+++ b/src/tests/unit/test_reactive_alertmanager.py
56@@ -176,6 +176,47 @@ class TestPrometheusContext(unittest.TestCase):
57 contents, "ARGS=.*-web.listen.address :{}.*".format(fake_port)
58 )
59
60+ def test_log_level_configs(self, mock_get_peers, mock_hookenv_config, *args):
61+ config = self.def_config
62+ fake_port = "9876"
63+ fake_ext_url = "http://foo:1234/bar"
64+ config.update({
65+ "port": fake_port,
66+ "external_url": fake_ext_url,
67+ "log-level": "debug",
68+ })
69+ mock_hookenv_config.return_value = config
70+ alertmanager.write_alertmanager_config_yml()
71+ alertmanager.write_alertmanager_config_def()
72+ # Verify etc/default/alertmanager has --log.level
73+ with open(self.cfg_def) as fh:
74+ contents = fh.read()
75+ self.assertRegex(contents, "ARGS=.*--log.level debug.*")
76+
77+ def test_bad_log_level_configs(
78+ self, mock_get_peers, mock_hookenv_config, mock_hookenv_unit_get,
79+ mock_valudate_config, mock_data_changed,
80+ mock_host_service_running, mock_host_service_restart,
81+ mock_hookenv_status_set, *args
82+ ):
83+ config = self.def_config
84+ fake_port = "9876"
85+ fake_ext_url = "http://foo:1234/bar"
86+ config.update({
87+ "port": fake_port,
88+ "external_url": fake_ext_url,
89+ "log-level": "dummylevel",
90+ })
91+ mock_hookenv_config.return_value = config
92+ alertmanager.write_alertmanager_config_yml()
93+ alertmanager.write_alertmanager_config_def()
94+ # Verify charm status is blocked
95+ mock_hookenv_status_set.assert_called_with(
96+ "blocked",
97+ "log-level dummylevel is unknown."
98+ " Pick one of the following value: debug,error,info,warn"
99+ )
100+
101 @mock.patch("reactive.alertmanager.remove_state")
102 def test_install_packages_conditionally_called(
103 self,

Subscribers

People subscribed via source and target branches

to all changes: