Use mongoose_instrument for roster hook metrics #4295
Conversation
chrzaszcz
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/instrument #4295 +/- ##
======================================================
- Coverage 84.76% 84.73% -0.04%
======================================================
Files 557 557
Lines 33897 33901 +4
======================================================
- Hits 28734 28726 -8
- Misses 5163 5175 +12 ☔ View full report in Codecov by Sentry. |
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/hook-handlers
branch
from
24fd770 to
b8731ed
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
chrzaszcz
force-pushed
the
instrument/hook-handlers
branch
from
7e01820 to
f4a6556
Compare
|
elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / f4a6556 small_tests_25 / small_tests / f4a6556 small_tests_26 / small_tests / f4a6556 small_tests_26_arm64 / small_tests / f4a6556 ldap_mnesia_25 / ldap_mnesia / f4a6556 ldap_mnesia_26 / ldap_mnesia / f4a6556 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / f4a6556 dynamic_domains_mysql_redis_26 / mysql_redis / f4a6556 internal_mnesia_26 / internal_mnesia / f4a6556 dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / f4a6556 dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / f4a6556 pgsql_cets_26 / pgsql_cets / f4a6556 mysql_redis_26 / mysql_redis / f4a6556 pgsql_mnesia_25 / pgsql_mnesia / f4a6556 mam_SUITE:rdbms_muc_all:muc_prefs_cases:muc_prefs_set_request_not_an_owner{skip,
{failed,
{mam_SUITE,init_per_testcase,
{{badrpc,
{'EXIT',
{#{host_type => <<"localhost">>,
module => mod_mam_rdbms_prefs,
text => <<"Module missing from mongoose_config">>,
what => module_not_loaded},
[{gen_mod,assert_loaded,2,
[{file,"/home/circleci/project/src/gen_mod.erl"},
{line,346}]},
{gen_mod,start_module,3,
[{file,"/home/circleci/project/src/gen_mod.erl"},
{line,96}]},
{mongoose_modules,start_module,4,
[{file,
"/home/circleci/project/src/mongoose_modules.erl"},
{line,90}]},
{mongoose_modules,ensure_started,3,
[{file,
"/home/circleci/project/src/mongoose_modules.erl"},
{line,80}]},
{mongoose_modules,'-replace_modules/3-lc$^1/1-1-',2,
[{file,
"/home/circleci/project/src/mongoose_modules.erl"},
{line,50}]},
{mongoose_modules,replace_modules,3,
[{file,
"/home/circleci/project/src/mongoose_modules.erl"},
{line,51}]}]}}},
[{distributed_helper,rpc,
[#{node => mongooseim@localhost},
mongoose_modules,replace_modules,
[<<"localhost">>,[],
#{mod_mam =>
#{archive_chat_markers => false,
async_writer =>
#{ba...pgsql_mnesia_26 / pgsql_mnesia / f4a6556 mssql_mnesia_26 / odbc_mssql_mnesia / f4a6556 |
Instead of adding hook handlers, instrument the hook caller. - mod_disco and ejabberd_sm don't depend on mod_roster, so they have separate instrumentation. - the call from mod_roster_api could be made from legacy REST API without mod_roster running, but it's the same situation as for other API calls - which fail in such situation
Legacy metrics are still needed, because sm is still using them.
By replacing metrics with instrumentation in assertions, tests can be parallelized.
This event is trigered separately in mod_roster_api, hence and additional test.
The main reason is that all metrics will become multi-segment soon, and this would mean a significant decrease of test coverage. Alternatively, we could just drop the support for the legacy metrics API, but this would be a different task, and we haven't decided yet to do so. Details: - Metric types are explicitly limited to the supported ones. This was implicit before, because metrics with one-segment names had simple types. - Simplify prepare_name, because it took an extra argument only for logging, which made it more difficult to use with little benefit.
This is possible now, because multi-part names are supported. An alternative would be to just delete metrics progressively, but that would leave us with uncovered functionality.
So far, only the legacy metrics had this option enabled, leading to inconsistency between old and new metrics for mim2. When we convert all metrics to mongoose_instrument, we can remove the legacy option.
Also: remove test repeat (we are doing this for all suites)
chrzaszcz
force-pushed
the
instrument/hook-handlers
branch
from
f4a6556 to
4c8e4b5
Compare
|
elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 4c8e4b5 small_tests_25 / small_tests / 4c8e4b5 small_tests_26 / small_tests / 4c8e4b5 small_tests_26_arm64 / small_tests / 4c8e4b5 ldap_mnesia_25 / ldap_mnesia / 4c8e4b5 dynamic_domains_mysql_redis_26 / mysql_redis / 4c8e4b5 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 4c8e4b5 ldap_mnesia_26 / ldap_mnesia / 4c8e4b5 dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 4c8e4b5 internal_mnesia_26 / internal_mnesia / 4c8e4b5 dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 4c8e4b5 pgsql_cets_26 / pgsql_cets / 4c8e4b5 pgsql_mnesia_25 / pgsql_mnesia / 4c8e4b5 mysql_redis_26 / mysql_redis / 4c8e4b5 pgsql_mnesia_26 / pgsql_mnesia / 4c8e4b5 mssql_mnesia_26 / odbc_mssql_mnesia / 4c8e4b5 |
chrzaszcz
changed the title
The main goal is to replace the roster-related hook handlers in
mongoose_metrics_hookswith instrumentation in the code executing the hooks.Instrumentation is placed in
mod_roster,mod_roster_apiandmod_discoto cover the same cases as before. Big tests are added for all the new instrumentation events.The
modRosterGetsmetric was renamed tomod_roster_get.count. This metric was checked inmetrics_api_SUITE, and the issue was, that the REST API for metrics didn't support metrics with multi-part names. As we are going to rename all metrics, we would have to remove the tests, decreasing coverage. As we aren't planning to remove the REST API yet, I decided to add support for multi-part metric names in the REST API. The API was also working only for selected metric types (gauge, counter, spiral), which was the case for the metrics with single-part names, so the tests were succeeding. Now, as we are returning all metrics, I needed to add explicit filtering of metrics by type to make the tests pass.Notes:
disco_and_caps_SUITEmetrics_roster_SUITEallowed parallelizing tests.mod_roster_apifails ifmod_rosteris not running, but it has to be running for other operations, and it only affects the legacy REST API, so I didn't add extra error handling for this particular operation.all_metrics_are_globalconsistently - onmim2, it should be enabled for instrumentation as well as for metrics.