From 0a515771f3b6bcf01deaddfdbbee1d936a8d59f2 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Wed, 3 Apr 2024 18:16:35 +0200 Subject: [PATCH] update coding style and add review workflow --- .github/workflows/review.yaml | 44 +++++ rebar.config | 18 ++- rebar.config.script | 6 +- src/prometheus_diameter_collector.erl | 160 +++++++++---------- test/prometheus_diameter_collector_SUITE.erl | 74 ++++----- 5 files changed, 175 insertions(+), 127 deletions(-) create mode 100644 .github/workflows/review.yaml diff --git a/.github/workflows/review.yaml b/.github/workflows/review.yaml new file mode 100644 index 0000000..8f97c61 --- /dev/null +++ b/.github/workflows/review.yaml @@ -0,0 +1,44 @@ +name: Review + +on: + pull_request_target: + types: + - opened + - synchronize + branches: + - master + +jobs: + code-style-review: + runs-on: ubuntu-22.04 + env: + ACCESS_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - + name: work around permission issue + run: | + git config --global --add safe.directory /__w/prometheus_diameter_collector/prometheus_diameter_collector + - + name: Check out repository + uses: actions/checkout@v4 + with: + ref: ${{github.event.pull_request.head.ref}} + repository: ${{github.event.pull_request.head.repo.full_name}} + - + name: install dependencies + run: | + sudo apt update + sudo apt install rebar3 emacs erlang-mode + - + name: format + run: rebar3 fmt + - + name: automated review + uses: googleapis/code-suggester@v4 + with: + command: review + pull_number: ${{ github.event.pull_request.number }} + git_dir: '.' + - + name: check + run: git diff --quiet --exit-code diff --git a/rebar.config b/rebar.config index 509fcfd..2daa531 100644 --- a/rebar.config +++ b/rebar.config @@ -1,3 +1,5 @@ +%%-*-Erlang-*- + %% == Erlang Compiler == {erl_opts, [debug_info]}. @@ -9,16 +11,18 @@ %% == Xref == {xref_checks, [undefined_function_calls, undefined_functions, - locals_not_used, deprecated_function_calls, - deprecated_funcqtions]}. + locals_not_used, deprecated_function_calls, + deprecated_funcqtions]}. %% == Plugins == -{plugins, [ - % @TODO: Folow https://github.com/markusn/coveralls-erl/pull/36 and use `coveralls` after release - {coveralls, {git, "https://github.com/RoadRunnr/coveralls-erl.git", {branch, "feature/git-info"}}}, - rebar3_hex] -}. +{plugins, + [ + %% @TODO: Folow https://github.com/markusn/coveralls-erl/pull/36 and use `coveralls` after release + {coveralls, {git, "https://github.com/RoadRunnr/coveralls-erl.git", {branch, "feature/git-info"}}}, + rebar3_hex, + rebar3_fmt + ]}. %% == Cover == {cover_enabled, true}. diff --git a/rebar.config.script b/rebar.config.script index ababb5d..6cd04b4 100644 --- a/rebar.config.script +++ b/rebar.config.script @@ -2,9 +2,9 @@ case {os:getenv("GITHUB_ACTIONS"), os:getenv("GITHUB_TOKEN")} of {"true", Token} when is_list(Token) -> CONFIG1 = [{coveralls_repo_token, Token}, - {coveralls_service_job_id, os:getenv("GITHUB_RUN_ID")}, - {coveralls_commit_sha, os:getenv("GITHUB_SHA")}, - {coveralls_flag_name, os:getenv("COVERALLS_FLAG_NAME")} | CONFIG], + {coveralls_service_job_id, os:getenv("GITHUB_RUN_ID")}, + {coveralls_commit_sha, os:getenv("GITHUB_SHA")}, + {coveralls_flag_name, os:getenv("COVERALLS_FLAG_NAME")} | CONFIG], case os:getenv("GITHUB_EVENT_NAME") =:= "pull_request" andalso string:tokens(os:getenv("GITHUB_REF"), "/") of [_, "pull", PRNO, _] -> diff --git a/src/prometheus_diameter_collector.erl b/src/prometheus_diameter_collector.erl index d652ef7..ad8f4b4 100644 --- a/src/prometheus_diameter_collector.erl +++ b/src/prometheus_diameter_collector.erl @@ -18,21 +18,21 @@ -include_lib("prometheus/include/prometheus.hrl"). -export([ - deregister_cleanup/1, - collect_mf/2, collect_metrics/2, - gather/0, - gather_statistics/5 - ]). + deregister_cleanup/1, + collect_mf/2, collect_metrics/2, + gather/0, + gather_statistics/5 + ]). -import(prometheus_model_helpers, [create_mf/5, - gauge_metric/2]). + gauge_metric/2]). -define(METRIC_NAME_PREFIX, "diameter_"). -define(METRICS, [{applications, gauge, "Number of installed DIAMETER applications."}, - {connections, gauge, "Number of connections to peers."}, - {messages, gauge, "Number of requests."}, - {errors, gauge, "Number of errors."}]). + {connections, gauge, "Number of connections to peers."}, + {messages, gauge, "Number of requests."}, + {errors, gauge, "Number of errors."}]). %%==================================================================== %% Collector API @@ -47,15 +47,15 @@ collect_mf(_Registry, Callback) -> mf(Callback, {Name, Type, Help}, Stats) -> Callback(create_mf(?METRIC_NAME(Name), Help, Type, ?MODULE, - {Type, fun(S) -> maps:get(Name, S, undefined) end, Stats})), + {Type, fun(S) -> maps:get(Name, S, undefined) end, Stats})), ok. collect_metrics(_, {Type, Fun, Stats}) -> case Fun(Stats) of - M when is_map(M) -> - [metric(Type, Labels, Value) || {Labels, Value} <- maps:to_list(M)]; - _ -> - undefined + M when is_map(M) -> + [metric(Type, Labels, Value) || {Labels, Value} <- maps:to_list(M)]; + _ -> + undefined end. metric(gauge, Labels, Value) -> @@ -76,76 +76,76 @@ gather() -> Services = diameter:services(), lists:foldl( fun(SvcName, Stats) -> - Info = diameter:service_info(SvcName, [applications, peers]), - gather_service(SvcName, Info, Stats) + Info = diameter:service_info(SvcName, [applications, peers]), + gather_service(SvcName, Info, Stats) end, #{}, Services). gather_service(SvcName, Info, Stats) -> lists:foldl( fun({applications, Apps}, S0) -> - add([applications, [{svc, SvcName}]], length(Apps), S0); - ({peers, Peers}, S0) -> - Apps = proplists:get_value(applications, Info, []), - lists:foldl( - fun({PeerName, Peer}, S1) -> - gather_peer(SvcName, PeerName, Peer, Apps, S1) - end, S0, Peers) + add([applications, [{svc, SvcName}]], length(Apps), S0); + ({peers, Peers}, S0) -> + Apps = proplists:get_value(applications, Info, []), + lists:foldl( + fun({PeerName, Peer}, S1) -> + gather_peer(SvcName, PeerName, Peer, Apps, S1) + end, S0, Peers) end, Stats, Info). gather_peer(SvcName, Peer, Info, Apps, Stats) -> lists:foldl( fun({connections, C}, S0) -> - lists:foldl( - fun(X, S1) -> - gather_connection(SvcName, Peer, X, S1) - end, S0, C); - ({statistics, S}, S0) -> - gather_statistics(SvcName, Peer, S, Apps, S0) + lists:foldl( + fun(X, S1) -> + gather_connection(SvcName, Peer, X, S1) + end, S0, C); + ({statistics, S}, S0) -> + gather_statistics(SvcName, Peer, S, Apps, S0) end, Stats, Info). gather_connection(SvcName, Peer, Values, Stats) -> {_, _, State} = proplists:get_value(watchdog, Values, {undefine, undefined, unknown}), Type = case proplists:get_value(type, Values, unknown) of - accept -> responder; - connect -> initiator; - Other -> Other - end, + accept -> responder; + connect -> initiator; + Other -> Other + end, Port = proplists:get_value(port, Values, []), Connection = case proplists:get_value(module, Port, unknown) of - diameter_tcp -> tcp; - diameter_sctp -> sctp; - OtherTP -> OtherTP - end, + diameter_tcp -> tcp; + diameter_sctp -> sctp; + OtherTP -> OtherTP + end, add([connections, [{svc, SvcName}, {peer, Peer}, {type, Type}, - {state, State}, {protocol, Connection}]], 1, Stats). + {state, State}, {protocol, Connection}]], 1, Stats). gather_statistics(SvcName, Peer, S, Apps, Stats) -> lists:foldl( fun({{{_, _, 1} = Msg, Direction}, Cnt}, S1) -> - add([messages, [{svc, SvcName}, {peer, Peer}, - {direction, Direction}, - {type, msg_type(Msg)}, - {msg, msg_name(Msg, Apps)}]], Cnt, S1); - ({{Msg, Direction, {'Result-Code', RC}}, Cnt}, S1) -> - add([messages, [{svc, SvcName}, {peer, Peer}, - {direction, Direction}, - {type, msg_type(Msg)}, - {msg, msg_name(Msg, Apps)}, - {rc, RC}]], Cnt, S1); - ({{_, _, error}, Cnt}, S1) -> + add([messages, [{svc, SvcName}, {peer, Peer}, + {direction, Direction}, + {type, msg_type(Msg)}, + {msg, msg_name(Msg, Apps)}]], Cnt, S1); + ({{Msg, Direction, {'Result-Code', RC}}, Cnt}, S1) -> + add([messages, [{svc, SvcName}, {peer, Peer}, + {direction, Direction}, + {type, msg_type(Msg)}, + {msg, msg_name(Msg, Apps)}, + {rc, RC}]], Cnt, S1); + ({{_, _, error}, Cnt}, S1) -> add([errors,[{svc, SvcName}, {peer, Peer}, {error, unknown}]], Cnt, S1); - ({{Msg, Direction, Result}, Cnt}, S1) when is_atom(Result) -> - add([messages, [{svc, SvcName}, {peer, Peer}, - {direction, Direction}, - {type, msg_type(Msg)}, - {msg, msg_name(Msg, Apps)}, - {rc, Result}]], Cnt, S1); - ({Error, Cnt}, S1) when is_atom(Error) -> - add([errors,[{svc, SvcName}, {peer, Peer}, - {error, Error}]], Cnt, S1); - (_, S1) -> - S1 + ({{Msg, Direction, Result}, Cnt}, S1) when is_atom(Result) -> + add([messages, [{svc, SvcName}, {peer, Peer}, + {direction, Direction}, + {type, msg_type(Msg)}, + {msg, msg_name(Msg, Apps)}, + {rc, Result}]], Cnt, S1); + ({Error, Cnt}, S1) when is_atom(Error) -> + add([errors,[{svc, SvcName}, {peer, Peer}, + {error, Error}]], Cnt, S1); + (_, S1) -> + S1 end, Stats, S). msg_type({_, 0}) -> answer; @@ -155,33 +155,33 @@ msg_type({_, _, 1}) -> request. try_dict(Dict, {_, CmdCode, Rbit} = Cmd) -> case code:is_loaded(Dict) of - {file, _} -> - try Dict:msg_name(CmdCode, Rbit =:= 1) of - '' -> Cmd; - Name when is_atom(Name) -> Name; - _ -> Cmd - catch - _:_ -> - Cmd - end; - _ -> - Cmd + {file, _} -> + try Dict:msg_name(CmdCode, Rbit =:= 1) of + '' -> Cmd; + Name when is_atom(Name) -> Name; + _ -> Cmd + catch + _:_ -> + Cmd + end; + _ -> + Cmd end. msg_name({0, _, _} = Cmd, _Apps) -> case try_dict(diameter_gen_base_rfc6733, Cmd) of - Name when is_atom(Name) -> - Name; - _ -> - try_dict(diameter_gen_base_rfc3588, Cmd) + Name when is_atom(Name) -> + Name; + _ -> + try_dict(diameter_gen_base_rfc3588, Cmd) end; msg_name({ApplId, _, _} = Cmd, Apps) -> case lists:filter( - fun(E) -> ApplId =:= proplists:get_value(id, E, -1) end, Apps) of - [App|_] -> - try_dict(proplists:get_value(dictionary, App, undefined), Cmd); - _ -> - Cmd + fun(E) -> ApplId =:= proplists:get_value(id, E, -1) end, Apps) of + [App|_] -> + try_dict(proplists:get_value(dictionary, App, undefined), Cmd); + _ -> + Cmd end; msg_name(_, _Apps) -> unknown. diff --git a/test/prometheus_diameter_collector_SUITE.erl b/test/prometheus_diameter_collector_SUITE.erl index 13c2ad4..ddd9a2a 100644 --- a/test/prometheus_diameter_collector_SUITE.erl +++ b/test/prometheus_diameter_collector_SUITE.erl @@ -19,33 +19,33 @@ %%% ================================================================== -export([ - all/0, - groups/0, - init_per_suite/1, - end_per_suite/1 -]). + all/0, + groups/0, + init_per_suite/1, + end_per_suite/1 + ]). %%% ================================================================== %%% Common Tests Info Callbacks Exports %%% ================================================================== -export([ - diameter_applications/0, - diameter_connections/0, - diameter_messages/0, - gather_statistics/0 -]). + diameter_applications/0, + diameter_connections/0, + diameter_messages/0, + gather_statistics/0 + ]). %%% ================================================================== %%% Common Tests Exports %%% ================================================================== -export([ - diameter_applications/1, - diameter_connections/1, - diameter_messages/1, - gather_statistics/1 -]). + diameter_applications/1, + diameter_connections/1, + diameter_messages/1, + gather_statistics/1 + ]). %%% ================================================================== %%% Includes @@ -58,34 +58,34 @@ %%% ================================================================== -define(match(Guard, Expr), - ((fun () -> - case (Expr) of - Guard -> - ok; - V -> - ct:pal("MISMATCH(~s:~b, ~s)~nExpected: ~p~nActual: ~p~n", - [?FILE, ?LINE, ??Expr, ??Guard, V]), - error(badmatch) - end - end)())). + ((fun () -> + case (Expr) of + Guard -> + ok; + V -> + ct:pal("MISMATCH(~s:~b, ~s)~nExpected: ~p~nActual: ~p~n", + [?FILE, ?LINE, ??Expr, ??Guard, V]), + error(badmatch) + end + end)())). %%% ================================================================== %%% Common Tests Callbacks %%% ================================================================== all() -> - [ - {group, collector} - ]. + [ + {group, collector} + ]. groups() -> [ - {collector, [sequence], [ - diameter_applications, - diameter_connections, - diameter_messages, - gather_statistics - ]} + {collector, [sequence], [ + diameter_applications, + diameter_connections, + diameter_messages, + gather_statistics + ]} ]. init_per_suite(Config) -> @@ -132,7 +132,7 @@ gather_statistics(_Config) -> {{{unknown,0},recv,discarded},2618}, {{{0,257,0},recv,{'Result-Code',2001}},1}], R = #{messages => #{ - [{svc,testsvc}, {peer,<<"testpeer">>}, {direction,recv}, {type,answer}, {msg,'CEA'}, {rc,2001}] => 1, - [{svc,testsvc}, {peer,<<"testpeer">>}, {direction,recv}, {type,answer}, {msg,unknown}, {rc,discarded}] => 2618, - [{svc,testsvc}, {peer,<<"testpeer">>}, {direction,send}, {type,request}, {msg,'CER'}] => 1}}, + [{svc,testsvc}, {peer,<<"testpeer">>}, {direction,recv}, {type,answer}, {msg,'CEA'}, {rc,2001}] => 1, + [{svc,testsvc}, {peer,<<"testpeer">>}, {direction,recv}, {type,answer}, {msg,unknown}, {rc,discarded}] => 2618, + [{svc,testsvc}, {peer,<<"testpeer">>}, {direction,send}, {type,request}, {msg,'CER'}] => 1}}, ?match(R, prometheus_diameter_collector:gather_statistics(testsvc, <<"testpeer">>, S, [], #{})).