Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrument/backend #4282

Merged
merged 8 commits into from
May 23, 2024
Merged

Instrument/backend #4282

merged 8 commits into from
May 23, 2024

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented May 21, 2024

The goal is to use mongoose_instrument instead of mongoose_metrics in mongoose_backend.

The new event naming is different from the previously used metric naming:

  • For consistency with other module events, events are named Mod_Backend, e.g. mod_roster_rdbms.
  • Labels are host_type and function (which is a new label supported by mongoose_instrument). The events are per host type unless the backend is set up globally. This differs from the previously used global metrics, which were implemented before we introduced dynamic domains. With the limited number of host types and the possibility to use the all_metrics_are_global option, it shouldn't be an issue.
  • Measurements contain count, time and args. The latter is a list of arguments, which can be used for debugging and is used in big tests to match expected events. Unless debug logs are enabled, a heavy term in arguments shouldn't cause any problems, because it is not copied anywhere.

Other changes:

  • Support M, F, A arguments in mongoose_instrument:span.
  • Fix missing tracked_funs, which were discovered because it is illegal to trigger a non-existing event.
  • Update tests. Note that metrics_roster_SUITE checks two selected backends events now, which is as a sample used to verify that mongoose_backend events are working.

Note: if mongoose_backend had a stop function in addition to init, it would be possible to eliminate the try ... catch and prevent other unwanted issues when a backend is initialized many times. This would require a lot of changes, and I will log it as a separate story.

@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch 2 times, most recently from 24f238f to 72ed24d Compare May 21, 2024 10:50
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from 101a419 to 7543844 Compare May 21, 2024 13:33
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

Copy link

codecov bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.69%. Comparing base (a744408) to head (2ce2ec2).
Report is 4 commits behind head on feature/instrument.

Additional details and impacted files
@@                  Coverage Diff                   @@
##           feature/instrument    #4282      +/-   ##
======================================================
+ Coverage               84.28%   84.69%   +0.41%     
======================================================
  Files                     556      556              
  Lines                   33889    33889              
======================================================
+ Hits                    28564    28703     +139     
+ Misses                   5325     5186     -139     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from d545ca3 to 2680ead Compare May 22, 2024 08:30
@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from 2680ead to 41afdf8 Compare May 22, 2024 12:28
@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from 41afdf8 to 7b7fa14 Compare May 22, 2024 13:57
@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from 7b7fa14 to f5fbe45 Compare May 22, 2024 14:15
@mongoose-im
Copy link
Collaborator

mongoose-im commented May 22, 2024

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / f5fbe45
Reports root/ big
OK: 457 / Failed: 0 / User-skipped: 41 / Auto-skipped: 0


small_tests_25 / small_tests / f5fbe45
Reports root / small


small_tests_26 / small_tests / f5fbe45
Reports root / small


small_tests_26_arm64 / small_tests / f5fbe45
Reports root / small


ldap_mnesia_25 / ldap_mnesia / f5fbe45
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / f5fbe45
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


internal_mnesia_26 / internal_mnesia / f5fbe45
Reports root/ big
OK: 2428 / Failed: 0 / User-skipped: 765 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / f5fbe45
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_mysql_redis_26 / mysql_redis / f5fbe45
Reports root/ big
OK: 4589 / Failed: 1 / User-skipped: 138 / Auto-skipped: 0

offline_SUITE:with_groupchat:groupchat_message_is_stored
{error,{{badrpc,timeout},
    [{escalus_rpc,call_with_cookie_match,
            [mongooseim@localhost,ejabberd_admin,register,
             [<<"alicE_groupchat_message_is_stored_2787">>,
            <<"domain.example.com">>,<<"matygrysa">>],
             3000,mongooseim],
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_rpc.erl"},
             {line,34}]},
     {lists,foreach_1,2,[{file,"lists.erl"},{line,1686}]},
     {escalus_ejabberd,create_users,2,
               [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_ejabberd.erl"},
              {line,211}]},
     {escalus_fresh,create_users,2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_fresh.erl"},
             {line,62}]},
     {escalus_fresh,story_with_config,3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_fresh.erl"},
             {line,50}]},
     {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1793}]},
     {test_server,run_test_case_eval1,6,
            [{file,"test_server.erl"},{line,1302}]},
     {test_server,run_test_case_eval,9,
            [{file,"test_server.erl"},{line,1234}]}]}}

Report log


