Bugfix for imetrics_vm_metrics (#27)

- Time is now converted to an integer - Tests have been added - Bugfix for varz module attempting to display the wrong type of metrics
relaypro-open · Mar 27, 2024 · de2ade7 · de2ade7
1 parent d3df4d0
commit de2ade7
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 5 deletions.
diff --git a/src/imetrics_http_server.erl b/src/imetrics_http_server.erl
@@ -110,6 +110,7 @@ start_server(Port) ->
     ).
 
 handle_info({'EXIT', _From, Reason}, State) ->
+    io:format("imetrics_http_server trapped EXIT for reason ~p~n", [Reason]),
     {stop, Reason, State}.
 
 terminate(_Reason, _State = #state{cowboy_pid = Pid}) when is_pid(Pid) ->

diff --git a/src/imetrics_vm_metrics.erl b/src/imetrics_vm_metrics.erl
@@ -3,7 +3,6 @@
 -export([start_link/0]).
 -export([init/1, handle_cast/2, handle_call/3, handle_info/2, terminate/2, code_change/3]).
 -export([install/0, proc_count/2]).
--define(RefreshInterval, 60000). % 60 seconds
 
 start_link() ->
     gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
@@ -64,10 +63,10 @@ handle_info(refresh_gauges, State) ->
     imetrics:set_gauge(erlang_vm, #{ vm_metric => atom_count }, AtomCount),
     imetrics:set_gauge(erlang_vm, #{ vm_metric => atom_limit }, AtomLimit),
     imetrics:set_gauge(erlang_vm, #{ vm_metric => process_count }, ProcessCount),
-    imetrics:set_gauge(erlang_vm, #{ vm_metric => last_update_time }, erlang:timestamp()),
+    imetrics:set_gauge(erlang_vm, #{ vm_metric => last_update_time }, os:system_time(second)),
 
-    % recalculate after the interval time has passed
-    erlang:send_after(?RefreshInterval, self(), refresh_gauges),
+    % recalculate after the interval time has passed. default interval is 60 seconds.
+    erlang:send_after(application:get_env(imetrics, vm_metrics_refresh_interval, 60_000), self(), refresh_gauges),
 
     % We don't need to reply nor update the state.
     {noreply, State};

diff --git a/src/openmetrics.erl b/src/openmetrics.erl
@@ -36,6 +36,8 @@ deliver_data(Req, _Type = types, Data) ->
     lists:foreach(fun(Value) -> Func(Req, Value) end, Data).
 
 deliver_metricfamily(Req, {Name, {Type, MetricValue}}) ->
+    % as new types of metrics are added to this case statement, they should be added
+    % to the exclusion list in varz.erl
     ShouldPrintMetric =
         case {Type, application:get_env(imetrics, strict_openmetrics_compat, false)} of
             {counter, _} ->

diff --git a/src/varz.erl b/src/varz.erl
@@ -21,7 +21,9 @@ get(Req) ->
                 [
                     {MetricName, MetricPoints}
                  || {MetricName, {Type, MetricPoints}} <- imetrics:get_with_types(),
-                    not ((Type =:= counter) or (Type =:= gauge))
+                    % as new types of metrics are added to openmetrics.erl, they should be added
+                    % to the exclusion list below:
+                    not ((Type =:= counter) or (Type =:= gauge) or (Type =:= histogram) or (Type =:= info))
                 ]
             end)
     end.

diff --git a/test/imetrics_vm_metrics_tests.erl b/test/imetrics_vm_metrics_tests.erl
@@ -0,0 +1,93 @@
+-module(imetrics_vm_metrics_tests).
+-include_lib("eunit/include/eunit.hrl").
+
+start() ->
+    application:load(imetrics),
+    application:set_env(imetrics, http_server_port, 8087),
+    application:ensure_all_started(imetrics),
+    #{}.
+
+stop(_Fixture) ->
+    application:stop(imetrics).
+
+%% http tests
+http_test_() ->
+    {setup,
+        fun start_http/0,
+        fun stop/1,
+        fun http_test/1}. 
+
+start_http() ->
+    F = start(),
+    ok = imetrics_http_server:await(1000),
+    {ok, Port} = imetrics_http_server:port(),
+    F#{port => Port}.
+
+http_test(#{port := Port}) ->
+    http_test_blank(#{port => Port}) ++
+    http_test_vm_metrics(#{port => Port}).
+
+http_test_blank(#{port := Port}) ->
+    Url = "http://localhost:"++integer_to_list(Port)++"/metrics",
+    {ok, {Status, _Headers, Body}} = 
+        httpc:request(get, {Url, []}, [], []),
+    {_Vsn, Code, _Friendly} = Status,
+
+    Res = [
+        "# EOF"
+    ],
+    Res2 = string:join(Res, "\n") ++ "\n",
+    [
+        ?_assertEqual(Res2, Body),
+        ?_assertEqual(200, Code)
+    ].
+
+http_test_vm_metrics(#{port := Port}) ->
+    % reduce to 5 seconds from the default of 60s
+    application:set_env(imetrics, vm_metrics_refresh_interval, 5_000),
+    imetrics_vm_metrics:install(),
+
+    timer:sleep(1_000), % sleep for 1 second to allow the vm_metrics to populate
+
+    Url = "http://localhost:"++integer_to_list(Port)++"/metrics",
+    {ok, {Status1, _Headers1, Body1}} = 
+        httpc:request(get, {Url, []}, [], []),
+    {_Vsn1, Code1, _Friendly1} = Status1,
+
+    timer:sleep(6_000), % sleep for 6 seconds for the metrics to update
+
+    {ok, {Status2, _Headers2, Body2}} = 
+        httpc:request(get, {Url, []}, [], []),
+    {_Vsn2, Code2, _Friendly2} = Status2,
+
+    timer:sleep(1_000), % sleep for 1 second, we don't expect the metrics to update
+
+    {ok, {Status3, _Headers3, Body3}} = 
+        httpc:request(get, {Url, []}, [], []),
+    {_Vsn3, Code3, _Friendly3} = Status3,
+
+    [
+        ?_assertNotMatch(nomatch, string:find(Body1, "vm_metric=\"atom_limit\"")),
+        ?_assertNotMatch(nomatch, string:find(Body1, "vm_metric=\"max_memory\"")),
+        ?_assertNotMatch(nomatch, string:find(Body1, "vm_metric=\"max_message_queue_len\"")),
+        ?_assertNotMatch(nomatch, string:find(Body1, "vm_metric=\"atom_count\"")),
+        ?_assertNotMatch(nomatch, string:find(Body1, "vm_metric=\"process_count\"")),
+        ?_assertNotMatch(nomatch, string:find(Body1, "vm_metric=\"last_update_time\"")),
+        ?_assertNotEqual(Body1, Body2),
+        ?_assertNotMatch(nomatch, string:find(Body2, "vm_metric=\"atom_limit\"")),
+        ?_assertNotMatch(nomatch, string:find(Body2, "vm_metric=\"max_memory\"")),
+        ?_assertNotMatch(nomatch, string:find(Body2, "vm_metric=\"max_message_queue_len\"")),
+        ?_assertNotMatch(nomatch, string:find(Body2, "vm_metric=\"atom_count\"")),
+        ?_assertNotMatch(nomatch, string:find(Body2, "vm_metric=\"process_count\"")),
+        ?_assertNotMatch(nomatch, string:find(Body2, "vm_metric=\"last_update_time\"")),
+        ?_assertEqual(Body2, Body3),
+        ?_assertNotMatch(nomatch, string:find(Body3, "vm_metric=\"atom_limit\"")),
+        ?_assertNotMatch(nomatch, string:find(Body3, "vm_metric=\"max_memory\"")),
+        ?_assertNotMatch(nomatch, string:find(Body3, "vm_metric=\"max_message_queue_len\"")),
+        ?_assertNotMatch(nomatch, string:find(Body3, "vm_metric=\"atom_count\"")),
+        ?_assertNotMatch(nomatch, string:find(Body3, "vm_metric=\"process_count\"")),
+        ?_assertNotMatch(nomatch, string:find(Body3, "vm_metric=\"last_update_time\"")),
+        ?_assertEqual(200, Code1),
+        ?_assertEqual(200, Code2),
+        ?_assertEqual(200, Code3)
+    ].