Merge ~hloeung/content-cache-charm:metrics into content-cache-charm:master

Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: 0e91790adfd255cc8f23b73a9d1c5f460f89dd36
Merged at revision: 7cdad621afbaf34f094a19243c12b807536e841b
Proposed branch: ~hloeung/content-cache-charm:metrics
Merge into: content-cache-charm:master
Diff against target: 178 lines (+38/-12)
6 files modified
config.yaml (+6/-1)
lib/nginx.py (+7/-2)
reactive/content_cache.py (+4/-3)
templates/nginx_metrics_cfg.tmpl (+6/-2)
tests/unit/test_content_cache.py (+6/-1)
tests/unit/test_nginx.py (+9/-3)
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Content Cache Charmers Pending
Review via email: mp+398665@code.launchpad.net

Commit message

Enable Nginx basic server status metrics - LP:1916078

To post a comment you must log in.
Revision history for this message
Canonical IS Mergebot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Stuart Bishop (stub) wrote :

Looks good. Confirmed the usual use case is the metrics will be exposed via a telegraf subordinate.

review: Approve
Revision history for this message
Haw Loeung (hloeung) :
Revision history for this message
Canonical IS Mergebot (canonical-is-mergebot) wrote :

Change successfully merged at revision 7cdad621afbaf34f094a19243c12b807536e841b

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/config.yaml b/config.yaml
2index a7cd0e9..84f8834 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -45,7 +45,7 @@ options:
6 type: boolean
7 description: >
8 Export metrics for the number of requests and the number of cache hits.
9- Prometheus metrics are exposed at /metrics on port 9145.
10+ Prometheus metrics are exposed at /metrics on localhost:9145.
11 haproxy_hard_stop_after:
12 default: "5m"
13 type: string
14@@ -85,6 +85,11 @@ options:
15 description: >
16 Configure maximum number of connections per site on frontend
17 HAProxy. Defaults to auto-calculate (0).
18+ metrics_listen_address:
19+ type: string
20+ default: "localhost"
21+ description: >
22+ Restrict exposed metrics to listen on certain addresses.
23 nagios_context:
24 type: string
25 default: "juju"
26diff --git a/lib/nginx.py b/lib/nginx.py
27index d880d72..2b733a3 100644
28--- a/lib/nginx.py
29+++ b/lib/nginx.py
30@@ -9,6 +9,7 @@ from lib import utils
31
32
33 INDENT = ' ' * 4
34+METRICS_LISTEN = 'localhost'
35 METRICS_PORT = 9145
36 METRICS_SITE = 'nginx_metrics'
37 NGINX_BASE_PATH = '/etc/nginx'
38@@ -189,7 +190,7 @@ class NginxConf:
39
40 return changed
41
42- def toggle_metrics_site(self, enable_prometheus_metrics):
43+ def toggle_metrics_site(self, enable_prometheus_metrics, listen_address=None):
44 """Create/delete the metrics site configuration and links.
45
46 :param bool enable_prometheus_metrics: True if metrics are exposed to prometheus
47@@ -203,8 +204,12 @@ class NginxConf:
48 # If no cache metrics, remove the site
49 if not enable_prometheus_metrics:
50 return self._remove_metrics_site(available, enabled)
51+
52+ if listen_address is None:
53+ listen_address = METRICS_LISTEN
54+
55 template = self.jinja_env.get_template('templates/nginx_metrics_cfg.tmpl')
56- content = template.render({'nginx_conf_path': self.conf_path, 'port': METRICS_PORT})
57+ content = template.render({'nginx_conf_path': self.conf_path, 'address': listen_address, 'port': METRICS_PORT})
58 # Check if contents changed
59 try:
60 with open(available, 'r', encoding='utf-8') as f:
61diff --git a/reactive/content_cache.py b/reactive/content_cache.py
62index a215f62..d396779 100644
63--- a/reactive/content_cache.py
64+++ b/reactive/content_cache.py
65@@ -102,7 +102,7 @@ def service_start_or_reload():
66 reactive.clear_flag('content_cache.{}.reload-required'.format(name))
67
68
69-def configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
70+def configure_nginx_metrics(ngx_conf, enable_prometheus_metrics, listen_address):
71 """Configure nginx to expose metrics.
72
73 Create the dedicated server exposing the metrics and add the logging of the cache hits for the other sites.
74@@ -114,7 +114,7 @@ def configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
75 changed = False
76 if copy_file('files/prometheus.lua', os.path.join(ngx_conf.conf_path, 'prometheus.lua')):
77 changed = True
78- if ngx_conf.toggle_metrics_site(enable_prometheus_metrics):
79+ if ngx_conf.toggle_metrics_site(enable_prometheus_metrics, listen_address):
80 changed = True
81 old_ports = [int(port.split('/')[0]) for port in hookenv.opened_ports()]
82 hookenv.log("Current opened ports: {}".format(old_ports))
83@@ -187,7 +187,8 @@ def configure_nginx(conf_path=None):
84 hookenv.log('Wrote out new configs for site: {}:{}'.format(site, conf['listen_port']))
85 changed = True
86
87- if configure_nginx_metrics(ngx_conf, enable_prometheus_metrics):
88+ metrics_listen = config.get('metrics_listen_address', None)
89+ if configure_nginx_metrics(ngx_conf, enable_prometheus_metrics, listen_address=metrics_listen):
90 hookenv.log('nginx metrics exposed to prometheus')
91 changed = True
92
93diff --git a/templates/nginx_metrics_cfg.tmpl b/templates/nginx_metrics_cfg.tmpl
94index b3c8b9e..bad2db4 100644
95--- a/templates/nginx_metrics_cfg.tmpl
96+++ b/templates/nginx_metrics_cfg.tmpl
97@@ -4,14 +4,14 @@ init_by_lua_block {
98 prometheus = require("prometheus").init("prometheus_metrics")
99 http_request_total = prometheus:counter(
100 "nginx_http_request_total", "Number of HTTP requests per site", {"host", "status"})
101- cache_request_total = prometheus:counter(
102+ cache_request_total = prometheus:counter(
103 "nginx_cache_request_total", "Number of cache requests per site", {"host"})
104 cache_request_hit_total = prometheus:counter(
105 "nginx_cache_request_hit_total", "Number of cache hits per site", {"host"})
106 }
107
108 server {
109- listen {{ port }};
110+ listen {% if address %}{{address}}:{% endif %}{{port}};
111 allow all;
112 location /metrics {
113 content_by_lua_block {
114@@ -19,6 +19,10 @@ server {
115 }
116 }
117
118+ location /server_status {
119+ stub_status;
120+ }
121+
122 access_log /var/log/nginx/nginx_metrics-access.log;
123 error_log /var/log/nginx/nginx_metrics-error.log;
124 }
125diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
126index d8300a9..4d2a8e8 100644
127--- a/tests/unit/test_content_cache.py
128+++ b/tests/unit/test_content_cache.py
129@@ -1299,6 +1299,7 @@ site1.local:
130 'enable_cache_background_update': False,
131 'enable_cache_lock': False,
132 'enable_prometheus_metrics': True,
133+ 'metrics_listen_address': '127.0.0.2',
134 'sites': ngx_config,
135 'worker_connections': 768,
136 'worker_processes': 0,
137@@ -1344,7 +1345,11 @@ site1.local:
138 jinja_env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
139 template = jinja_env.get_template('templates/nginx_metrics_cfg.tmpl')
140 content = template.render(
141- {'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'), 'port': nginx.METRICS_PORT}
142+ {
143+ 'address': '127.0.0.2',
144+ 'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'),
145+ 'port': nginx.METRICS_PORT,
146+ }
147 )
148 want = content
149 test_file = os.path.join(self.tmpdir, 'sites-available/{0}.conf'.format(nginx.METRICS_SITE))
150diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
151index b76c674..b0abde9 100644
152--- a/tests/unit/test_nginx.py
153+++ b/tests/unit/test_nginx.py
154@@ -170,15 +170,21 @@ class TestLibNginx(unittest.TestCase):
155 script_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..')
156 env = jinja2.Environment(loader=jinja2.FileSystemLoader(script_dir))
157 template = env.get_template('templates/nginx_metrics_cfg.tmpl')
158- content = template.render({'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'), 'port': nginx.METRICS_PORT})
159+ content = template.render(
160+ {
161+ 'address': '',
162+ 'nginx_conf_path': os.path.join(self.tmpdir, 'conf.d'),
163+ 'port': nginx.METRICS_PORT,
164+ }
165+ )
166 nginx_metrics_conf = content
167
168 metrics_site_available = os.path.join(self.tmpdir, 'sites-available', metrics_site_conf)
169 metrics_site_enabled = os.path.join(self.tmpdir, 'sites-enabled', metrics_site_conf)
170- self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
171+ self.assertTrue(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True, listen_address=''))
172 # Write again with same contents, this time it should return 'False'
173 # as there's no change, thus no need to restart/reload Nginx.
174- self.assertFalse(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True))
175+ self.assertFalse(ngx_conf.toggle_metrics_site(enable_prometheus_metrics=True, listen_address=''))
176 self.assertTrue(os.path.exists(metrics_site_available))
177
178 # Compare what's been written out matches what's in tests/unit/files.

Subscribers

People subscribed via source and target branches