-
Notifications
You must be signed in to change notification settings - Fork 437
Use a hook with handlers in mod_event_pusher #4578
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4578 +/- ##
==========================================
+ Coverage 85.97% 86.00% +0.02%
==========================================
Files 565 565
Lines 33880 33887 +7
==========================================
+ Hits 29130 29144 +14
+ Misses 4750 4743 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2b22618 to
23b8128
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This hook will replace the iteration over all push backends
- push_event_acc() is the hook acc containing mongoose_acc and a map of extensible metadata. - push_event_params() are the hook params containing only the event. Hook handlers can modify the acc and metadata, but not the event. Metadata modified by a hook can be utilized by the subsequent hooks.
23b8128 to
88a6f1c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
88a6f1c to
812f09f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Currently, rabbit is the only one using metadata. All handlers have the same priority, because there is no specific ordering that would make sense here.
It demonstrates how the hook-and-handlers mechanism can be used to: - Filter events. - Insert additional metadata.
The example module mod_event_pusher_filter can be used to veridy if both functions work as expected.
812f09f to
f58ea9e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
- Delete functions having calls now - Add delete_handler/5 because mod_event_pusher_push is no longer using this. Sadly ejabberd_sm uses add_handler but not delete_handler...
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Also: - Restructure the docs - Start with a simple introduction - Follow with configuration - Only then provide developer's info - Update migration guide
f836db3 to
a12ce09
Compare
|
elasticsearch_and_cassandra_28 / elasticsearch_and_cassandra_mnesia / a12ce09 small_tests_28 / small_tests / a12ce09 small_tests_27 / small_tests / a12ce09 small_tests_28_arm64 / small_tests / a12ce09 ldap_mnesia_28 / ldap_mnesia / a12ce09 ldap_mnesia_27 / ldap_mnesia / a12ce09 dynamic_domains_mysql_redis_28 / mysql_redis / a12ce09 dynamic_domains_pgsql_mnesia_27 / pgsql_mnesia / a12ce09 internal_mnesia_28 / internal_mnesia / a12ce09 dynamic_domains_pgsql_mnesia_28 / pgsql_mnesia / a12ce09 dynamic_domains_mssql_mnesia_28 / odbc_mssql_mnesia / a12ce09 pgsql_cets_28 / pgsql_cets / a12ce09 cockroachdb_cets_28 / cockroachdb_cets / a12ce09 pubsub_SUITE:tree+last_item_cache:send_last_published_item_no_items_test{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"alice_send_last_published_item_no_items_test_3712@localhost/res1">>,
escalus_tcp,<0.116778.0>,
[{event_manager,<0.116767.0>},
{server,<<"localhost">>},
{username,
<<"alicE_send_last_published_item_no_items_test_3712">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.116767.0>},
{server,<<"localhost">>},
{username,
<<"alicE_send_last_published_item_no_items_test_3712">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"alice_send_last_published_item_no_items_test_3712">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,fun escalus_auth:auth_plain/2},
{wspath,undefined},
{username,
<<"alicE_send_last_published_item_no_items_test_3712">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"55ea4aef3cf3e2b3">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{pubsub_tools,receive_response,3,
[{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
{line,444}]},
{pubsub_tools,receive_and_c...mysql_redis_28 / mysql_redis / a12ce09 acc_e2e_SUITE:cache_and_strip:filter_local_packet_uses_recipient_values{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"bob_filter_local_packet_uses_recipient_values_20@localhost/res1">>,
escalus_tcp,<0.827.0>,
[{event_manager,<0.825.0>},
{server,<<"localhost">>},
{username,
<<"bOb_filter_local_packet_uses_recipient_values_20">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.825.0>},
{server,<<"localhost">>},
{username,
<<"bOb_filter_local_packet_uses_recipient_values_20">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"bob_filter_local_packet_uses_recipient_values_20">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,fun escalus_auth:auth_plain/2},
{wspath,undefined},
{username,
<<"bOb_filter_local_packet_uses_recipient_values_20">>},
{server,<<"localhost">>},
{password,<<"makrolika">>},
{stream_id,<<"d65d8938b18bc0a2">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{acc_e2e_SUITE,
'-filter_local_packet_uses_recipient_values/1-fun-0-',2,
[{file,
"/home/circleci/project/big_tests/tests/acc_e2e_SUITE.erl"},
{l...pgsql_mnesia_28 / pgsql_mnesia / a12ce09 pgsql_mnesia_27 / pgsql_mnesia / a12ce09 mssql_mnesia_28 / odbc_mssql_mnesia / a12ce09 |
telezynski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, useful mechanism to extend event processing!
The main goal is to allow injecting extra data into the generated push notifications for RabbitMQ as requested by @niecore.
I replaced the iteration over backend modules with a new hook
push_event. By using a hook with handlers, one can implement a module that would insert somemetadatain its handler topush_event. Such metadata would be later merged with the notification in the handler ofmod_event_pusher_rabbit. If needed, the key-value pairs frommetadatacould event override existing event properties, but this is not recommended.A module could also act as a filter, dropping selected events. An example of a module combining metadata with filtering is implemented as
mod_event_pusher_filter.I added tests in
mod_event_pusher_rabbit_SUITE.Note: Although only the Rabbit backend supports metadata, we could follow with other backends later - when there is need. Other backends simply ignore metadata for now.