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

Clean up logs 3.x.v2 #3380

Merged
merged 4 commits into from Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion rebar.config.script
Expand Up @@ -144,7 +144,7 @@ SubDirs = [

DepDescs = [
%% Independent Apps
{config, "config", {tag, "2.1.7"}},
{config, "config", {tag, "2.1.8"}},
{b64url, "b64url", {tag, "1.0.2"}},
{ets_lru, "ets-lru", {tag, "1.1.0"}},
{khash, "khash", {tag, "1.1.0"}},
Expand Down
4 changes: 3 additions & 1 deletion src/chttpd/src/chttpd_node.erl
Expand Up @@ -71,7 +71,9 @@ handle_node_req(#httpd{method='PUT', path_parts=[_, Node, <<"_config">>, Section
Value = couch_util:trim(chttpd:json_body(Req)),
Persist = chttpd:header_value(Req, "X-Couch-Persist") /= "false",
OldValue = call_node(Node, config, get, [Section, Key, ""]),
case call_node(Node, config, set, [Section, Key, ?b2l(Value), Persist]) of
IsSensitive = Section == <<"admins">>,
Opts = #{persist => Persist, sensitive => IsSensitive},
case call_node(Node, config, set, [Section, Key, ?b2l(Value), Opts]) of
ok ->
send_json(Req, 200, list_to_binary(OldValue));
{error, Reason} ->
Expand Down
11 changes: 9 additions & 2 deletions src/couch_log/src/couch_log_config.erl
Expand Up @@ -49,7 +49,8 @@ entries() ->
[
{level, "level", "info"},
{level_int, "level", "info"},
{max_message_size, "max_message_size", "16000"}
{max_message_size, "max_message_size", "16000"},
{strip_last_msg, "strip_last_msg", "true"}
].


Expand Down Expand Up @@ -97,4 +98,10 @@ transform(max_message_size, SizeStr) ->
Size -> Size
catch _:_ ->
16000
end.
end;

transform(strip_last_msg, "false") ->
false;

transform(strip_last_msg, _) ->
true.
3 changes: 2 additions & 1 deletion src/couch_log/src/couch_log_config_dyn.erl
Expand Up @@ -25,4 +25,5 @@

get(level) -> info;
get(level_int) -> 2;
get(max_message_size) -> 16000.
get(max_message_size) -> 16000;
get(strip_last_msg) -> true.
24 changes: 21 additions & 3 deletions src/couch_log/src/couch_log_formatter.erl
Expand Up @@ -68,23 +68,41 @@ format(Event) ->

do_format({error, _GL, {Pid, "** Generic server " ++ _, Args}}) ->
%% gen_server terminate
[Name, LastMsg, State, Reason | Extra] = Args,
[Name, LastMsg0, State, Reason | Extra] = Args,
LastMsg = case couch_log_config:get(strip_last_msg) of
true ->
redacted;
false ->
LastMsg0
end,
MsgFmt = "gen_server ~w terminated with reason: ~s~n" ++
" last msg: ~p~n state: ~p~n extra: ~p",
MsgArgs = [Name, format_reason(Reason), LastMsg, State, Extra],
format(error, Pid, MsgFmt, MsgArgs);

do_format({error, _GL, {Pid, "** State machine " ++ _, Args}}) ->
%% gen_fsm terminate
[Name, LastMsg, StateName, State, Reason | Extra] = Args,
[Name, LastMsg0, StateName, State, Reason | Extra] = Args,
LastMsg = case couch_log_config:get(strip_last_msg) of
true ->
redacted;
false ->
LastMsg0
end,
MsgFmt = "gen_fsm ~w in state ~w terminated with reason: ~s~n" ++
" last msg: ~p~n state: ~p~n extra: ~p",
MsgArgs = [Name, StateName, format_reason(Reason), LastMsg, State, Extra],
format(error, Pid, MsgFmt, MsgArgs);

do_format({error, _GL, {Pid, "** gen_event handler" ++ _, Args}}) ->
%% gen_event handler terminate
[ID, Name, LastMsg, State, Reason] = Args,
[ID, Name, LastMsg0, State, Reason] = Args,
LastMsg = case couch_log_config:get(strip_last_msg) of
true ->
redacted;
false ->
LastMsg0
end,
MsgFmt = "gen_event ~w installed in ~w terminated with reason: ~s~n" ++
" last msg: ~p~n state: ~p",
MsgArgs = [ID, Name, format_reason(Reason), LastMsg, State],
Expand Down
2 changes: 2 additions & 0 deletions src/couch_log/src/couch_log_sup.erl
Expand Up @@ -63,6 +63,8 @@ handle_config_change("log", Key, _, _, S) ->
couch_log_config:reconfigure();
"max_message_size" ->
couch_log_config:reconfigure();
"strip_last_msg" ->
couch_log_config:reconfigure();
_ ->
% Someone may have changed the config for
% the writer so we need to re-initialize.
Expand Down
37 changes: 36 additions & 1 deletion src/couch_log/test/eunit/couch_log_config_test.erl
Expand Up @@ -25,7 +25,9 @@ couch_log_config_test_() ->
fun check_level/0,
fun check_max_message_size/0,
fun check_bad_level/0,
fun check_bad_max_message_size/0
fun check_bad_max_message_size/0,
fun check_strip_last_msg/0,
fun check_bad_strip_last_msg/0
]
}.

Expand Down Expand Up @@ -108,3 +110,36 @@ check_bad_max_message_size() ->
couch_log_test_util:wait_for_config(),
?assertEqual(16000, couch_log_config:get(max_message_size))
end).


