Fixing cr issues

esl · May 4, 2023 · cf2ddec · cf2ddec
1 parent 2cf0160
commit cf2ddec
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 43 deletions.
diff --git a/big_tests/tests/service_mongoose_system_metrics_SUITE.erl b/big_tests/tests/service_mongoose_system_metrics_SUITE.erl
@@ -12,11 +12,11 @@
 -define(TRACKING_ID_EXTRA, #{id => "UA-EXTRA-TRACKING-ID", secret => "Secret3"}).
 
 -record(event, {
-    name = <<"">>,
+    name = <<>>,
     params = #{},
-    client_id = <<"">>,
-    instance_id = <<"">>,
-    app_secret = <<"">>}).
+    client_id = <<>>,
+    instance_id = <<>>,
+    app_secret = <<>>}).
 
 -import(distributed_helper, [mim/0, mim2/0, mim3/0, rpc/4,
                              require_rpc_nodes/1
@@ -387,7 +387,7 @@ all_event_have_the_same_client_id() ->
     1 = length(UniqueSortedTab).
 
 is_host_count_reported() ->
-    is_in_table(<<"hosts_count">>).
+    is_in_table(<<"host_count">>).
 
 are_modules_reported() ->
     is_in_table(<<"module">>).
@@ -399,7 +399,7 @@ is_in_table(EventName) ->
             verify_name(Name, EventName, Params)
         end, Tab).
 
-verify_name(<<"module_with_opts">>, <<"module">>, Params) ->
+verify_name(<<"module_with_opt">>, <<"module">>, Params) ->
     Module = maps:get(<<"module">>, Params),
     Result = re:run(Module, "^mod_.*"),
     case Result of
@@ -484,7 +484,7 @@ is_cluster_uptime_reported() ->
     is_feature_reported(<<"cluster">>, <<"uptime">>).
 
 are_xmpp_components_reported() ->
-    is_feature_reported(<<"cluster">>, <<"components">>).
+    is_feature_reported(<<"cluster">>, <<"component">>).
 
 is_config_type_reported() ->
     IsToml = is_feature_reported(<<"cluster">>, <<"config_type">>, <<"toml">>),
@@ -498,7 +498,7 @@ are_transport_mechanisms_reported() ->
     is_in_table(<<"transport_mechanism">>).
 
 are_outgoing_pools_reported() ->
-    is_in_table(<<"outgoing_pools">>).
+    is_in_table(<<"outgoing_pool">>).
 
 is_iq_count_reported() ->
     is_feature_reported(<<"xmpp_stanza_count">>,
@@ -521,8 +521,8 @@ assert_message_count_is_incremented(Sent, Received) ->
 assert_increment(EventCategory, InitialValue) ->
     Events = match_events(<<"xmpp_stanza_count">>, <<"stanza_type">>, EventCategory),
     % expect exactly one event with an increment of 1
-    SeekedEvent = [Event || Event = #event{params = 
-        #{<<"total">> := Total, <<"increment">> := 1}} <- Events, Total == InitialValue + 1],
+    SeekedEvent = [Event || Event = #event{params = #{<<"total">> := Total, <<"increment">> := 1}}
+        <- Events, Total == InitialValue + 1],
     ?assertMatch([_], SeekedEvent).
 
 get_metric_value(EventCategory) ->
@@ -532,7 +532,7 @@ get_metric_value(EventCategory) ->
 more_than_one_component_is_reported() ->
     Events = match_events(<<"cluster">>),
     lists:any(fun(#event{params = Params}) ->
-                       maps:get(<<"components">>, Params) > 0
+                       maps:get(<<"component">>, Params) > 0
               end, Events).
 
 match_events(EventName) ->
@@ -547,7 +547,7 @@ match_events(EventName, ParamKey, ParamValue) ->
     [Event || Event = #event{params = #{ParamKey := Value}} <- Res, Value == ParamValue].
 
 get_matched_events_for_module(ParamModule, Key, ParamValue) ->
-    Res = ets:match_object(?ETS_TABLE, #event{name = <<"module_with_opts">>, _ = '_'}),
+    Res = ets:match_object(?ETS_TABLE, #event{name = <<"module_with_opt">>, _ = '_'}),
     [Event || Event = #event{params = #{<<"module">> := Module, Key := Value}} <- Res,
          Value == ParamValue, Module == ParamModule].
 
@@ -570,5 +570,5 @@ handler_init(Req0) ->
             %% TODO there is a race condition when table is not available
             ets:insert(?ETS_TABLE, EventRecord)
         end, EventTab),
-    Req1 = cowboy_req:reply(200, #{}, <<"">>, Req),
+    Req1 = cowboy_req:reply(200, #{}, <<>>, Req),
     {ok, Req1, no_state}.
diff --git a/src/system_metrics/mongoose_system_metrics_collector.erl b/src/system_metrics/mongoose_system_metrics_collector.erl
@@ -4,9 +4,8 @@
 
 -type report_struct() ::
     #{
-        report_name := term(),
-        key := term(),
-        value := term()
+        name := term(),
+        params := #{term() := term()}
     }.
 
 -export_type([report_struct/0]).
@@ -52,11 +51,11 @@ get_cluster_data() ->
 get_hosts_count() ->
     HostTypes = ?ALL_HOST_TYPES,
     NumberOfHosts = length(HostTypes),
-    [#{name => hosts_count, params => #{value => NumberOfHosts}}].
+    [#{name => host_count, params => #{value => NumberOfHosts}}].
 
 get_domains_count() ->
     DomainsCount = mongoose_domain_core:domains_count(),
-    [#{name => domains_count, params => #{value => DomainsCount}}].
+    [#{name => domain_count, params => #{value => DomainsCount}}].
 
 get_modules() ->
     HostTypes = ?ALL_HOST_TYPES,
@@ -88,7 +87,7 @@ get_modules_metrics(Host, Modules) ->
         end, Modules).
 
 report_module_with_opts(Module, Opts) ->
-    #{name => module_with_opts, params =>
+    #{name => module_with_opt, params =>
         lists:foldl(
             fun
                 ({none, _}, Acc) -> 
@@ -108,7 +107,7 @@ get_number_of_custom_modules() ->
                                                      mongoose_module_metrics),
     MetricsModuleSet = sets:from_list(MetricsModule),
     CountCustomMods= sets:size(sets:subtract(GenModsSet, MetricsModuleSet)),
-    [#{name => custom_modules_count, params => #{value => CountCustomMods}}].
+    [#{name => custom_module_count, params => #{value => CountCustomMods}}].
 
 get_uptime() ->
     {Uptime, _} = statistics(wall_clock),
@@ -119,7 +118,7 @@ get_uptime() ->
 
 get_cluster_size() ->
     NodesNo = length(nodes()) + 1,
-    #{number_of_nodes => NodesNo}.
+    #{node_number => NodesNo}.
 
 get_version() ->
     case lists:keyfind(mongooseim, 1, application:which_applications()) of
@@ -133,14 +132,14 @@ get_components() ->
     Domains = mongoose_router:get_all_domains() ++ ejabberd_router:dirty_get_all_components(all),
     Components = [ejabberd_router:lookup_component(D, node()) || D <- Domains],
     LenComponents = length(lists:flatten(Components)),
-    #{components => LenComponents}.
+    #{component => LenComponents}.
 
 get_api() ->
     ApiList = filter_unknown_api(get_http_handler_modules()),
-    [#{name => http_api, params => 
-        lists:foldl(fun(Element, Acc) ->
-            maps:put(Element, enabled, Acc)
-        end, #{}, ApiList)}].
+    [#{name => http_api,
+       params => lists:foldl(fun(Element, Acc) ->
+                                 maps:put(Element, enabled, Acc)
+                             end, #{}, ApiList)}].
 
 filter_unknown_api(ApiList) ->
     AllowedToReport = [mongoose_client_api, mongoose_admin_api, mod_bosh, mod_websockets],
@@ -169,7 +168,7 @@ get_http_handler_modules(#{handlers := Handlers}) ->
 get_tls_options() ->
     TLSOptions = lists:flatmap(fun extract_tls_options/1, get_listeners(mongoose_c2s_listener)),
     lists:foldl(fun({Mode, Module}, Acc) ->
-                    [#{name => tls_options, params => #{mode => Mode, module => Module}}] ++ Acc
+                    [#{name => tls_option, params => #{mode => Mode, module => Module}}] ++ Acc
                 end, [], lists:usort(TLSOptions)).
 
 extract_tls_options(#{tls := #{mode := TLSMode, module := TLSModule}}) ->
@@ -178,7 +177,7 @@ extract_tls_options(_) -> [].
 
 get_outgoing_pools() ->
     OutgoingPools = mongoose_config:get_opt(outgoing_pools),
-    [#{name => outgoing_pools,
+    [#{name => outgoing_pool,
        params => #{value => Type}} || #{type := Type} <- OutgoingPools].
 
 get_xmpp_stanzas_count(PrevReport) ->

diff --git a/src/system_metrics/mongoose_system_metrics_sender.erl b/src/system_metrics/mongoose_system_metrics_sender.erl
@@ -12,29 +12,22 @@
            [report_struct()],
            [service_mongoose_system_metrics:tracking_id()]) -> ok.
 send(ClientId, Reports, TrackingIds) ->
-    send_reports_for_each_tracking_id(ClientId, TrackingIds, Reports),
-    ok.
-
-%-spec build_reports_for_each_tracking_id(service_mongoose_system_metrics:client_id(),
-%                                         [service_mongoose_system_metrics:tracking_id()],
-%                                         [report_struct()]) -> [google_analytics_report()].
-% 
-send_reports_for_each_tracking_id(ClientId, TrackingIds, Reports) ->
     Url = get_url(),
     lists:map(
         fun(TrackingId) ->
             flush_reports(Url, Reports, ClientId, TrackingId)
-        end, TrackingIds).
+        end, TrackingIds),
+    ok.
 
 get_url() ->
     mongoose_config:get_opt(google_analytics_url, ?BASE_URL).
 
-% % https://developers.google.com/analytics/devguides/collection/protocol/v1/devguide#batch-limitations
-% % A maximum of 20 hits can be specified per request.
+% https://developers.google.com/analytics/devguides/collection/protocol/ga4/sending-events?client_type=gtag#limitations
+% % A maximum of 25 hits can be specified per request.
 flush_reports(_, [], _, _) ->
     {ok, nothing_to_do};
 flush_reports(ReportUrl, Reports, ClientId,
-              #{id := TrackingId, secret := TrackingSecret}) when length(Reports) =< 20 ->
+              #{id := TrackingId, secret := TrackingSecret}) when length(Reports) =< 25 ->
     Headers = [],
     ContentType = "application/json",
     Body = jiffy:encode(#{client_id => list_to_binary(ClientId), events => Reports}),
@@ -43,6 +36,6 @@ flush_reports(ReportUrl, Reports, ClientId,
     Request = {ReportUrl2, Headers, ContentType, Body},
     httpc:request(post, Request, [{ssl, [{verify, verify_none}]}], []);
 flush_reports(ReportUrl, Reports, ClientId, TrackingId) ->
-    {NewBatch, RemainingLines} = lists:split(20, Reports),
+    {NewBatch, RemainingLines} = lists:split(25, Reports),
     flush_reports(ReportUrl, NewBatch, ClientId, TrackingId),
     flush_reports(ReportUrl, RemainingLines, ClientId, TrackingId).
diff --git a/src/system_metrics/service_mongoose_system_metrics.erl b/src/system_metrics/service_mongoose_system_metrics.erl
@@ -7,6 +7,8 @@
 -include("mongoose_config_spec.hrl").
 
 -ifdef(PROD_NODE).
+% The value "Secret" here is an app id from Google Analytics.
+% We refer to it as "Secret" because it is named there that way.
 -define(TRACKING_ID, #{id => "G-7KQE4W9SVJ",
                        secret => "8P4wQIkwSV6zay22uKsnLg"}).
 -else.

diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl
@@ -2946,8 +2946,8 @@ service_mongoose_system_metrics(_Config) ->
     ?err(T(#{<<"periodic_report">> => <<"forever">>})),
     ?err(T(#{<<"initial_report">> => -1})),
     ?err(T(#{<<"periodic_report">> => -1})),
-    ?err(T(#{<<"tracking_id">> => #{<<"id">> => 666}})),
-    ?err(T(#{<<"tracking_id">> => #{<<"secret">> => 666}})),
+    ?err(T(#{<<"tracking_id">> => #{<<"id">> => "G-12345678"}})),
+    ?err(T(#{<<"tracking_id">> => #{<<"secret">> => "Secret"}})),
     ?err(T(#{<<"tracking_id">> => #{<<"secret">> => 666, <<"id">> => 666}})),
     ?err(T(#{<<"report">> => <<"maybe">>})).