Skip to content

Only start syslog application if it is required #1728

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

Merged
merged 1 commit into from
Oct 12, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ define PROJECT_ENV
endef

LOCAL_DEPS = sasl mnesia os_mon inets
BUILD_DEPS = rabbitmq_cli
DEPS = ranch syslog lager rabbit_common
BUILD_DEPS = rabbitmq_cli syslog
DEPS = ranch lager rabbit_common
TEST_DEPS = rabbitmq_ct_helpers rabbitmq_ct_client_helpers amqp_client meck proper

dep_syslog = git https://github.com/schlagert/syslog 3.4.3
dep_syslog = git https://github.com/schlagert/syslog 3.4.5

define usage_xml_to_erl
$(subst __,_,$(patsubst $(DOCS_DIR)/rabbitmq%.1.xml, src/rabbit_%_usage.erl, $(subst -,_,$(1))))
Expand Down
1 change: 0 additions & 1 deletion src/rabbit.erl
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,6 @@ start_apps(Apps) ->

start_apps(Apps, RestartTypes) ->
app_utils:load_applications(Apps),

ConfigEntryDecoder = case application:get_env(rabbit, config_entry_decoder) of
undefined ->
[];
Expand Down
103 changes: 75 additions & 28 deletions src/rabbit_lager.erl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

-module(rabbit_lager).

-include("rabbit_log.hrl").
-include_lib("rabbit_common/include/rabbit_log.hrl").

%% API
-export([start_logger/0, log_locations/0, fold_sinks/2,
Expand All @@ -26,11 +26,10 @@
-export([configure_lager/0]).

start_logger() ->
application:stop(lager),
application:stop(syslog),
ensure_lager_configured(),
application:ensure_all_started(syslog),
lager:start(),
ok = maybe_remove_logger_handler(),
ok = app_utils:stop_applications([lager, syslog]),
ok = ensure_lager_configured(),
ok = app_utils:start_applications([lager]),
fold_sinks(
fun
(_, [], Acc) ->
Expand Down Expand Up @@ -176,9 +175,12 @@ lager_configured() ->
application:get_env(syslog, syslog_error_logger) =/= undefined.

configure_lager() ->
application:load(lager),
application:load(syslog),
ok = app_utils:load_applications([lager]),
%% Turn off reformatting for error_logger messages
case application:get_env(lager, error_logger_redirect) of
undefined -> application:set_env(lager, error_logger_redirect, true);
_ -> ok
end,
case application:get_env(lager, error_logger_format_raw) of
undefined -> application:set_env(lager, error_logger_format_raw, true);
_ -> ok
Expand All @@ -190,7 +192,7 @@ configure_lager() ->
%% difference.
case application:get_env(rabbit, lager_log_root) of
{ok, Value} ->
application:set_env(lager, log_root, Value);
ok = application:set_env(lager, log_root, Value);
_ ->
ok
end;
Expand All @@ -199,7 +201,7 @@ configure_lager() ->
%% Set rabbit.log config variable based on environment.
prepare_rabbit_log_config(),
%% Configure syslog library.
configure_syslog(),
ok = configure_syslog_error_logger(),
%% At this point we should have rabbit.log application variable
%% configured to generate RabbitMQ log handlers.
GeneratedHandlers = generate_lager_handlers(),
Expand All @@ -217,8 +219,8 @@ configure_lager() ->
FormerRabbitHandlers)
end,

application:set_env(lager, handlers, Handlers),
application:set_env(lager, rabbit_handlers, GeneratedHandlers),
ok = application:set_env(lager, handlers, Handlers),
ok = application:set_env(lager, rabbit_handlers, GeneratedHandlers),

%% Setup extra sink/handlers. If they are not configured, redirect
%% messages to the default sink. To know the list of expected extra
Expand Down Expand Up @@ -253,29 +255,60 @@ configure_lager() ->
[error_logger_lager_event | list_expected_sinks()],
SinkConfigs),
Sinks = merge_lager_sink_handlers(LagerSinks, GeneratedSinks, []),
application:set_env(lager, extra_sinks, Sinks),
ok = application:set_env(lager, extra_sinks, Sinks),

case application:get_env(lager, error_logger_hwm) of
undefined ->
application:set_env(lager, error_logger_hwm, 1000),
ok = application:set_env(lager, error_logger_hwm, 1000),
% NB: 50 is the default value in lager.app.src
application:set_env(lager, error_logger_hwm_original, 50);
ok = application:set_env(lager, error_logger_hwm_original, 50);
{ok, Val} when is_integer(Val) andalso Val < 1000 ->
application:set_env(lager, error_logger_hwm, 1000),
application:set_env(lager, error_logger_hwm_original, Val);
ok = application:set_env(lager, error_logger_hwm, 1000),
ok = application:set_env(lager, error_logger_hwm_original, Val);
{ok, Val} ->
application:set_env(lager, error_logger_hwm_original, Val),
ok = application:set_env(lager, error_logger_hwm_original, Val),
ok
end,
ok.

configure_syslog() ->
configure_syslog_error_logger() ->
%% Disable error_logger forwarding to syslog if it's not configured
case application:get_env(syslog, syslog_error_logger) of
undefined -> application:set_env(syslog, syslog_error_logger, false);
undefined ->
application:set_env(syslog, syslog_error_logger, false);
_ -> ok
end.

-define(SYSLOG_LOCAL_IP, {ip,{127,0,0,1}}).
configure_syslog() ->
ok = app_utils:load_applications([syslog]),
%% https://github.com/schlagert/syslog#configuration
Protocol = case application:get_env(syslog, protocol) of
undefined ->
{rfc3164, udp, [?SYSLOG_LOCAL_IP]};
%% {protocol,
%% rfc3164 |
%% rfc5424 |
%% {rfc3164 | rfc5424, tcp | udp} |
%% {rfc3164 | rfc5424, udp, [gen_udp:option()]} |
%% {rfc3164 | rfc5424, tcp, [gen_tcp:option()]} |
%% {rfc5424, tls, [ssl:connect_option()]}
%% }
{ok, Rfc} when Rfc =:= rfc3164; Rfc =:= rfc5424 ->
{Rfc, udp, [?SYSLOG_LOCAL_IP]};
{ok, {Rfc, Transport}} when Rfc =:= rfc3164; Rfc =:= rfc5424 ->
{Rfc, Transport, [?SYSLOG_LOCAL_IP]};
{ok, {Rfc, Transport, Opts}} when Rfc =:= rfc3164; Rfc =:= rfc5424 ->
case proplists:lookup(ip, Opts) of
none ->
{Rfc, Transport, [?SYSLOG_LOCAL_IP|Opts]};
_ ->
{Rfc, Transport, Opts}
end
end,
ok = application:unset_env(syslog, protocol),
ok = application:set_env(syslog, protocol, Protocol).

remove_rabbit_handlers(Handlers, FormerHandlers) ->
lists:filter(fun(Handler) ->
not lists:member(Handler, FormerHandlers)
Expand Down Expand Up @@ -317,8 +350,9 @@ lager_backend(exchange) -> lager_exchange_backend.
%% Syslog backend is using an old API for configuration and
%% does not support proplists.
generate_handler(syslog_lager_backend, HandlerConfig) ->
Level = proplists:get_value(level, HandlerConfig,
default_config_value(level)),
DefaultConfigVal = default_config_value(level),
Level = proplists:get_value(level, HandlerConfig, DefaultConfigVal),
ok = configure_syslog(),
[{syslog_lager_backend,
[Level,
{},
Expand Down Expand Up @@ -384,7 +418,7 @@ prepare_rabbit_log_config() ->

set_env_default_log_disabled() ->
%% Disabling all the logs.
application:set_env(rabbit, log, []).
ok = application:set_env(rabbit, log, []).

set_env_default_log_console() ->
LogConfig = application:get_env(rabbit, log, []),
Expand All @@ -395,7 +429,7 @@ set_env_default_log_console() ->
{enabled, true})}),
%% Remove the file handler - disable logging to file
LogConfigConsoleNoFile = lists:keydelete(file, 1, LogConfigConsole),
application:set_env(rabbit, log, LogConfigConsoleNoFile).
ok = application:set_env(rabbit, log, LogConfigConsoleNoFile).

set_env_default_log_file(FileName, Override) ->
LogConfig = application:get_env(rabbit, log, []),
Expand All @@ -416,7 +450,7 @@ set_env_default_log_file(FileName, Override) ->
LogConfig
end
end,
application:set_env(rabbit, log, NewLogConfig).
ok = application:set_env(rabbit, log, NewLogConfig).

set_env_upgrade_log_file(FileName) ->
LogConfig = application:get_env(rabbit, log, []),
Expand All @@ -436,7 +470,7 @@ set_env_upgrade_log_file(FileName) ->
%% No cahnge. We don't want to override the configured value.
_File -> LogConfig
end,
application:set_env(rabbit, log, NewLogConfig).
ok = application:set_env(rabbit, log, NewLogConfig).

generate_lager_sinks(SinkNames, SinkConfigs) ->
lists:map(fun(SinkName) ->
Expand Down Expand Up @@ -523,7 +557,6 @@ merge_lager_sink_handlers([{Name, Sink} | RestSinks], GeneratedSinks, Agg) ->
end;
merge_lager_sink_handlers([], GeneratedSinks, Agg) -> GeneratedSinks ++ Agg.


list_expected_sinks() ->
case application:get_env(rabbit, lager_extra_sinks) of
{ok, List} ->
Expand All @@ -543,6 +576,20 @@ list_expected_sinks() ->
%% module is later cover-compiled, the compile option will
%% be lost, so we will be able to retrieve the list from the
%% application environment.
application:set_env(rabbit, lager_extra_sinks, List),
ok = application:set_env(rabbit, lager_extra_sinks, List),
List
end.

maybe_remove_logger_handler() ->
M = logger,
F = remove_handler,
try
ok = erlang:apply(M, F, [default])
catch
error:undef ->
% OK since the logger module only exists in OTP 21.1 or later
ok;
Err:Reason ->
error_logger:error_msg("calling ~p:~p failed: ~p:~p~n",
[M, F, Err, Reason])
end.
25 changes: 16 additions & 9 deletions src/rabbit_plugins.erl
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ setup() ->
{error, E1} -> throw({error, {cannot_delete_plugins_expand_dir,
[ExpandDir, E1]}})
end,

Enabled = enabled_plugins(),
prepare_plugins(Enabled).

Expand Down Expand Up @@ -196,7 +195,7 @@ list(PluginsPath, IncludeRequiredDeps) ->
{AllPlugins, LoadingProblems} = discover_plugins(split_path(PluginsPath)),
{UniquePlugins, DuplicateProblems} = remove_duplicate_plugins(AllPlugins),
Plugins1 = maybe_keep_required_deps(IncludeRequiredDeps, UniquePlugins),
Plugins2 = remove_otp_overrideable_plugins(Plugins1),
Plugins2 = remove_plugins(Plugins1),
maybe_report_plugin_loading_problems(LoadingProblems ++ DuplicateProblems),
ensure_dependencies(Plugins2).

Expand Down Expand Up @@ -248,11 +247,19 @@ strictly_plugins(Plugins) ->

%% For a few known cases, an externally provided plugin can be trusted.
%% In this special case, it overrides the plugin.
plugin_provided_by_otp(#plugin{name = eldap}) ->
is_plugin_provided_by_otp(#plugin{name = eldap}) ->
%% eldap was added to Erlang/OTP R15B01 (ERTS 5.9.1). In this case,
%% we prefer this version to the plugin.
rabbit_misc:version_compare(erlang:system_info(version), "5.9.1", gte);
plugin_provided_by_otp(_) ->
is_plugin_provided_by_otp(_) ->
false.

is_skipped_plugin(#plugin{name = syslog}) ->
% syslog is shipped as an .ez file, but it's not an actual plugin and
% it's not a direct dependency of the rabbit application, so we must
% skip it here
true;
is_skipped_plugin(_) ->
false.

%% Make sure we don't list OTP apps in here, and also that we detect
Expand Down Expand Up @@ -295,7 +302,6 @@ running_plugins() ->

prepare_plugins(Enabled) ->
ExpandDir = plugins_expand_dir(),

AllPlugins = list(plugins_dir()),
Wanted = dependencies(false, Enabled, AllPlugins),
WantedPlugins = lookup_plugins(Wanted, AllPlugins),
Expand All @@ -306,7 +312,6 @@ prepare_plugins(Enabled) ->
{error, E2} -> throw({error, {cannot_create_plugins_expand_dir,
[ExpandDir, E2]}})
end,

[prepare_plugin(Plugin, ExpandDir) || Plugin <- ValidPlugins],
Wanted.

Expand Down Expand Up @@ -681,9 +686,11 @@ list_all_deps([Application | Applications], Deps) ->
list_all_deps([], Deps) ->
Deps.

remove_otp_overrideable_plugins(Plugins) ->
lists:filter(fun(P) -> not plugin_provided_by_otp(P) end,
Plugins).
remove_plugins(Plugins) ->
Fun = fun(P) ->
not (is_plugin_provided_by_otp(P) orelse is_skipped_plugin(P))
end,
lists:filter(Fun, Plugins).

maybe_report_plugin_loading_problems([]) ->
ok;
Expand Down