check_strip_last_msg() ->
% Default is true
?assertEqual(true, couch_log_config:get(strip_last_msg)),

couch_log_test_util:with_config_listener(fun() ->
config:set("log", "strip_last_msg", "false"),
couch_log_test_util:wait_for_config(),
?assertEqual(false, couch_log_config:get(strip_last_msg)),

config:delete("log", "strip_last_msg"),
couch_log_test_util:wait_for_config(),
?assertEqual(true, couch_log_config:get(strip_last_msg))
end).

check_bad_strip_last_msg() ->
% Default is true
?assertEqual(true, couch_log_config:get(strip_last_msg)),

couch_log_test_util:with_config_listener(fun() ->
config:set("log", "strip_last_msg", "false"),
couch_log_test_util:wait_for_config(),
?assertEqual(false, couch_log_config:get(strip_last_msg)),

config:set("log", "strip_last_msg", "this is not a boolean"),
couch_log_test_util:wait_for_config(),
?assertEqual(true, couch_log_config:get(strip_last_msg)),

config:delete("log", "strip_last_msg"),
couch_log_test_util:wait_for_config(),
?assertEqual(true, couch_log_config:get(strip_last_msg))
end).
114 changes: 109 additions & 5 deletions src/couch_log/test/eunit/couch_log_formatter_test.erl
Expand Up @@ -81,7 +81,7 @@ gen_server_error_test() ->
do_matches(do_format(Event), [
"gen_server a_gen_server terminated",
"with reason: some_reason",
"last msg: {foo,bar}",
"last msg: redacted",
"state: server_state",
"extra: \\[\\]"
]).
Expand All @@ -108,7 +108,7 @@ gen_server_error_with_extra_args_test() ->
do_matches(do_format(Event), [
"gen_server a_gen_server terminated",
"with reason: some_reason",
"last msg: {foo,bar}",
"last msg: redacted",
"state: server_state",
"extra: \\[sad,args\\]"
]).
Expand All @@ -135,7 +135,7 @@ gen_fsm_error_test() ->
do_matches(do_format(Event), [
"gen_fsm a_gen_fsm in state state_name",
"with reason: barf",
"last msg: {ohai,there}",
"last msg: redacted",
"state: curr_state",
"extra: \\[\\]"
]).
Expand All @@ -162,7 +162,7 @@ gen_fsm_error_with_extra_args_test() ->
do_matches(do_format(Event), [
"gen_fsm a_gen_fsm in state state_name",
"with reason: barf",
"last msg: {ohai,there}",
"last msg: redacted",
"state: curr_state",
"extra: \\[sad,args\\]"
]).
Expand Down Expand Up @@ -195,7 +195,7 @@ gen_event_error_test() ->
do_matches(do_format(Event), [
"gen_event handler_id installed in a_gen_event",
"reason: barf",
"last msg: {ohai,there}",
"last msg: redacted",
"state: curr_state"
]).

Expand Down Expand Up @@ -850,6 +850,110 @@ coverage_test() ->
})
).

