[prometheus] exporter prototype #629

mikz · 2018-02-27T17:21:08Z

introduce metrics phase to export metrics
executor prints collects the response from prometheus
metrics can be also published from any other phase or timer

Example

See for policy changes and metrics examples:
3scale/apicast-cloud-hosted#5

maneta · 2018-02-27T20:52:12Z

gateway/config/development.lua

@@ -28,5 +28,6 @@ return {
    lua_code_cache = 'on',
    configuration_loader = 'lazy',
    configuration_cache = 0,
-    configuration = data_url('application/json', configuration)
+    configuration = data_url('application/json', configuration),
+    port = { metrics = 9100 },


@mikz maybe reserve a port for the APIcast endpoint so we can publish it on the Prometheus Default Port Allocation list?

I think we should agree how we want to proceed first.
We are not going to reserve a port for every app we have, right? We need some generic port for openresty exporter?

Got It, I thought that at least for APICast we were willing to create a specific "exporter" but yes makes more sense to create a generic port for openresty exporter.

To me it doesn't really make sense to reserve a port for each APP, but this is not a /generic/ openresty prometheus exporter, so, should we define a port for the "APIcast framework" and reuse that one in all our applications?

That makes sense if we want to converge all our proxies running on the "APIcast framework".

Reserved 9421.

We should converge them all, now with policies it should be easier ;)

maneta

👍

davidor · 2018-02-28T09:31:24Z

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Error loading policy chain configuration JSON with null value [PR #626](https://github.com/3scale/apicast/pull/626)
 - Splitted `resolv.conf` in lines,to avoid commented lines  [PR #618](https://github.com/3scale/apicast/pull/618)

+## Addwed


davidor · 2018-02-28T09:40:25Z

gateway/conf/nginx.conf.liquid

@@ -106,6 +106,24 @@ http {
    {% include "conf.d/apicast.conf" %}
  }

+  {% if port.metrics %}
+    lua_shared_dict prometheus_metrics 16M;


Should this be configurable?

Probably, but I'd wait with some real operations use to see if it is really needed.

Hah. This just gave me an idea for another exporter: shmem dict stats about free space.

Done in 3scale/apicast-cloud-hosted@09f65c9

davidor · 2018-02-28T09:41:32Z

gateway/conf/nginx.conf.liquid

+        content_by_lua_block { require('apicast.executor'):metrics() }
+      }
+
+      location /nginx_status {


Why is this needed?

It is used in https://github.com/3scale/apicast-cloud-hosted/pull/5/files#diff-047b1780b0ffeb4eba7b3d05beb76d5eR77 to extract the connection stats.
There is no easy way to add that location from customization, so I added it here as this will become part of upstream at some point.

davidor · 2018-02-28T09:42:21Z

gateway/config/development.lua

@@ -28,5 +28,6 @@ return {
    lua_code_cache = 'on',
    configuration_loader = 'lazy',
    configuration_cache = 0,
-    configuration = data_url('application/json', configuration)
+    configuration = data_url('application/json', configuration),
+    port = { metrics = 9421 }, -- see https://github.com/prometheus/prometheus/wiki/Default-port-allocations


davidor · 2018-02-28T09:47:08Z

gateway/src/apicast/policy.lua

@@ -13,7 +13,7 @@ local PHASES = {
    'rewrite', 'access',
    'content', 'balancer',
    'header_filter', 'body_filter',
-    'post_action',  'log'
+    'post_action',  'log', 'metrics',


We should add new phases to the policies doc: https://github.com/3scale/apicast/blob/master/doc/policies.md#policies

Done in c44b062

davidor · 2018-02-28T09:52:53Z

gateway/src/apicast/executor.lua

@@ -58,4 +59,11 @@ function _M:context(phase)
    return shared_build_context(self)
 end

+local metrics = _M.metrics
+--- Render metrics from all policies.
+function _M:metrics(...)


Maybe we should add a test for this. Define 2 policies with only a metrics phase that incr some counter and make sure that this returns the sum, or something similar.

Yeah but this does not really "return" anything. It renders the output.
I'll definitely do that when moving some policies with metrics to upstream from 3scale/apicast-cloud-hosted#5.

davidor · 2018-02-28T10:05:44Z

gateway/src/apicast/prometheus.lua

+
+  local metrics = { }
+  local __call = function(_, type, name, ...)
+    local metric_name = assert(name, 'missing metric name')


Why do we check name and type? Can't we delegate this to the Prometheus lib?

Because we actually use it. The type gets a function that is later called and the name is used as a key to store the metric. I had few tricky errors so decided to go for explicit ones.

davidor · 2018-02-28T10:07:48Z

spec/prometheus_spec.lua

+      prometheus = assert(require('apicast.prometheus'))
+    end)
+
+    it('can be called', function()


Should we change this to test that it delegates counter, gauge, etc. to the Prometheus lib?

Yep, makes sense.

octobot assigned mikz Feb 27, 2018

mikz requested review from maneta and davidor February 27, 2018 17:21

mikz force-pushed the prometheus-exporter branch from cfa4364 to 2aa9754 Compare February 27, 2018 20:34

maneta reviewed Feb 27, 2018

View reviewed changes

maneta approved these changes Feb 27, 2018

View reviewed changes

mikz mentioned this pull request Feb 27, 2018

[apicast] prometheus metrics policy 3scale/apicast-cloud-hosted#5

Closed

mikz force-pushed the prometheus-exporter branch 5 times, most recently from 70e7c6e to 6749c54 Compare February 28, 2018 08:12

davidor reviewed Feb 28, 2018

View reviewed changes

mikz force-pushed the prometheus-exporter branch from 6749c54 to 780f029 Compare February 28, 2018 09:36

jmprusi approved these changes Feb 28, 2018

View reviewed changes

davidor reviewed Feb 28, 2018

View reviewed changes

mikz added 2 commits February 28, 2018 14:57

[apicast] make apicast policy be created as other policies

af14a54

[prometheus] create prometheus exporter skeleton

619bc40

mikz force-pushed the prometheus-exporter branch from c44b062 to 619bc40 Compare February 28, 2018 13:57

davidor approved these changes Feb 28, 2018

View reviewed changes

mikz merged commit c9df38c into master Feb 28, 2018

mikz deleted the prometheus-exporter branch February 28, 2018 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[prometheus] exporter prototype #629

[prometheus] exporter prototype #629

mikz commented Feb 27, 2018 •

edited

Loading

maneta Feb 27, 2018

mikz Feb 27, 2018

maneta Feb 27, 2018

jmprusi Feb 28, 2018

mikz Feb 28, 2018

mikz Feb 28, 2018

jmprusi Feb 28, 2018

maneta left a comment

davidor Feb 28, 2018

davidor Feb 28, 2018

mikz Feb 28, 2018

mikz Feb 28, 2018

mikz Feb 28, 2018

davidor Feb 28, 2018

mikz Feb 28, 2018

davidor Feb 28, 2018

davidor Feb 28, 2018

davidor Feb 28, 2018

mikz Feb 28, 2018

davidor Feb 28, 2018

mikz Feb 28, 2018

davidor Feb 28, 2018

mikz Feb 28, 2018

davidor Feb 28, 2018

mikz Feb 28, 2018

mikz Feb 28, 2018

[prometheus] exporter prototype #629

[prometheus] exporter prototype #629

Conversation

mikz commented Feb 27, 2018 • edited Loading

Example

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maneta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz commented Feb 27, 2018 •

edited

Loading