Instrument mod_mam #4224
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| %% @doc This module is injected to the mim node by instrument_helper. | ||
| %% It collects events in an ETS table. | ||
|
|
||
| -module(instrument_event_table). | ||
|
|
||
| -export([set_up/1, tear_down/0, get_events/2]). | ||
|
|
||
| set_up(DeclaredEvents) -> | ||
| ets_helper:new(?MODULE, [bag]), % repeating measurements are stored only once | ||
| meck:new(mongoose_instrument_log, [no_link, passthrough]), | ||
| meck:expect(mongoose_instrument_log, handle_event, | ||
| fun(Event, Labels, _Config, Measurements) -> | ||
| case lists:member({Event, Labels}, DeclaredEvents) of | ||
| true -> ets:insert(?MODULE, {{Event, Labels}, Measurements}); | ||
| false -> ok | ||
| end, | ||
| meck:passthrough() | ||
| end). | ||
|
|
||
| tear_down() -> | ||
| meck:unload(mongoose_instrument_log), | ||
| Logged = all_keys(?MODULE), | ||
| ets_helper:delete(?MODULE), | ||
| {ok, Logged}. | ||
|
|
||
| get_events(EventName, Labels) -> | ||
| ets:lookup(?MODULE, {EventName, Labels}). | ||
|
|
||
| all_keys(Tab) -> | ||
| all_keys(Tab, ets:first(Tab), []). | ||
|
|
||
| all_keys(_Tab, '$end_of_table', Acc) -> | ||
| lists:sort(Acc); | ||
| all_keys(Tab, Key, Acc) -> | ||
| all_keys(Tab, ets:next(Tab, Key), [Key|Acc]). | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| %% @doc A helper module for checking instrumentation events in tests. | ||
| %% For now it always uses mim(). We can extend it to other nodes when needed. | ||
|
|
||
| -module(instrument_helper). | ||
|
|
||
| -export([declared_events/1, start/1, stop/0, assert/3]). | ||
|
|
||
| -import(distributed_helper, [rpc/4, mim/0]). | ||
|
|
||
| -include_lib("eunit/include/eunit.hrl"). | ||
|
|
||
| -define(STATUS_TABLE, instrument_event_status_table). | ||
|
|
||
| -type event_name() :: atom(). | ||
| -type labels() :: #{atom() => term()}. | ||
| -type measurements() :: #{atom() => term()}. | ||
|
|
||
| %% API | ||
|
|
||
| %% @doc Helper to get `DeclaredEvents' needed by `start/1' | ||
| -spec declared_events([module()] | module()) -> [{event_name(), labels()}]. | ||
| declared_events(Modules) when is_list(Modules) -> | ||
| lists:flatmap(fun declared_events/1, Modules); | ||
| declared_events(Module) -> | ||
| Specs = rpc(mim(), Module, instrumentation, [domain_helper:host_type()]), | ||
| [{Event, Labels} || {Event, Labels, _Config} <- Specs]. | ||
|
|
||
| %% @doc Only `DeclaredEvents' will be logged, and can be tested with `assert/3' | ||
| -spec start([{event_name(), labels()} | module()]) -> ok. | ||
| start(DeclaredEvents) -> | ||
| mongoose_helper:inject_module(ets_helper), | ||
| mongoose_helper:inject_module(instrument_event_table), | ||
| ets_helper:new(?STATUS_TABLE), | ||
| [ets:insert(?STATUS_TABLE, {Event, untested}) || Event <- DeclaredEvents], | ||
| ok = rpc(mim(), instrument_event_table, set_up, [DeclaredEvents]). | ||
|
|
||
| -spec stop() -> ok. | ||
| stop() -> | ||
| #{tested := Tested, untested := Untested} = classify_events(), | ||
| ets_helper:delete(?STATUS_TABLE), | ||
| {ok, Logged} = rpc(mim(), instrument_event_table, tear_down, []), | ||
| ct:log("Tested instrumentation events:~n ~p", [lists:sort(Tested)]), | ||
| verify_unlogged(Untested -- Logged), | ||
| verify_logged_but_untested(Logged -- Tested). | ||
|
|
||
| %% @doc `CheckF' can return a boolean or fail with `function_clause', which means `false'. | ||
| %% This is for convenience - you only have to code one clause. | ||
| -spec assert(event_name(), labels(), fun((measurements()) -> boolean())) -> ok. | ||
| assert(EventName, Labels, CheckF) -> | ||
| Events = rpc(mim(), instrument_event_table, get_events, [EventName, Labels]), | ||
| case lists:filter(fun({_, Measurements}) -> | ||
| try CheckF(Measurements) catch error:function_clause -> false end | ||
| end, Events) of | ||
| [] -> | ||
| ct:log("All ~p events with labels ~p:~n~p", [EventName, Labels, Events]), | ||
| ct:fail("No instrumentation events matched"); | ||
| Filtered -> | ||
| ct:log("Matching events: ~p", [Filtered]), | ||
| event_tested(EventName, Labels) | ||
| end. | ||
|
|
||
| %% Internal functions | ||
|
|
||
| %% Don't fail if some events are unlogged, because we don't have full test coverage (yet) | ||
| verify_unlogged([]) -> ok; | ||
| verify_unlogged(Events) -> | ||
| ct:log("Instrumentation events that were not logged - functionality not covered by tests:~n~p", | ||
| [lists:sort(Events)]). | ||
|
|
||
| %% Fail if any events were logged, but there were no assertions for them | ||
| verify_logged_but_untested([]) -> ok; | ||
| verify_logged_but_untested(Events) -> | ||
| ct:fail("Instrumentation events that were logged, but not tested:~n~p~n" | ||
| "You need to test them with ~p:assert/3", | ||
| [lists:sort(Events), ?MODULE]). | ||
|
|
||
| event_tested(EventName, Labels) -> | ||
| ets:insert(?STATUS_TABLE, {{EventName, Labels}, tested}), | ||
| ok. | ||
|
|
||
| classify_events() -> | ||
| ets:foldl(fun classify_event/2, #{tested => [], untested => []}, ?STATUS_TABLE). | ||
|
|
||
| classify_event({Event, Status}, M) -> | ||
| M#{Status => [Event | maps:get(Status, M)]}. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm... There should be a better way than meck here. If we're already rpc-inserting a module here, and we're creating an ets table in the remote node, we can just rpc-insert a module that implements the
mongoose_instrumentbehaviour already and does the ets inserts on its own, entirely bypassing meck.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.
There are many ways without meck, and I tried the following:
mongoose_instrument_*is not easy, because all instrumentation is set up on startup. You would need to restartmongooseim.I implemented all three of them, and the current one is the fourth attempt. I really don't see a cleaner solution.
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.
This is unfortunate. Points 2 and 3 aren't good at all from my point of view, so I'd discard them before even trying. Point 1 is the one that sounds best to me, wonder if there's a way to rework how handlers are initialised so that they can be injected dynamically without restarting MIM. I'll make a note for this one, it can be in a separate PR, it's well encapsulated in the instrumentation helper modules.
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.
I didn't add this functionality, because it's not needed during normal operation, and I didn't want something just for tests. Also, we want to avoid the situation where configuration (
mongoose_config) is different from actual setup (an ad-hoc added handler). So it would be something likedynamic_modules, but for handlers. It's doable, but do we really need it?