Add histogram metrics#18
Conversation
kjnilsson
left a comment
kjnilsson
left a comment
There was a problem hiding this comment.
a few comments on the API mostly
| Labels = maps:get(labels, Opts, #{}), | ||
| Help = maps:get(help, Opts, ""), | ||
| %% Store in persistent_term for fast observe path | ||
| persistent_term:put({?MODULE, Group, Id}, |
There was a problem hiding this comment.
I dont think we should do this for every histogram we want to create surely?
|
|
||
| %% @doc Record an observation. | ||
| -spec observe(group(), id(), non_neg_integer()) -> ok. | ||
| observe(Group, Id, Value) when is_integer(Value), Value >= 0 -> |
There was a problem hiding this comment.
rather than relying on persistent terms we should expect callers to retain an opaque type with all the counters and stuff necessary to perform the observation. E.g. the main seshat API assumes local state and I dont think we want a completely different approach here.
| %% A histogram metric type for seshat, backed by Erlang counters. | ||
| %% Histograms are registered in groups alongside regular counters | ||
| %% and can be exported via prom_format. | ||
| -module(seshat_histogram). |
There was a problem hiding this comment.
Do we need a new and different module for this or can the API needed be added to seshat?
This adds a osiris_histogram module adapted from the RabbitMQ message size histogram metrics. A histogram can be registered within a group with a custom set of buckets. Then callers record with `observe/3`.
|
Yeah I agree on the persistent term part. Better for seshat to expose the ref information you need and give you control over how to hold it. I'm not sure which approach I like for the API better ( |
2 participants
This adds a osiris_histogram module adapted from the RabbitMQ message size histogram metrics. A histogram can be registered within a group with a custom set of buckets. Then callers record with
observe/3.This is spun out of rabbitmq/osiris#215.