Improve error handling in metric API

big_tests/tests/graphql_metric_SUITE.erl

@@ -5,7 +5,8 @@
 -compile([export_all, nowarn_export_all]).
 
 -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]).
--import(graphql_helper, [execute_command/4, get_ok_value/2, get_unauthorized/1]).
+-import(graphql_helper, [execute_command/4, get_ok_value/2, get_unauthorized/1,
+                         get_err_msg/1, get_err_code/1]).
 
 suite() ->
     MIM2NodeName = maps:get(node, distributed_helper:mim2()),
@@ -27,15 +28,26 @@ metrics_tests() ->
     [get_all_metrics,
      get_all_metrics_check_by_type,
      get_by_name_global_erlang_metrics,
+     get_metrics_by_empty_name,
+     get_metrics_by_nonexistent_name,
      get_process_queue_length,
      get_inet_stats,
      get_vm_stats_memory,
      get_all_metrics_as_dicts,
      get_by_name_metrics_as_dicts,
+     get_metrics_as_dicts_by_nonexistent_name,
      get_metrics_as_dicts_with_key_one,
+     get_metrics_as_dicts_with_nonexistent_key,
+     get_metrics_as_dicts_incorrect_params,
+     get_metrics_as_dicts_empty_params,
      get_cluster_metrics,
      get_by_name_cluster_metrics_as_dicts,
-     get_mim2_cluster_metrics].
+     get_mim2_cluster_metrics,
+     get_by_nonexisting_name_cluster_metrics_as_dicts,
+     get_cluster_metrics_for_nonexistent_nodes,
+     get_cluster_metrics_with_nonexistent_key,
+     get_cluster_metrics_incorrect_params,
+     get_cluster_metrics_empty_params].
 
 domain_admin_metrics_tests() ->
     [domain_admin_get_metrics,
@@ -126,6 +138,16 @@ get_by_name_global_erlang_metrics(Config) ->
     %% Other metrics are filtered out
     undef = maps:get(ReadsKey, Map, undef).
 
+get_metrics_by_empty_name(Config) ->
+    Result = get_metrics([], Config),
+    ParsedResult = get_ok_value([data, metric, getMetrics], Result),
+    lists:foreach(fun check_metric_by_type/1, ParsedResult).
+
+get_metrics_by_nonexistent_name(Config) ->
+    Result = get_metrics([<<"not_existing">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result)).
+
 get_process_queue_length(Config) ->
     Result = get_metrics([<<"global">>, <<"processQueueLengths">>], Config),
     ParsedResult = get_ok_value([data, metric, getMetrics], Result),
@@ -167,6 +189,11 @@ get_by_name_metrics_as_dicts(Config) ->
                           check_spiral_dict(Dict)
             end, ParsedResult).
 
+get_metrics_as_dicts_by_nonexistent_name(Config) ->
+    Result = get_metrics_as_dicts_by_name([<<"not_existing">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result)).
+
 get_metrics_as_dicts_with_key_one(Config) ->
     Result = get_metrics_as_dicts_with_keys([<<"one">>], Config),
     ParsedResult = get_ok_value([data, metric, getMetricsAsDicts], Result),
@@ -175,6 +202,36 @@ get_metrics_as_dicts_with_key_one(Config) ->
     [#{<<"key">> := <<"one">>, <<"value">> := One}] = maps:get(SentName, Map),
     true = is_integer(One).
 
+get_metrics_as_dicts_with_nonexistent_key(Config) ->
+    Result = get_metrics_as_dicts_with_keys([<<"not_existing">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_key_found">>, get_err_code(Result)).
+
+get_metrics_as_dicts_incorrect_params(Config) ->
+    %% Correct name, incorrect keys
+    Result = get_metrics_as_dicts([<<"_">>, <<"xmppStanzaSent">>], [<<"wrong">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_key_found">>, get_err_code(Result)),
+    %% Incorrect name, correct keys
+    Result2 = get_metrics_as_dicts([<<"wrong">>], [<<"one">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result2), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result2)),
+    %% Incorrect name, incorrect keys
+    Result3 = get_metrics_as_dicts([<<"wrong">>], [<<"wrong">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result3), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result3)).
+
+get_metrics_as_dicts_empty_params(Config) ->
+    %% Empty name
+    Result = get_metrics_as_dicts([], [<<"median">>], Config),
+    ParsedResult = get_ok_value([data, metric, getMetricsAsDicts], Result),
+    Map = dict_objects_to_map(ParsedResult),
+    true = maps:size(Map) > 40 andalso maps:size(Map) < 50,
+    %% Empty keys
+    Result2 = get_metrics_as_dicts([<<"global">>, <<"erlang">>], [], Config),
+    ParsedResult2 = get_ok_value([data, metric, getMetricsAsDicts], Result2),
+    true = length(ParsedResult2) == 2.
+
 get_cluster_metrics(Config) ->
     %% We will have at least these two nodes
     Node1 = atom_to_binary(maps:get(node, distributed_helper:mim())),
@@ -205,6 +262,61 @@ get_mim2_cluster_metrics(Config) ->
     [#{<<"node">> := Node, <<"result">> := ResList}] = ParsedResult,
     check_node_result_is_valid(ResList, true).
 
+get_cluster_metrics_for_nonexistent_nodes(Config) ->
+    Result = get_cluster_metrics_as_dicts_for_nodes([<<"nonexistent">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result)).
+
+get_by_nonexisting_name_cluster_metrics_as_dicts(Config) ->
+    Result = get_cluster_metrics_as_dicts_by_name([<<"nonexistent">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result)).
+
+get_cluster_metrics_with_nonexistent_key(Config) ->
+    Result = get_cluster_metrics_as_dicts_with_keys([<<"nonexistent">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_key_found">>, get_err_code(Result)).
+
+get_cluster_metrics_incorrect_params(Config) ->
+    Node = atom_to_binary(maps:get(node, distributed_helper:mim())),
+    %% Name doesn't contain an existing key
+    Result = get_cluster_metrics_as_dicts([<<"global">>, <<"erlang">>], [<<"one">>], [Node],
+                                          Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result), <<"No value found">>)),
+    ?assertEqual(<<"no_key_found">>, get_err_code(Result)),
+    %% Incorrect name
+    Result2 = get_cluster_metrics_as_dicts([<<"incorrect">>], [<<"one">>], [Node], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result2), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result2)),
+    %% Incorrect key
+    Result3 = get_cluster_metrics_as_dicts([<<"global">>, <<"erlang">>], [<<"wrong">>], [Node],
+                                           Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result3), <<"No value found">>)),
+    ?assertEqual(<<"no_key_found">>, get_err_code(Result3)),
+    %% Incorrect node
+    Result4 = get_cluster_metrics_as_dicts([<<"global">>, <<"erlang">>], [<<"ets_limit">>],
+                                           [<<"bad">>], Config),
+    ?assertNotEqual(nomatch, binary:match(get_err_msg(Result4), <<"No value found">>)),
+    ?assertEqual(<<"no_name_found">>, get_err_code(Result4)).
+
+get_cluster_metrics_empty_params(Config) ->
+    Node = atom_to_binary(maps:get(node, distributed_helper:mim2())),
+    %% Empty name
+    Result = get_cluster_metrics_as_dicts([], [<<"median">>], [Node], Config),
+    ParsedResult2 = get_ok_value([data, metric, getClusterMetricsAsDicts], Result),
+    [#{<<"node">> := Node, <<"result">> := ResList2}] = ParsedResult2,
+    true = length(ResList2) > 20 andalso length(ResList2) < 30,
+    %% Empty keys
+    Result2 = get_cluster_metrics_as_dicts([<<"_">>], [], [Node], Config),
+    ParsedResult = get_ok_value([data, metric, getClusterMetricsAsDicts], Result2),
+    [#{<<"node">> := Node, <<"result">> := ResList}] = ParsedResult,
+    check_node_result_is_valid(ResList, true),
+    %% Empty nodes
+    Result3 = get_cluster_metrics_as_dicts([<<"_">>, <<"erlang">>], [<<"ets_limit">>], [], Config),
+    ParsedResult3 = get_ok_value([data, metric, getClusterMetricsAsDicts], Result3),
+    Map = node_objects_to_map(ParsedResult3),
+    true = maps:size(Map) > 1.
+
 check_node_result_is_valid(ResList, MetricsAreGlobal) ->
     %% Check that result contains something
     Map = dict_objects_to_map(ResList),
@@ -277,6 +389,10 @@ get_metrics(Name, Config) ->
 get_metrics_as_dicts(Config) ->
     execute_command(<<"metric">>, <<"getMetricsAsDicts">>, #{}, Config).
 
+get_metrics_as_dicts(Name, Keys, Config) ->
+    Vars = #{<<"name">> => Name, <<"keys">> => Keys},
+    execute_command(<<"metric">>, <<"getMetricsAsDicts">>, Vars, Config).
+
 get_metrics_as_dicts_by_name(Name, Config) ->
     Vars = #{<<"name">> => Name},
     execute_command(<<"metric">>, <<"getMetricsAsDicts">>, Vars, Config).
@@ -288,6 +404,10 @@ get_metrics_as_dicts_with_keys(Keys, Config) ->
 get_cluster_metrics_as_dicts(Config) ->
     execute_command(<<"metric">>, <<"getClusterMetricsAsDicts">>, #{}, Config).
 
+get_cluster_metrics_as_dicts(Name, Keys, Nodes, Config) ->
+    Vars = #{<<"name">> => Name, <<"nodes">> => Nodes, <<"keys">> => Keys},
+    execute_command(<<"metric">>, <<"getClusterMetricsAsDicts">>, Vars, Config).
+
 get_cluster_metrics_as_dicts_by_name(Name, Config) ->
     Vars = #{<<"name">> => Name},
     execute_command(<<"metric">>, <<"getClusterMetricsAsDicts">>, Vars, Config).
@@ -296,6 +416,10 @@ get_cluster_metrics_as_dicts_for_nodes(Nodes, Config) ->
     Vars = #{<<"nodes">> => Nodes},
     execute_command(<<"metric">>, <<"getClusterMetricsAsDicts">>, Vars, Config).
 
+get_cluster_metrics_as_dicts_with_keys(Keys, Config) ->
+    Vars = #{<<"keys">> => Keys},
+    execute_command(<<"metric">>, <<"getClusterMetricsAsDicts">>, Vars, Config).
+
 %% Helpers
 
 check_spiral_dict(Dict) ->

src/graphql/admin/mongoose_graphql_metric_admin_query.erl

@@ -9,16 +9,35 @@
 
 execute(_Ctx, _Obj, <<"getMetrics">>, Args) ->
     Name = get_name(Args),
-    mongoose_metrics_api:get_metrics(Name);
+    case mongoose_metrics_api:get_metrics(Name) of
+        {no_name_found, Msg} ->
+            mongoose_graphql_helper:make_error(no_name_found, Msg, #{names => Name});
+        {ok, Info} ->
+            {ok, Info}
+    end;
 execute(_Ctx, _Obj, <<"getMetricsAsDicts">>, Args) ->
     Name = get_name(Args),
     Keys = get_keys2(Args),
-    mongoose_metrics_api:get_metrics_as_dicts(Name, Keys);
+    case mongoose_metrics_api:get_metrics_as_dicts(Name, Keys) of
+        {no_name_found, Msg} ->
+            mongoose_graphql_helper:make_error(no_name_found, Msg, #{name => Name});
+        {no_key_found, Msg} ->
+            mongoose_graphql_helper:make_error(no_key_found, Msg, #{keys => Keys});
+        {ok, Info} ->
+            {ok, Info}
+    end;
 execute(_Ctx, _Obj, <<"getClusterMetricsAsDicts">>, Args) ->
     Name = get_name(Args),
     Keys = get_keys2(Args),
     Nodes = get_nodes(Args),
-    mongoose_metrics_api:get_cluster_metrics_as_dicts(Name, Keys, Nodes).
+    case mongoose_metrics_api:get_cluster_metrics_as_dicts(Name, Keys, Nodes) of
+        {no_name_found, Msg} ->
+            mongoose_graphql_helper:make_error(no_name_found, Msg, #{name => Name});
+        {no_key_found, Msg} ->
+            mongoose_graphql_helper:make_error(no_key_found, Msg, #{keys => Keys});
+        {ok, Info} ->
+            {ok, Info}
+    end.
 
 %% get_keys is a BIF, so we have a name conflict
 get_keys2(Args) ->

src/metrics/mongoose_metrics_api.erl

@@ -16,30 +16,59 @@
     {ok, #{binary() => binary() | [metric_dict_result()]}}
     | {error, binary()}.
 
--spec get_metrics(Name :: name()) -> {ok, [metric_result()]}.
+-spec get_metrics(Name :: name()) -> {ok, [metric_result()]} | {no_name_found, binary()}.
 get_metrics(Name) ->
-    Values = exometer:get_values(Name),
-    {ok, lists:map(fun make_metric_result/1, Values)}.
+    case exometer:get_values(Name) of
+        [] ->
+            {no_name_found, <<"No value found for queried name">>};
+        Values ->
+            {ok, lists:map(fun make_metric_result/1, Values)}
+    end.
 
 -spec get_metrics_as_dicts(Name :: name(), Keys :: [key()]) ->
-    {ok, [metric_dict_result()]}.
+    {ok, [metric_dict_result()]} | {no_name_found | no_key_found, binary()}.
 get_metrics_as_dicts(Name, Keys) ->
-    Values = exometer:get_values(Name),
-    {ok, [make_metric_dict_result(V, Keys) || V <- Values]}.
+    case exometer:get_values(Name) of
+        [] ->
+            {no_name_found, <<"No value found for queried names">>};
+        Values ->
+            Results = [make_metric_dict_result({N, Dict}, Keys) ||
+                       {N, Dict} <- Values, filter_keys(Dict, Keys) =/= []],
+            case Results of
+                [] ->
+                    {no_key_found, <<"No value found for queried keys">>};
+                _ ->
+                    {ok, Results}
+            end
+    end.
 
 -spec get_cluster_metrics_as_dicts(Name :: name(), Keys :: [key()],
                                    Nodes :: [node()]) ->
-    {ok, [metric_node_dict_result()]}.
+    {ok, [metric_node_dict_result()]} | {no_name_found | no_key_found, binary()}.
 get_cluster_metrics_as_dicts(Name, Keys, Nodes) ->
     Nodes2 = existing_nodes(Nodes),
     F = fun(Node) -> rpc:call(Node, exometer, get_values, [Name]) end,
     Results = mongoose_lib:pmap(F, Nodes2),
-    {ok, [make_node_result(Node, Result, Keys)
-          || {Node, Result} <- lists:zip(Nodes2, Results)]}.
+    case lists:all(fun(Elem) -> Elem == {ok, []} end, Results) of
+        true ->
+            {no_name_found, <<"No value found for queried names or nodes">>};
+        false ->
+            Results2 = [make_node_result(Node, Result, Keys)
+                        || {Node, Result} <- lists:zip(Nodes2, Results)],
+            AreResultsEmpty = lists:all(fun({ok, Elem}) -> maps:find(<<"result">>, Elem) == {ok, []}
+                                        end, Results2),
+            case AreResultsEmpty of
+                true ->
+                    {no_key_found, <<"No value found for queried keys">>};
+                false ->
+                    {ok, Results2}
+            end
+    end.
 
 make_node_result(Node, {ok, Values}, Keys) ->
     {ok, #{<<"node">> => Node,
-           <<"result">> => [make_metric_dict_result(V, Keys) || V <- Values]}};
+           <<"result">> => [make_metric_dict_result({N, Dict}, Keys) ||
+                            {N, Dict} <- Values, filter_keys(Dict, Keys) =/= []]}};
 make_node_result(Node, Other, _Keys) ->
     ?LOG_ERROR(#{what => metric_get_failed,
                  remote_node => Node, reason => Other}),