pgsql_cets_26 / pgsql_cets / f5fbe45
Reports root/ big
OK: 4517 / Failed: 0 / User-skipped: 174 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / f5fbe45
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / f5fbe45
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 108 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / f5fbe45
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / f5fbe45
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


mysql_redis_26 / mysql_redis / f5fbe45
Reports root/ big
OK: 4990 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


mssql_mnesia_26 / odbc_mssql_mnesia / f5fbe45
Reports root/ big
OK: 5008 / Failed: 0 / User-skipped: 115 / Auto-skipped: 0

@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from f5fbe45 to f48f62e Compare May 22, 2024 14:49
chrzaszcz added 6 commits May 22, 2024 16:53
Make sure the instrumentation is set up only once.
Unfortunately there is no 'stop', and adding it would be a substantial
task. Moreover, some modules intentionally call it multiple times
for the 'global' scope.
It can be reused e.g. in small tests.
This way tracked funs are always the same.
- Start instrumentation in tests
- Use the new mongoose_rdbms_backend:init/1
@chrzaszcz chrzaszcz force-pushed the instrument/backend branch from f48f62e to 2ce2ec2 Compare May 22, 2024 14:54
@mongoose-im
Copy link
Collaborator

mongoose-im commented May 22, 2024

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / f48f62e
Reports root/ big
OK: 457 / Failed: 0 / User-skipped: 41 / Auto-skipped: 0


small_tests_25 / small_tests / f48f62e
Reports root / small


small_tests_26 / small_tests / f48f62e
Reports root / small


small_tests_26_arm64 / small_tests / f48f62e
Reports root / small


ldap_mnesia_25 / ldap_mnesia / f48f62e
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / f48f62e
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


internal_mnesia_26 / internal_mnesia / f48f62e
Reports root/ big
OK: 2428 / Failed: 0 / User-skipped: 765 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / f48f62e
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / f48f62e
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_mysql_redis_26 / mysql_redis / f48f62e
Reports root/ big
OK: 4589 / Failed: 0 / User-skipped: 138 / Auto-skipped: 0


pgsql_cets_26 / pgsql_cets / f48f62e
Reports root/ big
OK: 4517 / Failed: 0 / User-skipped: 174 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / f48f62e
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 108 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / f48f62e
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / f48f62e
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


mysql_redis_26 / mysql_redis / f48f62e
Reports root/ big
OK: 4990 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


mssql_mnesia_26 / odbc_mssql_mnesia / f48f62e
Reports root/ big
OK: 5008 / Failed: 0 / User-skipped: 115 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented May 22, 2024

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 2ce2ec2
Reports root/ big
OK: 457 / Failed: 0 / User-skipped: 41 / Auto-skipped: 0


small_tests_25 / small_tests / 2ce2ec2
Reports root / small


small_tests_26 / small_tests / 2ce2ec2
Reports root / small


small_tests_26_arm64 / small_tests / 2ce2ec2
Reports root / small


ldap_mnesia_25 / ldap_mnesia / 2ce2ec2
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / 2ce2ec2
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


internal_mnesia_26 / internal_mnesia / 2ce2ec2
Reports root/ big
OK: 2428 / Failed: 0 / User-skipped: 765 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 2ce2ec2
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_mysql_redis_26 / mysql_redis / 2ce2ec2
Reports root/ big
OK: 4589 / Failed: 0 / User-skipped: 138 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 2ce2ec2
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


pgsql_cets_26 / pgsql_cets / 2ce2ec2
Reports root/ big
OK: 4517 / Failed: 0 / User-skipped: 174 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 2ce2ec2
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 108 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 2ce2ec2
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


mysql_redis_26 / mysql_redis / 2ce2ec2
Reports root/ big
OK: 4990 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / 2ce2ec2
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


mssql_mnesia_26 / odbc_mssql_mnesia / 2ce2ec2
Reports root/ big
OK: 5008 / Failed: 0 / User-skipped: 115 / Auto-skipped: 0

@chrzaszcz chrzaszcz marked this pull request as ready for review May 22, 2024 15:31
Copy link
Contributor

@gustawlippa gustawlippa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! Looks good to me 👍

@gustawlippa gustawlippa merged commit 54d4a51 into feature/instrument May 23, 2024
4 checks passed
@gustawlippa gustawlippa deleted the instrument/backend branch May 23, 2024 10:10
@jacekwegr jacekwegr added this to the 6.3.0 milestone Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants