Move exometer to prometheus #6
Conversation
NelsonVides
force-pushed
the
openapi_server
branch
from
cde9151 to
5c9d8a4
Compare
NelsonVides
force-pushed
the
prometheus
branch
from
ab53820 to
4f29bee
Compare
NelsonVides
force-pushed
the
openapi_server
branch
from
5c9d8a4 to
95a4a10
Compare
NelsonVides
force-pushed
the
prometheus
branch
3 times, most recently
from
7d8c9a5 to
5a0c529
Compare
NelsonVides
force-pushed
the
openapi_server
branch
2 times, most recently
from
1062fcd to
c6f6fe0
Compare
NelsonVides
force-pushed
the
prometheus
branch
from
5a0c529 to
01da3cd
Compare
NelsonVides
force-pushed
the
prometheus
branch
2 times, most recently
from
2fb819b to
86a30e8
Compare
doc/metrics.md
Outdated
| * example: `AMOC_GRAPHITE_PORT='2003'` | ||
| * `prometheus_ip` - prometheus IP: | ||
| * default value - `{127, 0, 0, 1}` | ||
| * example: `AMOC_PROMETHEUS_IP='{0, 0, 0, 0}'` |
There was a problem hiding this comment.
Do we need to specify this as an Erlang tuple? It's fine for me, but I guess it might be more difficult to e.g. share the same variable between components, or understand for non-erlangers.
There was a problem hiding this comment.
The env string is parsed as an erlang term so it would at the very least need to be '"127.0.0.1"' or "\"127.0.0.1\"", with the quotes inside quotes.
src/amoc_metrics.erl
Outdated
| init(gauge, Name) -> | ||
| prometheus_gauge:new([{name, Name}]); | ||
| init(times, Name) -> | ||
| prometheus_summary:new([{name, Name}]); |
There was a problem hiding this comment.
Here I'd suggest using a histogram (with a good set of buckets) instead, because for times like TTD, we really need the quantiles, while prometheus_summary would only give us the number of events and the sum of the values. Btw, prometheus_quantile_summary would not help, because it is completely broken and unusable (this is why are not using it in MIM). See https://github.com/esl/MongooseIM/blob/master/src/instrument/mongoose_instrument_prometheus.erl#L53 for reference.
doc/metrics.md
Outdated
| * default value - `"127.0.0.1"` | ||
| * example: `AMOC_PROMETHEUS_IP='"127,0,0,1"'` |
There was a problem hiding this comment.
These two lines are inconsistent now...
NelsonVides
force-pushed
the
prometheus
branch
from
dd8064e to
1b2ea18
Compare
There was a problem hiding this comment.
It looks good in general, but by just reading it the second time, I found new small issues. This makes me think: should we have some minimal unit tests here? Just like the ones for MongooseIM...
NelsonVides
force-pushed
the
prometheus
branch
2 times, most recently
from
3b53a95 to
016c23b
Compare
We can move support in amoc-arsenal from exometer to prometheus.
Note that this will potentially change the names of many metrics. Also note that it fixes an issue where env-loaded metrics would not be initialised for applications pulling this repo as a dependency (amoc-arsenal-xmpp) because it was checking the env of amoc-arsenal instead of the parent repo.
See also esl/amoc-arsenal-xmpp#40.