Instrument/hook handlers - session #4305

chrzaszcz · 2024-06-17T09:41:31Z

The goal is to use mongoose_instrument rather than hook handlers and mongoose_metrics for session-related events.

Set up and execute c2s_auth_failed in mongoose_c2s.
Create an sm_session event with logins, logouts and count metrics. I decided to keep it as one event to be able to increment and decrement the session count.
- Session count is not a probe, because as a probe it would need to be calculated per host type, and this could be a substantial work. Making it a probe could be a separate task.
Change subscription events to include the host type, because it is known and other events in the sm module have the host type.
Add support for a counter metric. In Prometheus, it is a gauge.
Remove metric definitions, including the ones already taken care of in the previous PR's.
Disable the sessions_cleanup test in last_SUITE, because it was creating sessions with processes from another node, causing confusion and inconsistent metrics. As agreed with @arcusfelis, this test could be reworked and re-enabled later.
Update metrics descriptions in docs (should have been already done for some of the metrics).
Notes:
auth_failed hook has no handlers now, but I thought it might still be useful.

codecov · 2024-06-17T09:50:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.67%. Comparing base (6ee91a4) to head (ccb47ee).

Additional details and impacted files

@@                  Coverage Diff                   @@
##           feature/instrument    #4305      +/-   ##
======================================================
- Coverage               84.73%   84.67%   -0.07%     
======================================================
  Files                     557      557              
  Lines                   33888    33889       +1     
======================================================
- Hits                    28714    28694      -20     
- Misses                   5174     5195      +21

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

Keep the session count as a counter, and update it accordingly. It was not a probe for performance reasons, and it stays that way. sm_presence_subscription becomes per-host-type for consistency.

It needs to be a gauge, because a counter is more like a spiral, and it can only be increased.

Some of them should be already removed.

Also: ensure that a spiral can't be decreased.

This test is a bit dirty. It should be rewritten, because it makes the system inconsistent. Details in the comments.

The implementation has changed to include the host type.

- Use instrumentation events - Parallelize tests

As we add more events, the tests started failing, because they were expecting all possible events including c2s_auth_failed.

mongoose-im · 2024-06-19T13:32:52Z

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

small_tests_25 / small_tests / ccb47ee
Reports root / small

small_tests_26 / small_tests / ccb47ee
Reports root / small

small_tests_26_arm64 / small_tests / ccb47ee
Reports root / small

ldap_mnesia_25 / ldap_mnesia / ccb47ee
Reports root/ big
OK: 2289 / Failed: 0 / User-skipped: 906 / Auto-skipped: 0

dynamic_domains_mysql_redis_26 / mysql_redis / ccb47ee
Reports root/ big
OK: 4587 / Failed: 0 / User-skipped: 139 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / ccb47ee
Reports root/ big
OK: 4620 / Failed: 0 / User-skipped: 106 / Auto-skipped: 0

ldap_mnesia_26 / ldap_mnesia / ccb47ee
Reports root/ big
OK: 2289 / Failed: 0 / User-skipped: 906 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / ccb47ee
Reports root/ big
OK: 4620 / Failed: 0 / User-skipped: 106 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / ccb47ee
Reports root/ big
OK: 4616 / Failed: 1 / User-skipped: 109 / Auto-skipped: 0

tr_util_SUITE:c2s_elements

