Skip to content

Commit 09b95f8

Browse files
NelsonVidesNelsonVides
committed
Optimise handling of integers and floats for counters, gauges, summaries
The point is that these tables keep separate positions in the tuple for integers and for floats, as on the integer position we can use the optimised `ets:update_counter/3,4`, and probabilistically integers are the most common input. The issue is that under some circumstances, integer values would be appended to the float position, which, while correct, misses the opportunity for the optimisation. Note too that gauges could take the value `undefined` to effectively block changes until explicitly set. This was implemented, but not tested, and in some edge-cases (the integer/float optimisation) it would be buggy.
1 parent fb5e95a commit 09b95f8

File tree

4 files changed

+133
-54
lines changed

4 files changed

+133
-54
lines changed

‎src/metrics/prometheus_counter.erl‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ inc(Caller) ->
8787
-behaviour(prometheus_metric).
8888
-behaviour(prometheus_collector).
8989

90+
%% Records will be triplets `{Key, ISum, FSum}`
91+
%% where `ISum` are integer sums, and `FSum` are float sums
92+
%% Keys use the scheduler index, because counters by definition aggregate
93+
%% even after parallel updates. Hence we can run updates without locks.
9094
-define(TABLE, ?PROMETHEUS_COUNTER_TABLE).
9195
-define(ISUM_POS, 2).
9296
-define(FSUM_POS, 3).
@@ -180,23 +184,23 @@ Raises:
180184
Registry :: prometheus_registry:registry(),
181185
Name :: prometheus_metric:name(),
182186
LabelValues :: prometheus_metric:labels(),
183-
Value :: non_neg_integer().
187+
Value :: non_neg_integer() | float().
184188
inc(Registry, Name, LabelValues, Value) when is_integer(Value), Value >= 0 ->
185189
Key = key(Registry, Name, LabelValues),
186190
Spec = {?ISUM_POS, Value},
187191
try
188192
ets:update_counter(?TABLE, Key, Spec)
189193
catch
190194
error:badarg ->
191-
insert_metric(Registry, Name, LabelValues, Value, fun inc/4)
195+
insert_metric_int(Registry, Name, LabelValues, Value, fun inc/4)
192196
end,
193197
ok;
194-
inc(Registry, Name, LabelValues, Value) when is_number(Value), Value >= 0 ->
198+
inc(Registry, Name, LabelValues, Value) when is_float(Value), Value >= 0 ->
195199
Key = key(Registry, Name, LabelValues),
196200
Spec = [{{Key, '$1', '$2'}, [], [{{{Key}, '$1', {'+', '$2', Value}}}]}],
197201
case ets:select_replace(?TABLE, Spec) of
198202
0 ->
199-
insert_metric(Registry, Name, LabelValues, Value, fun inc/4);
203+
insert_metric_float(Registry, Name, LabelValues, Value, fun inc/4);
200204
1 ->
201205
ok
202206
end;
@@ -349,9 +353,16 @@ collect_metrics(Name, {CLabels, Labels, Registry}) ->
349353
deregister_select(Registry, Name) ->
350354
[{{{Registry, Name, '_', '_'}, '_', '_'}, [], [true]}].
351355

352-
insert_metric(Registry, Name, LabelValues, Value, ConflictCB) ->
353-
prometheus_metric:check_mf_exists(?TABLE, Registry, Name, LabelValues),
356+
insert_metric_int(Registry, Name, LabelValues, Value, ConflictCB) ->
357+
Counter = {key(Registry, Name, LabelValues), Value, 0},
358+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
359+
360+
insert_metric_float(Registry, Name, LabelValues, Value, ConflictCB) ->
354361
Counter = {key(Registry, Name, LabelValues), 0, Value},
362+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
363+
364+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter) ->
365+
prometheus_metric:check_mf_exists(?TABLE, Registry, Name, LabelValues),
355366
case ets:insert_new(?TABLE, Counter) of
356367
%% some sneaky process already inserted
357368
false ->

‎src/metrics/prometheus_gauge.erl‎

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,13 @@ Gauge is a metric that represents a single numerical value that can arbitrarily
1515
A Gauge is typically used for measured values like temperatures or current memory usage,
1616
but also \"counts\" that can go up and down, like the number of running processes.
1717
18+
It can take the value of undefined when the value is not known.
19+
In that case, increments and decrements will not be possible
20+
and explicitly setting to a new value will be necessary.
21+
1822
Example use cases for Gauges:
1923
20-
* Inprogress requests
24+
* In-progress requests
2125
* Number of items in a queue
2226
* Free memory
2327
* Total memory
@@ -95,6 +99,8 @@ track_checked_out_sockets(CheckoutFun) ->
9599
-behaviour(prometheus_metric).
96100
-behaviour(prometheus_collector).
97101

102+
%% Records will be triplets `{Key, ISum, FSum}`
103+
%% where `IGauge` are integer gauges, and `FGauge` are float gauges
98104
-define(TABLE, ?PROMETHEUS_GAUGE_TABLE).
99105
-define(IGAUGE_POS, 2).
100106
-define(FGAUGE_POS, 3).
@@ -183,23 +189,33 @@ Raises:
183189
Registry :: prometheus_registry:registry(),
184190
Name :: prometheus_metric:name(),
185191
LabelValues :: prometheus_metric:labels(),
186-
Value :: number().
187-
set(Registry, Name, LabelValues, Value) ->
188-
Update =
189-
case Value of
190-
_ when is_number(Value) ->
191-
[{?IGAUGE_POS, 0}, {?FGAUGE_POS, Value}];
192-
undefined ->
193-
[{?IGAUGE_POS, undefined}, {?FGAUGE_POS, undefined}];
194-
_ ->
195-
erlang:error({invalid_value, Value, "set accepts only numbers and 'undefined'"})
196-
end,
192+
Value :: undefined | number().
193+
set(Registry, Name, LabelValues, Value) when is_integer(Value) ->
194+
Update = [{?IGAUGE_POS, Value}, {?FGAUGE_POS, 0}],
197195
case ets:update_element(?TABLE, {Registry, Name, LabelValues}, Update) of
198196
false ->
199-
insert_metric(Registry, Name, LabelValues, Value, fun set/4);
197+
insert_metric_int(Registry, Name, LabelValues, Value, fun set/4);
200198
true ->
201199
ok
202-
end.
200+
end;
201+
set(Registry, Name, LabelValues, Value) when is_float(Value) ->
202+
Update = [{?IGAUGE_POS, 0}, {?FGAUGE_POS, Value}],
203+
case ets:update_element(?TABLE, {Registry, Name, LabelValues}, Update) of
204+
false ->
205+
insert_metric_float(Registry, Name, LabelValues, Value, fun set/4);
206+
true ->
207+
ok
208+
end;
209+
set(Registry, Name, LabelValues, undefined) ->
210+
Update = [{?IGAUGE_POS, undefined}, {?FGAUGE_POS, undefined}],
211+
case ets:update_element(?TABLE, {Registry, Name, LabelValues}, Update) of
212+
false ->
213+
insert_metric_undefined(Registry, Name, LabelValues, undefined, fun set/4);
214+
true ->
215+
ok
216+
end;
217+
set(_, _, _, Value) ->
218+
erlang:error({invalid_value, Value, "set accepts only numbers and 'undefined'"}).
203219

