log type filtering, riak.conf support #1
Conversation
Leveled uses backend log_type, and kv_index_tictcatree uses aae log_type. This allows for different handling of these logs
Rather than have long translation fun in riak.schema - be able to refer to a translation function within riak_logger.
Have config helper function for producing the same log ouput as in openriak-3.2 as well as new formats for openriak-3.4
There was a problem hiding this comment.
Lots of comments, some to be taken with a grain of salt. I think we're a long way from sorting out the proper structure in here, especially in light of the JSON stuff we've got in progress. Since (I think) this is presently limited to your 3.4 stuff, if we can keep it on nhse branches for now we'll be able to compare with the upcoming wday branch and see what way we want to go.
I think there are generalizations to be had for the handler configs, though I'm not sure what they are yet - I just feel like "there's got to be a better way."
| @@ -69,6 +70,7 @@ | |||
|
|
|||
| -type f_action() :: log | stop. | |||
| -type f_result() :: logger:filter_return(). | |||
| -type f_type() :: backend|aae|metric. | |||
There was a problem hiding this comment.
I think this should be atom(), or even term() - the filter, as implemented, doesn't care about the type or value.
| {Action :: f_action(), LogTypes :: list(f_type())}) | ||
| -> f_result(). | ||
| %% @doc Filter out logs with specific log_types, e.g. such as the backend log | ||
| %% type used in leveled |
There was a problem hiding this comment.
Add a head for the most common case?
filter_t(#{meta := #{log_type := LogType}} = Event, {Action, [LogType]}) ->
filter_match(Event, Action);| case lists:member(LogType, LogTypes) of | ||
| true -> | ||
| filter_match(Event, Action); | ||
| false -> |
There was a problem hiding this comment.
Change false to _ - generates less code.
| %% | ||
| %% ------------------------------------------------------------------- | ||
| %% Portions informed by OTP module logger_filters, | ||
| %% Copyright Ericsson AB 2017-2018, though no code is duplicated. |
There was a problem hiding this comment.
Is there anything in here warranting this attribution?
|
|
||
| -module(riak_logger_config). | ||
|
|
||
| -export([o34std_generator/2, o32std_generator/2]). |
There was a problem hiding this comment.
I don't think the generators should be pegged to versions, but should instead have descriptive names. Different orgs may not be conforming to the exact OpenRiak versions - for instance, some might be using much of the nominal 3.4 code in 3.2, or vice-versa.
If the generators were really version-specific, they wouldn't exist together.
| [Handler] = HandlerList, | ||
| ?assertMatch(ExpectedConfig, Handler). | ||
|
|
||
| wday_handler_test() -> |
There was a problem hiding this comment.
Shouldn't be named per-org, but instead for semantics.
Perhaps std_local_handler_test?
| [{crash_filter, {_FilterFunC, Action}}] = maps:get(filters, CLC), | ||
| ?assertMatch(log, Action). | ||
|
|
||
| nhs_handler_test() -> |
There was a problem hiding this comment.
Shouldn't be named per-org, but instead for semantics.
Perhaps std_divert_handler_test?
| %% The configuration can be ignored if preferred - and a statically defined | ||
| %% logging config be returned | ||
|
|
||
| -module(riak_logger_config). |
There was a problem hiding this comment.
Maybe refactor into more descriptive module name(s), to allow for additions without namespace confusion? Possible names:
riak_log_std_config
riak_log_type_config
riak_log_json_config
That suggests there may be a shared set of functions, perhaps in riak_log_common_config.
|
So, in implementing the JSON logger formatter, I've become fairly convinced that the |
|
OK, I'm happy with that. I was confused by the But if it is in fact the intention for this to be used by applications, I agree that we should use it. |
Changes to add filtering by log_type, and add a helper function to generate configuration from a schema (i.e. riak.conf cuttlefish)