gen_server_error_with_last_msg_test() ->
Pid = self(),
Event = {
error,
erlang:group_leader(),
{
Pid,
"** Generic server and some stuff",
[a_gen_server, {foo, bar}, server_state, some_reason]
}
},
?assertMatch(
#log_entry{
level = error,
pid = Pid
},
do_format(Event)
),
with_last(fun() ->
do_matches(do_format(Event), [
"gen_server a_gen_server terminated",
"with reason: some_reason",
"last msg: {foo,bar}",
"state: server_state",
"extra: \\[\\]"
])
end).

gen_event_error_with_last_msg_test() ->
Pid = self(),
Event = {
error,
erlang:group_leader(),
{
Pid,
"** gen_event handler did a thing",
[
handler_id,
a_gen_event,
{ohai,there},
curr_state,
barf
]
}
},
?assertMatch(
#log_entry{
level = error,
pid = Pid
},
do_format(Event)
),
with_last(fun() ->
do_matches(do_format(Event), [
"gen_event handler_id installed in a_gen_event",
"reason: barf",
"last msg: {ohai,there}",
"state: curr_state"
])
end).


gen_fsm_error_with_last_msg_test() ->
Pid = self(),
Event = {
error,
erlang:group_leader(),
{
Pid,
"** State machine did a thing",
[a_gen_fsm, {ohai,there}, state_name, curr_state, barf]
}
},
?assertMatch(
#log_entry{
level = error,
pid = Pid
},
do_format(Event)
),
with_last(fun() ->
do_matches(do_format(Event), [
"gen_fsm a_gen_fsm in state state_name",
"with reason: barf",
"last msg: {ohai,there}",
"state: curr_state",
"extra: \\[\\]"
])
end).


with_last(Fun) ->
meck:new(couch_log_config_dyn, [passthrough]),
try
meck:expect(couch_log_config_dyn, get, fun(Case) ->
case Case of
strip_last_msg -> false;
Case -> meck:passthrough([Case])
end
end),
Fun()
after
meck:unload(couch_log_config_dyn)
end.

do_format(Event) ->
E = couch_log_formatter:format(Event),
Expand Down
31 changes: 30 additions & 1 deletion src/couch_replicator/src/couch_replicator_httpc_pool.erl
Expand Up @@ -20,7 +20,7 @@

% gen_server API
-export([init/1, handle_call/3, handle_info/2, handle_cast/2]).
-export([code_change/3, terminate/2]).
-export([code_change/3, terminate/2, format_status/2]).

-include_lib("couch/include/couch_db.hrl").

Expand Down Expand Up @@ -145,6 +145,16 @@ code_change(_OldVsn, #state{}=State, _Extra) ->
terminate(_Reason, _State) ->
ok.

format_status(_Opt, [_PDict, State]) ->
#state{
url = Url,
proxy_url = ProxyUrl
} = State,
[{data, [{"State", State#state{
url = couch_util:url_strip_password(Url),
proxy_url = couch_util:url_strip_password(ProxyUrl)
}}]}].

monitor_client(Callers, Worker, {ClientPid, _}) ->
[{Worker, erlang:monitor(process, ClientPid)} | Callers].

Expand Down Expand Up @@ -182,3 +192,22 @@ release_worker_internal(Worker, State) ->
false ->
State#state{callers = NewCallers0}
end.


-ifdef(TEST).

-include_lib("couch/include/couch_eunit.hrl").

format_status_test_() ->
?_test(begin
State = #state{
url = "https://username1:password1@$ACCOUNT2.cloudant.com/db",
proxy_url = "https://username2:password2@proxy.thing.com:8080/"
},
[{data, [{"State", ScrubbedN}]}] = format_status(normal, [[], State]),
?assertEqual("https://username1:*****@$ACCOUNT2.cloudant.com/db", ScrubbedN#state.url),
?assertEqual("https://username2:*****@proxy.thing.com:8080/", ScrubbedN#state.proxy_url),
ok
end).

-endif.
2 changes: 1 addition & 1 deletion src/setup/src/setup.erl
Expand Up @@ -165,7 +165,7 @@ enable_cluster_int(Options, false) ->
couch_log:debug("Enable Cluster: ~p~n", [Options]).

set_admin(Username, Password) ->
config:set("admins", binary_to_list(Username), binary_to_list(Password)).
config:set("admins", binary_to_list(Username), binary_to_list(Password), #{sensitive => true}).

setup_node(NewCredentials, NewBindAddress, NodeCount, Port) ->
case NewCredentials of
Expand Down