204220
?DOC(#{equiv => inc(default, Name, [], 1)}).
205221
-spec inc(prometheus_metric:name()) -> ok.
@@ -236,19 +252,21 @@ Raises:
236252
LabelValues :: prometheus_metric:labels(),
237253
Value :: number().
238254
inc(Registry, Name, LabelValues, Value) when is_integer(Value) ->
255+
Key = key(Registry, Name, LabelValues),
256+
Spec = {?IGAUGE_POS, Value},
239257
try
240-
ets:update_counter(?TABLE, {Registry, Name, LabelValues}, {?IGAUGE_POS, Value})
258+
ets:update_counter(?TABLE, Key, Spec)
241259
catch
242260
error:badarg ->
243-
maybe_insert_metric_for_inc(Registry, Name, LabelValues, Value)
261+
maybe_insert_metric_for_inc_int(Registry, Name, LabelValues, Value)
244262
end,
245263
ok;
246-
inc(Registry, Name, LabelValues, Value) when is_number(Value) ->
264+
inc(Registry, Name, LabelValues, Value) when is_float(Value) ->
247265
Key = key(Registry, Name, LabelValues),
248-
Spec = [{{Key, '$1', '$2'}, [], [{{{Key}, '$1', {'+', '$2', Value}}}]}],
266+
Spec = [{{Key, '$1', '$2'}, [{is_integer, '$1'}], [{{{Key}, '$1', {'+', '$2', Value}}}]}],
249267
case ets:select_replace(?TABLE, Spec) of
250268
0 ->
251-
insert_metric(Registry, Name, LabelValues, Value, fun inc/4);
269+
maybe_insert_metric_for_inc_float(Registry, Name, LabelValues, Value);
252270
1 ->
253271
ok
254272
end;
@@ -522,20 +540,40 @@ collect_metrics(Name, {CLabels, Labels, Registry, DU}) ->
522540
key(Registry, Name, LabelValues) ->
523541
{Registry, Name, LabelValues}.
524542

525-
maybe_insert_metric_for_inc(Registry, Name, LabelValues, Value) ->
543+
maybe_insert_metric_for_inc_int(Registry, Name, LabelValues, Value) ->
544+
case ets:lookup(?TABLE, {Registry, Name, LabelValues}) of
545+
[{_Key, undefined, undefined}] ->
546+
erlang:error({invalid_operation, 'inc/dec', "Can't inc/dec undefined"});
547+
_ ->
548+
insert_metric_int(Registry, Name, LabelValues, Value, fun inc/4)
549+
end.
550+
551+
maybe_insert_metric_for_inc_float(Registry, Name, LabelValues, Value) ->
526552
case ets:lookup(?TABLE, {Registry, Name, LabelValues}) of
527553
[{_Key, undefined, undefined}] ->
528554
erlang:error({invalid_operation, 'inc/dec', "Can't inc/dec undefined"});
529555
_ ->
530-
insert_metric(Registry, Name, LabelValues, Value, fun inc/4)
556+
insert_metric_float(Registry, Name, LabelValues, Value, fun inc/4)
531557
end.
532558

533559
deregister_select(Registry, Name) ->
534560
[{{{Registry, Name, '_'}, '_', '_'}, [], [true]}].
535561

536-
insert_metric(Registry, Name, LabelValues, Value, ConflictCB) ->
562+
insert_metric_int(Registry, Name, LabelValues, Value, ConflictCB) ->
563+
Counter = {key(Registry, Name, LabelValues), Value, 0},
564+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
565+
566+
insert_metric_float(Registry, Name, LabelValues, Value, ConflictCB) ->
567+
Counter = {key(Registry, Name, LabelValues), 0, Value},
568+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
569+
570+
insert_metric_undefined(Registry, Name, LabelValues, Value, ConflictCB) ->
571+
Counter = {key(Registry, Name, LabelValues), undefined, undefined},
572+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
573+
574+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter) ->
537575
prometheus_metric:check_mf_exists(?TABLE, Registry, Name, LabelValues),
538-
case ets:insert_new(?TABLE, {{Registry, Name, LabelValues}, 0, Value}) of
576+
case ets:insert_new(?TABLE, Counter) of
539577
%% some sneaky process already inserted
540578
false ->
541579
ConflictCB(Registry, Name, LabelValues, Value);
@@ -546,7 +584,7 @@ insert_metric(Registry, Name, LabelValues, Value, ConflictCB) ->
546584
load_all_values(Registry, Name) ->
547585
ets:match(?TABLE, {{Registry, Name, '$1'}, '$2', '$3'}).
548586

549-
sum(_IValue, undefined) ->
587+
sum(undefined, _FValue) ->
550588
undefined;
551589
sum(IValue, FValue) ->
552590
IValue + FValue.

‎src/metrics/prometheus_summary.erl‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ Example use cases for Summaries:
1515
* Request size;
1616
* Response size.
1717
18+
This keeps track of the number of events, and the sum of their values.
19+
This allows you to calculate the average value of each event,
20+
and with enough datapoints, to keep track of its average.
21+
1822
Example:
1923
2024
```erlang
@@ -72,20 +76,11 @@ observe_response(Size) ->
7276
-behaviour(prometheus_collector).
7377

7478
-define(TABLE, ?PROMETHEUS_SUMMARY_TABLE).
79+
-define(COUNTER_POS, 2).
7580
-define(ISUM_POS, 3).
7681
-define(FSUM_POS, 4).
77-
-define(COUNTER_POS, 2).
7882
-define(WIDTH, 16).
7983

80-
-type conflict_cb() :: fun(
81-
(
82-
prometheus_registry:registry(),
83-
prometheus_metric:name(),
84-
prometheus_metric:labels(),
85-
number()
86-
) -> ok
87-
).
88-
8984
?DOC("""
9085
Creates a summary using `Spec`.
9186
@@ -173,15 +168,15 @@ observe(Registry, Name, LabelValues, Value) when is_integer(Value) ->
173168
ets:update_counter(?TABLE, Key, Spec)
174169
catch
175170
error:badarg ->
176-
insert_metric(Registry, Name, LabelValues, Value, fun observe/4)
171+
insert_metric_int(Registry, Name, LabelValues, Value, fun observe/4)
177172
end,
178173
ok;
179174
observe(Registry, Name, LabelValues, Value) when is_number(Value) ->
180175
Key = key(Registry, Name, LabelValues),
181176
Spec = [{{Key, '$1', '$2', '$3'}, [], [{{{Key}, {'+', '$1', 1}, '$2', {'+', '$3', Value}}}]}],
182177
case ets:select_replace(?TABLE, Spec) of
183178
0 ->
184-
insert_metric(Registry, Name, LabelValues, Value, fun observe/4);
179+
insert_metric_float(Registry, Name, LabelValues, Value, fun observe/4);
185180
1 ->
186181
ok
187182
end;
@@ -429,15 +424,17 @@ raise_error_if_quantile_label_found("quantile") ->
429424
raise_error_if_quantile_label_found(Label) ->
430425
Label.
431426

432-
-spec insert_metric(Registry, Name, LabelValues, Value, ConflictCB) -> ok when
433-
Registry :: prometheus_registry:registry(),
434-
Name :: prometheus_metric:name(),
435-
LabelValues :: prometheus_metric:labels(),
436-
Value :: number(),
437-
ConflictCB :: conflict_cb().
438-
insert_metric(Registry, Name, LabelValues, Value, ConflictCB) ->
427+
insert_metric_int(Registry, Name, LabelValues, Value, ConflictCB) ->
428+
Counter = {key(Registry, Name, LabelValues), 1, Value, 0},
429+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
430+
431+
insert_metric_float(Registry, Name, LabelValues, Value, ConflictCB) ->
432+
Counter = {key(Registry, Name, LabelValues), 1, 0, Value},
433+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter).
434+
435+
insert_metric(Registry, Name, LabelValues, Value, ConflictCB, Counter) ->
439436
prometheus_metric:check_mf_exists(?TABLE, Registry, Name, LabelValues),
440-
case ets:insert_new(?TABLE, {key(Registry, Name, LabelValues), 1, 0, Value}) of
437+
case ets:insert_new(?TABLE, Counter) of
441438
%% some sneaky process already inserted
442439
false ->
443440
ConflictCB(Registry, Name, LabelValues, Value);

‎test/eunit/metric/prometheus_gauge_tests.erl‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ prometheus_format_test_() ->
99
fun test_registration/1,
1010
fun test_errors/1,
1111
fun test_set/1,
12+
fun test_set_undefined/1,
1213
fun test_inc/1,
1314
fun test_dec/1,
1415
fun test_set_to_current_time/1,
@@ -44,6 +45,7 @@ test_registration(_) ->
4445

4546
test_errors(_) ->
4647
prometheus_gauge:new([{name, with_label}, {labels, [label]}, {help, ""}]),
48+
prometheus_gauge:new([{name, no_labels}, {help, ""}]),
4749

4850
%% basic name/labels/help validations test
4951
[
@@ -69,10 +71,19 @@ test_errors(_) ->
6971
{invalid_value, "qwe", "inc accepts only numbers"},
7072
prometheus_gauge:inc(pool_size, [], "qwe")
7173
),
74+
?_assertError(
75+
{invalid_value, <<"qwe">>, "dec accepts only numbers"},
76+
prometheus_gauge:dec(no_labels, <<"qwe">>)
77+
),
7278
?_assertError(
7379
{invalid_value, "qwe", "dec accepts only numbers"},
7480
prometheus_gauge:dec(pool_size, [], "qwe")
7581
),
82+
?_assertError(
83+
{invalid_value, "qwe", "dec accepts only numbers"},
84+
prometheus_gauge:dec(default, pool_size, [], "qwe")
85+
),
86+
7687
?_assertError(
7788
{invalid_value, "qwe", "track_inprogress accepts only functions"},
7889
prometheus_gauge:track_inprogress(pool_size, "qwe")
@@ -157,18 +168,40 @@ test_errors(_) ->
157168

158169
test_set(_) ->
159170
prometheus_gauge:new([{name, pool_size}, {labels, [client]}, {help, ""}]),
171+
prometheus_gauge:set(pool_size, [redis], undefined),
172+
ValueUndefined = prometheus_gauge:value(pool_size, [redis]),
173+
prometheus_gauge:set(pool_size, [float_pool], 92.3),
174+
ValueFloat = prometheus_gauge:value(pool_size, [float_pool]),
160175
prometheus_gauge:set(pool_size, [mongodb], 100),
161-
Value = prometheus_gauge:value(pool_size, [mongodb]),
162-
prometheus_gauge:set(pool_size, [mongodb], 105),
163176
Value1 = prometheus_gauge:value(pool_size, [mongodb]),
177+
prometheus_gauge:set(pool_size, [mongodb], 105),
178+
Value2 = prometheus_gauge:value(pool_size, [mongodb]),
164179
prometheus_gauge:reset(pool_size, [mongodb]),
165180
RValue = prometheus_gauge:value(pool_size, [mongodb]),
166181
[
167-
?_assertEqual(100, Value),
168-
?_assertEqual(105, Value1),
182+
?_assertEqual(undefined, ValueUndefined),
183+
?_assertEqual(92.3, ValueFloat),
184+
?_assertEqual(100, Value1),
185+
?_assertEqual(105, Value2),
169186
?_assertEqual(0, RValue)
170187
].
171188

189+
test_set_undefined(_) ->
190+
prometheus_gauge:new([{name, pool_size}, {labels, [client]}, {help, ""}]),
191+
prometheus_gauge:set(pool_size, [redis], undefined),
192+
ValueUndefined = prometheus_gauge:value(pool_size, [redis]),
193+
[
194+
?_assertError(
195+
{invalid_operation, 'inc/dec', "Can't inc/dec undefined"},
196+
prometheus_gauge:inc(pool_size, [redis], 1)
197+
),
198+
?_assertError(
199+
{invalid_operation, 'inc/dec', "Can't inc/dec undefined"},
200+
prometheus_gauge:inc(pool_size, [redis], 0.8)
201+
),
202+
?_assertEqual(undefined, ValueUndefined)
203+
].
204+
172205
test_inc(_) ->
173206
prometheus_gauge:new([{name, pool_size}, {labels, [client]}, {help, ""}]),
174207
prometheus_gauge:new([{name, temperature}, {help, ""}]),

0 commit comments

Comments
 (0)