jaydoane commented on code in PR #4672: URL: https://github.com/apache/couchdb/pull/4672#discussion_r1260358904
########## src/couch_stats/README.md: ########## @@ -1,18 +1,13 @@ # couch_stats -couch_stats is a simple statistics collection app for Erlang applications. Its -core API is a thin wrapper around a stat storage library (currently Folsom,) but -abstracting over that library provides several benefits: +couch_stats is a simple statistics collection app for Erlang applications. It +uses https://www.erlang.org/doc/man/counters.html to implement counters, +gauages and histograms. By default histograms record 10 seconds worth of data, Review Comment: s/gauages/gauges/ ########## src/couch_stats/README.md: ########## @@ -1,18 +1,13 @@ # couch_stats -couch_stats is a simple statistics collection app for Erlang applications. Its -core API is a thin wrapper around a stat storage library (currently Folsom,) but -abstracting over that library provides several benefits: +couch_stats is a simple statistics collection app for Erlang applications. It +uses https://www.erlang.org/doc/man/counters.html to implement counters, +gauages and histograms. By default histograms record 10 seconds worth of data, +with a granularity of 1 second. -* All references to stat storage are in one place, so it's easy to swap - the module out. - -* Some common patterns, such as tying a process's lifetime to a counter value, - are straightforward to support. - -* Configuration can be managed in a single place - for example, it's much easier - to ensure that all histogram metrics use a 10-second sliding window if those - metrics are instantiated/configured centrally. +Stats are can be fetched with `couch_stats:fetch()`. That returns the current Review Comment: eliminate extra "are"? ########## src/couch_stats/src/couch_stats.erl: ########## @@ -29,102 +24,174 @@ update_gauge/2 ]). --include("couch_stats.hrl"). - --type response() :: ok | {error, unknown_metric}. +-type response() :: ok | {error, unknown_metric} | {error, invalid_metric}. -type stat() :: {any(), [{atom(), any()}]}. -start() -> - application:start(couch_stats). - -stop() -> - application:stop(couch_stats). - fetch() -> - couch_stats_aggregator:fetch(). + Seconds = couch_stats_util:histogram_interval_sec(), + StartSec = now_sec() - (Seconds - 1), + % Last -1 is because the interval ends are inclusive + couch_stats_util:fetch(stats(), StartSec, Seconds). reload() -> - couch_stats_aggregator:reload(). + couch_stats_server:reload(). -spec sample(any()) -> stat(). sample(Name) -> - [{Name, Info}] = folsom_metrics:get_metric_info(Name), - sample_type(Name, proplists:get_value(type, Info)). - --spec new(atom(), any()) -> ok | {error, metric_exists | unsupported_type}. -new(counter, Name) -> - case folsom_metrics:new_counter(Name) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(histogram, Name) -> - Time = config:get_integer("stats", "interval", ?DEFAULT_INTERVAL), - case folsom_metrics:new_histogram(Name, slide_uniform, {Time, 1024}) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(gauge, Name) -> - case folsom_metrics:new_gauge(Name) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(_, _) -> - {error, unsupported_type}. - -delete(Name) -> - folsom_metrics:delete_metric(Name). - -list() -> - folsom_metrics:get_metrics_info(). + Seconds = couch_stats_util:histogram_interval_sec(), + StartSec = now_sec() - (Seconds - 1), + % Last -1 is because the interval ends are inclusive + couch_stats_util:sample(Name, stats(), StartSec, Seconds). -spec increment_counter(any()) -> response(). increment_counter(Name) -> - notify_existing_metric(Name, {inc, 1}, counter). + increment_counter(Name, 1). -spec increment_counter(any(), pos_integer()) -> response(). increment_counter(Name, Value) -> - notify_existing_metric(Name, {inc, Value}, counter). + case couch_stats_util:get_counter(Name, stats()) of + {ok, Ctx} -> couch_stats_counter:increment(Ctx, Value); + {error, Error} -> {error, Error} + end. -spec decrement_counter(any()) -> response(). decrement_counter(Name) -> - notify_existing_metric(Name, {dec, 1}, counter). + decrement_counter(Name, 1). -spec decrement_counter(any(), pos_integer()) -> response(). decrement_counter(Name, Value) -> - notify_existing_metric(Name, {dec, Value}, counter). + case couch_stats_util:get_counter(Name, stats()) of + {ok, Ctx} -> couch_stats_counter:decrement(Ctx, Value); + {error, Error} -> {error, Error} + end. + +-spec update_gauge(any(), number()) -> response(). +update_gauge(Name, Value) -> + case couch_stats_util:get_gauge(Name, stats()) of + {ok, Ctx} -> couch_stats_gauge:update(Ctx, Value); + {error, Error} -> {error, Error} + end. -spec update_histogram (any(), number()) -> response(); (any(), function()) -> any(). update_histogram(Name, Fun) when is_function(Fun, 0) -> - Begin = os:timestamp(), + Begin = erlang:monotonic_time(), Result = Fun(), - Duration = timer:now_diff(os:timestamp(), Begin) div 1000, - case notify_existing_metric(Name, Duration, histogram) of + Dt = erlang:monotonic_time() - Begin, + Duration = erlang:convert_time_unit(Dt, native, millisecond), + case update_histogram(Name, Duration) of ok -> Result; {error, unknown_metric} -> - throw({unknown_metric, Name}) + throw({unknown_metric, Name}); + {error, invalid_metric} -> + throw({invalid_metric, Name}) end; update_histogram(Name, Value) when is_number(Value) -> - notify_existing_metric(Name, Value, histogram). - --spec update_gauge(any(), number()) -> response(). -update_gauge(Name, Value) -> - notify_existing_metric(Name, Value, gauge). - --spec notify_existing_metric(any(), any(), any()) -> response(). -notify_existing_metric(Name, Op, Type) -> - try - ok = folsom_metrics:notify_existing_metric(Name, Op, Type) - catch - _:_ -> - error_logger:error_msg("unknown metric: ~p", [Name]), - {error, unknown_metric} + case couch_stats_util:get_histogram(Name, stats()) of + {ok, Ctx} -> couch_stats_histogram:update(Ctx, now_sec(), Value); + {error, Error} -> {error, Error} end. --spec sample_type(any(), atom()) -> stat(). -sample_type(Name, histogram) -> - folsom_metrics:get_histogram_statistics(Name); -sample_type(Name, _) -> - folsom_metrics:get_metric_value(Name). +stats() -> + couch_stats_util:stats(). + +now_sec() -> + erlang:monotonic_time(second). + +-ifdef(TEST). + +-include_lib("couch/include/couch_eunit.hrl"). + +couch_stats_test_() -> + { + foreach, + fun setup/0, + fun teardown/1, + [ + ?TDEF_FE(t_can_fetch_metrics), + ?TDEF_FE(t_can_sample_metrics), + ?TDEF_FE(t_can_reload), Review Comment: Could you omit "can" in the names of these first three tests to be more consistent with the naming of the remaining tests? ########## src/couch_stats/src/couch_stats.erl: ########## @@ -29,102 +24,174 @@ update_gauge/2 ]). --include("couch_stats.hrl"). - --type response() :: ok | {error, unknown_metric}. +-type response() :: ok | {error, unknown_metric} | {error, invalid_metric}. -type stat() :: {any(), [{atom(), any()}]}. -start() -> - application:start(couch_stats). - -stop() -> - application:stop(couch_stats). - fetch() -> - couch_stats_aggregator:fetch(). + Seconds = couch_stats_util:histogram_interval_sec(), + StartSec = now_sec() - (Seconds - 1), + % Last -1 is because the interval ends are inclusive + couch_stats_util:fetch(stats(), StartSec, Seconds). reload() -> - couch_stats_aggregator:reload(). + couch_stats_server:reload(). -spec sample(any()) -> stat(). sample(Name) -> - [{Name, Info}] = folsom_metrics:get_metric_info(Name), - sample_type(Name, proplists:get_value(type, Info)). - --spec new(atom(), any()) -> ok | {error, metric_exists | unsupported_type}. -new(counter, Name) -> - case folsom_metrics:new_counter(Name) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(histogram, Name) -> - Time = config:get_integer("stats", "interval", ?DEFAULT_INTERVAL), - case folsom_metrics:new_histogram(Name, slide_uniform, {Time, 1024}) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(gauge, Name) -> - case folsom_metrics:new_gauge(Name) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(_, _) -> - {error, unsupported_type}. - -delete(Name) -> - folsom_metrics:delete_metric(Name). - -list() -> - folsom_metrics:get_metrics_info(). + Seconds = couch_stats_util:histogram_interval_sec(), + StartSec = now_sec() - (Seconds - 1), + % Last -1 is because the interval ends are inclusive + couch_stats_util:sample(Name, stats(), StartSec, Seconds). -spec increment_counter(any()) -> response(). increment_counter(Name) -> - notify_existing_metric(Name, {inc, 1}, counter). + increment_counter(Name, 1). -spec increment_counter(any(), pos_integer()) -> response(). increment_counter(Name, Value) -> - notify_existing_metric(Name, {inc, Value}, counter). + case couch_stats_util:get_counter(Name, stats()) of + {ok, Ctx} -> couch_stats_counter:increment(Ctx, Value); + {error, Error} -> {error, Error} + end. -spec decrement_counter(any()) -> response(). decrement_counter(Name) -> - notify_existing_metric(Name, {dec, 1}, counter). + decrement_counter(Name, 1). -spec decrement_counter(any(), pos_integer()) -> response(). decrement_counter(Name, Value) -> - notify_existing_metric(Name, {dec, Value}, counter). + case couch_stats_util:get_counter(Name, stats()) of + {ok, Ctx} -> couch_stats_counter:decrement(Ctx, Value); + {error, Error} -> {error, Error} + end. + +-spec update_gauge(any(), number()) -> response(). +update_gauge(Name, Value) -> + case couch_stats_util:get_gauge(Name, stats()) of + {ok, Ctx} -> couch_stats_gauge:update(Ctx, Value); + {error, Error} -> {error, Error} + end. -spec update_histogram (any(), number()) -> response(); (any(), function()) -> any(). update_histogram(Name, Fun) when is_function(Fun, 0) -> - Begin = os:timestamp(), + Begin = erlang:monotonic_time(), Result = Fun(), - Duration = timer:now_diff(os:timestamp(), Begin) div 1000, - case notify_existing_metric(Name, Duration, histogram) of + Dt = erlang:monotonic_time() - Begin, + Duration = erlang:convert_time_unit(Dt, native, millisecond), + case update_histogram(Name, Duration) of ok -> Result; {error, unknown_metric} -> - throw({unknown_metric, Name}) + throw({unknown_metric, Name}); + {error, invalid_metric} -> + throw({invalid_metric, Name}) end; update_histogram(Name, Value) when is_number(Value) -> - notify_existing_metric(Name, Value, histogram). - --spec update_gauge(any(), number()) -> response(). -update_gauge(Name, Value) -> - notify_existing_metric(Name, Value, gauge). - --spec notify_existing_metric(any(), any(), any()) -> response(). -notify_existing_metric(Name, Op, Type) -> - try - ok = folsom_metrics:notify_existing_metric(Name, Op, Type) - catch - _:_ -> - error_logger:error_msg("unknown metric: ~p", [Name]), - {error, unknown_metric} + case couch_stats_util:get_histogram(Name, stats()) of + {ok, Ctx} -> couch_stats_histogram:update(Ctx, now_sec(), Value); + {error, Error} -> {error, Error} end. --spec sample_type(any(), atom()) -> stat(). -sample_type(Name, histogram) -> - folsom_metrics:get_histogram_statistics(Name); -sample_type(Name, _) -> - folsom_metrics:get_metric_value(Name). +stats() -> + couch_stats_util:stats(). + +now_sec() -> + erlang:monotonic_time(second). + +-ifdef(TEST). + +-include_lib("couch/include/couch_eunit.hrl"). + +couch_stats_test_() -> + { + foreach, + fun setup/0, + fun teardown/1, + [ + ?TDEF_FE(t_can_fetch_metrics), + ?TDEF_FE(t_can_sample_metrics), + ?TDEF_FE(t_can_reload), + ?TDEF_FE(t_increment_counter), + ?TDEF_FE(t_decrement_counter), + ?TDEF_FE(t_update_gauge), + ?TDEF_FE(t_update_histogram), + ?TDEF_FE(t_update_histogram_fun), + ?TDEF_FE(t_accessing_invalid_metrics) Review Comment: Would using "access" here instead of "accessing" be more consistent? ########## src/couch_stats/src/couch_stats_gauge.erl: ########## @@ -0,0 +1,54 @@ +% Licensed under the Apache License, Version 2.0 (the "License"); you may not +% use this file except in compliance with the License. You may obtain a copy of +% the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +% License for the specific language governing permissions and limitations under +% the License. + +-module(couch_stats_gauge). + +-export([ + new/0, + update/2, + read/1 +]). + +new() -> + counters:new(1, [write_concurrency]). + +update(Ctx, Val) when is_integer(Val) -> + counters:put(Ctx, 1, Val). + +read(Ctx) -> + counters:get(Ctx, 1). + +-ifdef(TEST). + +-include_lib("couch/include/couch_eunit.hrl"). + +counter_test() -> + C = new(), Review Comment: Maybe use `G` for gauge here instead? ########## src/couch_stats/src/couch_stats_math.erl: ########## @@ -0,0 +1,404 @@ +% Licensed under the Apache License, Version 2.0 (the "License"); you may not +% use this file except in compliance with the License. You may obtain a copy of +% the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +% WARRANTIES OR CONDITIONS OF ANY KIND, eithe r express or implied. See the +% License for the specific language governing permissions and limitations under +% the License. + +-module(couch_stats_math). + +-export([ + summary/2 +]). + +% Stats are computed over two passes. First one, with acc1, gathers some +% basics, then the second pass, with acc2, computes additional, more complex, +% statistics. +% +-record(acc1, { + % Non-zero bins = [{BinIndex, Counts}, ...] + bins = [], + n = 0, + sum = 0, + sum_log = 0, + sum_inv = 0 +}). + +-record(acc2, { + diff_2_sum = 0, + diff_3_sum = 0, + diff_4_sum = 0, + % pXX = {Rank, PercentileValue} + p50 = {0, 0}, + p75 = {0, 0}, + p90 = {0, 0}, + p95 = {0, 0}, + p99 = {0, 0}, + p999 = {0, 0} +}). + +summary(Counter, BinCount) when is_tuple(Counter), is_integer(BinCount) -> + calc_stats(pass1(Counter, BinCount)). + +calc_stats(#acc1{n = 0}) -> + % Can't do much with 0 items. Instead of inserting division checks for + % 0 everywhere, just return a empty stats object + n0_stats(); +calc_stats(#acc1{n = N} = Stats) when is_integer(N), N > 0 -> + #acc1{bins = Bins, sum = Sum, sum_log = SumLog, sum_inv = SumInv} = Stats, + Mean = Sum / N, + Acc2 = pass2(Bins, N, Mean), + #acc2{ + diff_2_sum = Diff2Sum, + diff_3_sum = Diff3Sum, + diff_4_sum = Diff4Sum, + p50 = {_, P50}, + p75 = {_, P75}, + p90 = {_, P90}, + p95 = {_, P95}, + p99 = {_, P99}, + p999 = {_, P999} + } = Acc2, + Variance = Diff2Sum / N, + StdDev = math:sqrt(Variance), + [ + {n, N}, + {min, couch_stats_histogram:bin_min(element(1, hd(Bins)))}, + {max, couch_stats_histogram:bin_max(element(1, lists:last(Bins)))}, + {arithmetic_mean, Mean}, + {geometric_mean, math:exp(SumLog / N)}, + {harmonic_mean, N / SumInv}, + {median, P50}, + {variance, Variance}, + {standard_deviation, StdDev}, + {skewness, skewness(N, Diff3Sum, StdDev)}, + {kurtosis, kurtosis(N, Diff4Sum, StdDev)}, + {percentile, [ + {50, P50}, + {75, P75}, + {90, P90}, + {95, P95}, + {99, P99}, + {999, P999} + ]}, + % Emit for compatibility reasons + {histogram, [{0, 0}]} + ]. + +% Fist pass removes 0 bins and calculates some basics like sums, counts, +% sum logs. Some of these that can only be used after the second pass over Review Comment: Is the "that" a typo? ########## src/couch_stats/src/couch_stats.erl: ########## @@ -29,102 +24,174 @@ update_gauge/2 ]). --include("couch_stats.hrl"). - --type response() :: ok | {error, unknown_metric}. +-type response() :: ok | {error, unknown_metric} | {error, invalid_metric}. -type stat() :: {any(), [{atom(), any()}]}. -start() -> - application:start(couch_stats). - -stop() -> - application:stop(couch_stats). - fetch() -> - couch_stats_aggregator:fetch(). + Seconds = couch_stats_util:histogram_interval_sec(), + StartSec = now_sec() - (Seconds - 1), + % Last -1 is because the interval ends are inclusive + couch_stats_util:fetch(stats(), StartSec, Seconds). reload() -> - couch_stats_aggregator:reload(). + couch_stats_server:reload(). -spec sample(any()) -> stat(). sample(Name) -> - [{Name, Info}] = folsom_metrics:get_metric_info(Name), - sample_type(Name, proplists:get_value(type, Info)). - --spec new(atom(), any()) -> ok | {error, metric_exists | unsupported_type}. -new(counter, Name) -> - case folsom_metrics:new_counter(Name) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(histogram, Name) -> - Time = config:get_integer("stats", "interval", ?DEFAULT_INTERVAL), - case folsom_metrics:new_histogram(Name, slide_uniform, {Time, 1024}) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(gauge, Name) -> - case folsom_metrics:new_gauge(Name) of - ok -> ok; - {error, Name, metric_already_exists} -> {error, metric_exists} - end; -new(_, _) -> - {error, unsupported_type}. - -delete(Name) -> - folsom_metrics:delete_metric(Name). - -list() -> - folsom_metrics:get_metrics_info(). + Seconds = couch_stats_util:histogram_interval_sec(), + StartSec = now_sec() - (Seconds - 1), + % Last -1 is because the interval ends are inclusive + couch_stats_util:sample(Name, stats(), StartSec, Seconds). -spec increment_counter(any()) -> response(). increment_counter(Name) -> - notify_existing_metric(Name, {inc, 1}, counter). + increment_counter(Name, 1). -spec increment_counter(any(), pos_integer()) -> response(). increment_counter(Name, Value) -> - notify_existing_metric(Name, {inc, Value}, counter). + case couch_stats_util:get_counter(Name, stats()) of + {ok, Ctx} -> couch_stats_counter:increment(Ctx, Value); + {error, Error} -> {error, Error} + end. -spec decrement_counter(any()) -> response(). decrement_counter(Name) -> - notify_existing_metric(Name, {dec, 1}, counter). + decrement_counter(Name, 1). -spec decrement_counter(any(), pos_integer()) -> response(). decrement_counter(Name, Value) -> - notify_existing_metric(Name, {dec, Value}, counter). + case couch_stats_util:get_counter(Name, stats()) of + {ok, Ctx} -> couch_stats_counter:decrement(Ctx, Value); + {error, Error} -> {error, Error} + end. + +-spec update_gauge(any(), number()) -> response(). +update_gauge(Name, Value) -> + case couch_stats_util:get_gauge(Name, stats()) of + {ok, Ctx} -> couch_stats_gauge:update(Ctx, Value); + {error, Error} -> {error, Error} + end. -spec update_histogram (any(), number()) -> response(); (any(), function()) -> any(). update_histogram(Name, Fun) when is_function(Fun, 0) -> - Begin = os:timestamp(), + Begin = erlang:monotonic_time(), Result = Fun(), - Duration = timer:now_diff(os:timestamp(), Begin) div 1000, - case notify_existing_metric(Name, Duration, histogram) of + Dt = erlang:monotonic_time() - Begin, + Duration = erlang:convert_time_unit(Dt, native, millisecond), + case update_histogram(Name, Duration) of ok -> Result; {error, unknown_metric} -> - throw({unknown_metric, Name}) + throw({unknown_metric, Name}); + {error, invalid_metric} -> + throw({invalid_metric, Name}) end; update_histogram(Name, Value) when is_number(Value) -> - notify_existing_metric(Name, Value, histogram). - --spec update_gauge(any(), number()) -> response(). -update_gauge(Name, Value) -> - notify_existing_metric(Name, Value, gauge). - --spec notify_existing_metric(any(), any(), any()) -> response(). -notify_existing_metric(Name, Op, Type) -> - try - ok = folsom_metrics:notify_existing_metric(Name, Op, Type) - catch - _:_ -> - error_logger:error_msg("unknown metric: ~p", [Name]), - {error, unknown_metric} + case couch_stats_util:get_histogram(Name, stats()) of + {ok, Ctx} -> couch_stats_histogram:update(Ctx, now_sec(), Value); + {error, Error} -> {error, Error} end. --spec sample_type(any(), atom()) -> stat(). -sample_type(Name, histogram) -> - folsom_metrics:get_histogram_statistics(Name); -sample_type(Name, _) -> - folsom_metrics:get_metric_value(Name). +stats() -> + couch_stats_util:stats(). + +now_sec() -> + erlang:monotonic_time(second). + +-ifdef(TEST). + +-include_lib("couch/include/couch_eunit.hrl"). + +couch_stats_test_() -> + { + foreach, + fun setup/0, + fun teardown/1, + [ + ?TDEF_FE(t_can_fetch_metrics), + ?TDEF_FE(t_can_sample_metrics), + ?TDEF_FE(t_can_reload), + ?TDEF_FE(t_increment_counter), + ?TDEF_FE(t_decrement_counter), + ?TDEF_FE(t_update_gauge), + ?TDEF_FE(t_update_histogram), + ?TDEF_FE(t_update_histogram_fun), + ?TDEF_FE(t_accessing_invalid_metrics) + ] + }. + +setup() -> + test_util:start_couch([couch_replicator]). + +teardown(Ctx) -> + config:delete("stats", "interval", _Persist = false), + test_util:stop_couch(Ctx). + +t_can_fetch_metrics(_) -> + Metrics = fetch(), + ?assertEqual(map_size(stats()), length(Metrics)), + ?assertMatch([{_, [{value, _}, {type, _}, {desc, _}]} | _], Metrics). + +t_can_sample_metrics(_) -> + Hist = sample([fsync, time]), + ?assertMatch([{_Name, _Val} | _], Hist), + + Count = sample([fsync, count]), + ?assert(is_integer(Count)), + ?assert(Count >= 0), + + ?assertEqual(0, sample([couch_replicator, jobs, total])). + +t_can_reload(_) -> + % This is tested in details in couch_stats_server in detail Review Comment: Maybe just `% Tested in detail in couch_stats_server`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