{error,{{badmatch,[#{name => <<"message">>,type => <<"chat">>,
           hooks => [user_send_packet],
           jid =>
             <<"alice_c2s_elements_3184@domain.example.com/res1">>,
           to_jid =>
             <<"bob_c2s_elements_3184@domain.example.com/res1">>,
           from_jid =>
             <<"alice_c2s_elements_3184@domain.example.com/res1">>,
           ref => #Ref<10231.2178159122.1897398273.69577>,
           contents => <<"<body>Hello</body>">>},
           #{name => <<"message">>,type => <<"chat">>,
           hooks => [user_receive_packet,user_receive_message],
           jid =>
             <<"bob_c2s_elements_3184@domain.example.com/res1">>,
           to_jid =>
             <<"bob_c2s_elements_3184@domain.example.com/res1">>,
           from_jid =>
             <<"alice_c2s_elements_3184@domain.example.com/res1">>,
           ref => #Ref<10231.2178159122.1897398273.69584>,
           contents => <<"<body>Hello</body>">>},
           #{name => <<"message">>,type => <<"chat">>,
           hooks => [user_send_message],
           jid =>
             <<"alice_c2s_elements_3184@domain.example.com/res1">>,
           to_jid =>
             <<"bob_c2s_elements_3184@domain.example.com/res1">>,
           from_jid =>
             <<"alice_c2s_elements_3184@domain.example.com/res1">>,
           ref => #Ref<10231.2178159122.1897398273.69577>,
           contents => <<"<body>Hello</body>">>},
           #{name => <<"message">>,type => <<"chat">>,
          ...

Report log

pgsql_cets_26 / pgsql_cets / ccb47ee
Reports root/ big
OK: 4519 / Failed: 0 / User-skipped: 174 / Auto-skipped: 0

internal_mnesia_26 / internal_mnesia / ccb47ee
Reports root/ big
OK: 2429 / Failed: 0 / User-skipped: 766 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / ccb47ee
Reports root/ big
OK: 5012 / Failed: 0 / User-skipped: 113 / Auto-skipped: 0

mysql_redis_26 / mysql_redis / ccb47ee
Reports root/ big
OK: 4991 / Failed: 0 / User-skipped: 134 / Auto-skipped: 0

pgsql_mnesia_26 / pgsql_mnesia / ccb47ee
Reports root/ big
OK: 5012 / Failed: 0 / User-skipped: 113 / Auto-skipped: 0

mssql_mnesia_26 / odbc_mssql_mnesia / ccb47ee
Reports root/ big
OK: 5009 / Failed: 0 / User-skipped: 116 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / ccb47ee
Reports root/ big
OK: 4617 / Failed: 0 / User-skipped: 109 / Auto-skipped: 0

arcusfelis

ok

This comment was marked as outdated.

Sign in to view

Base automatically changed from instrument/hook-handlers-auth to feature/instrument June 17, 2024 10:13

chrzaszcz force-pushed the instrument/hook-handlers-session branch from 10bbb5f to 326c6a4 Compare June 17, 2024 14:23

This comment was marked as outdated.

Sign in to view

chrzaszcz force-pushed the instrument/hook-handlers-session branch from 326c6a4 to 1b5489e Compare June 18, 2024 14:35

This comment was marked as outdated.

Sign in to view

chrzaszcz force-pushed the instrument/hook-handlers-session branch 2 times, most recently from f4bb23a to 9abfcc2 Compare June 19, 2024 07:48

This comment was marked as outdated.

Sign in to view

chrzaszcz force-pushed the instrument/hook-handlers-session branch from 9abfcc2 to 4d17179 Compare June 19, 2024 07:59

This comment was marked as outdated.

Sign in to view

chrzaszcz marked this pull request as ready for review June 19, 2024 08:30

chrzaszcz force-pushed the instrument/hook-handlers-session branch from 4d17179 to af516eb Compare June 19, 2024 11:47

chrzaszcz added 14 commits June 19, 2024 13:51

Set up and execute the auth_failed event in mongoose_c2s

e743336

Add session events to the sm module

2f41c02

Keep the session count as a counter, and update it accordingly. It was not a probe for performance reasons, and it stays that way. sm_presence_subscription becomes per-host-type for consistency.

Add the 'counter' metric, which is increased or decreased

2a95306

Support counter in exometer metrics

7cb0506

Support counter in prometheus metrics

0c10c53

It needs to be a gauge, because a counter is more like a spiral, and it can only be increased.

Remove unused metric definitions

4b5282a

Some of them should be already removed.

Remove unused metrics

423dfb0

Add missing host, which is detected by instrumentation

1d0c157

Add unit tests for the new counter metric

fe2dcc2

Also: ensure that a spiral can't be decreased.

Remove nonexistent test suite

f13c5cb

Export 'filter' utility from instrument_helper

a231f60

Disable test in last_SUITE that leads to inconsistent metrics

65b26ae

This test is a bit dirty. It should be rewritten, because it makes the system inconsistent. Details in the comments.

Fix indentation

a1bd7d0

Update metric names in the API test

2e63f2c

chrzaszcz added 3 commits June 19, 2024 13:51

Add host type to the tested subscription event

98cc1e1

The implementation has changed to include the host type.

Update session metrics tests

f145184

- Use instrumentation events - Parallelize tests

Update metrics description in docs

7baafa9

chrzaszcz force-pushed the instrument/hook-handlers-session branch from af516eb to 7baafa9 Compare June 19, 2024 11:51

This comment was marked as outdated.

Sign in to view

Explicitly declare tested c2s events

ccb47ee

As we add more events, the tests started failing, because they were expecting all possible events including c2s_auth_failed.

chrzaszcz changed the title ~~Instrument/hook handlers session~~ Instrument/hook handlers - session Jun 19, 2024

arcusfelis approved these changes Jun 19, 2024

View reviewed changes

arcusfelis merged commit 14280cf into feature/instrument Jun 19, 2024
4 checks passed

arcusfelis deleted the instrument/hook-handlers-session branch June 19, 2024 14:30

jacekwegr added this to the 6.3.0 milestone Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Instrument/hook handlers - session #4305

Instrument/hook handlers - session #4305

chrzaszcz commented Jun 17, 2024 •

edited

Loading

This comment was marked as outdated.

codecov bot commented Jun 17, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Jun 19, 2024 •

edited

Loading

arcusfelis left a comment

Instrument/hook handlers - session #4305

Instrument/hook handlers - session #4305

Conversation

chrzaszcz commented Jun 17, 2024 • edited Loading

This comment was marked as outdated.

codecov bot commented Jun 17, 2024 • edited Loading

Codecov Report

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Jun 19, 2024 • edited Loading

arcusfelis left a comment

Choose a reason for hiding this comment

chrzaszcz commented Jun 17, 2024 •

edited

Loading

codecov bot commented Jun 17, 2024 •

edited

Loading

mongoose-im commented Jun 19, 2024 •

edited

Loading