From 3d3cf17766304def51234c95bf4be70f59d5371d Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Wed, 16 Nov 2022 15:22:46 +0100 Subject: [PATCH] Improve error handling in metric API --- big_tests/tests/graphql_metric_SUITE.erl | 38 ++++++++++++++- .../mongoose_graphql_metric_admin_query.erl | 25 ++++++++-- src/metrics/mongoose_metrics_api.erl | 47 +++++++++++++++---- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/big_tests/tests/graphql_metric_SUITE.erl b/big_tests/tests/graphql_metric_SUITE.erl index a144d158ba7..a48c70daab4 100644 --- a/big_tests/tests/graphql_metric_SUITE.erl +++ b/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,20 @@ metrics_tests() -> [get_all_metrics, get_all_metrics_check_by_type, get_by_name_global_erlang_metrics, + get_nonexistent_metrics_by_name, get_process_queue_length, get_inet_stats, get_vm_stats_memory, get_all_metrics_as_dicts, get_by_name_metrics_as_dicts, + get_by_name_nonexistent_metrics_as_dicts, get_metrics_as_dicts_with_key_one, + get_metrics_with_nonexistent_key, 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_nonexisting_nodes]. domain_admin_metrics_tests() -> [domain_admin_get_metrics, @@ -126,6 +132,11 @@ get_by_name_global_erlang_metrics(Config) -> %% Other metrics are filtered out undef = maps:get(ReadsKey, Map, undef). +get_nonexistent_metrics_by_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 +178,11 @@ get_by_name_metrics_as_dicts(Config) -> check_spiral_dict(Dict) end, ParsedResult). +get_by_name_nonexistent_metrics_as_dicts(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 +191,11 @@ get_metrics_as_dicts_with_key_one(Config) -> [#{<<"key">> := <<"one">>, <<"value">> := One}] = maps:get(SentName, Map), true = is_integer(One). +get_metrics_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_cluster_metrics(Config) -> %% We will have at least these two nodes Node1 = atom_to_binary(maps:get(node, distributed_helper:mim())), @@ -205,6 +226,19 @@ get_mim2_cluster_metrics(Config) -> [#{<<"node">> := Node, <<"result">> := ResList}] = ParsedResult, check_node_result_is_valid(ResList, true). +get_cluster_metrics_for_nonexisting_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) -> + + check_node_result_is_valid(ResList, MetricsAreGlobal) -> %% Check that result contains something Map = dict_objects_to_map(ResList), diff --git a/src/graphql/admin/mongoose_graphql_metric_admin_query.erl b/src/graphql/admin/mongoose_graphql_metric_admin_query.erl index c2fdebaf76d..24ce766f535 100644 --- a/src/graphql/admin/mongoose_graphql_metric_admin_query.erl +++ b/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) -> diff --git a/src/metrics/mongoose_metrics_api.erl b/src/metrics/mongoose_metrics_api.erl index 53fa08941a5..bb38cd296e5 100644 --- a/src/metrics/mongoose_metrics_api.erl +++ b/src/metrics/mongoose_metrics_api.erl @@ -16,30 +16,57 @@ {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 -> + Result1 = [make_metric_dict_result({N, Dict}, Keys) || + {N, Dict} <- Values, filter_keys(Dict, Keys) =/= []], + case Result1 of + [] -> + {no_key_found, <<"No value found for queried keys">>}; + _ -> + {ok, Result1} + 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)], + case lists:all(fun({ok, Elem}) -> maps:find(<<"result">>, Elem) == {ok, []} end, Results2) 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}),