From 98cdb4d9f2588743b095db150392f29dfb17d69d Mon Sep 17 00:00:00 2001 From: Martin Sumner Date: Wed, 13 Nov 2024 13:56:57 +0000 Subject: [PATCH] Make log functions exportable (#455) * Make log functions exportable To make it easier to switch to logger in kv_index_tictactree - export the log functions from leveled so that they can be reused * Changes post review Added brief description of the module to explain why the approach to logging is used. * Result of log should be `ok` --- src/leveled_log.erl | 79 +++++++++++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/src/leveled_log.erl b/src/leveled_log.erl index d35a6557..4c1d9f12 100644 --- a/src/leveled_log.erl +++ b/src/leveled_log.erl @@ -1,5 +1,24 @@ -%% Module to abstract from choice of logger, and allow use of logReferences -%% for fast lookup +%% -------- LOG --------- +%% +%% Centralised logging, to make it easier to change log implementation. +%% +%% The use of a ?LOGBASE map is a personal preference. With formatting of code +%% using maximum column widths, I prefer not to have log text within the code +%% itself, as there may be a temptation to make log text misleadingly terse to +%% make the code more readable. +%% +%% This means that using logger's capability to add actual code line references +%% to the log becomes mute - as all logs log from the same code line. However, +%% the process does enforce the use of log references to provide a simple way +%% to search the code and find where the log occurred. This can be helpful +%% when there are frequent code changes which may change line numbers between +%% releases (whereas log references stay stable). The log references +%% themselves can be helpful when optimising query times in splunk-like log +%% indexing tools. +%% +%% There are overheads with the approach (e.g. the maps:get/2 call for each log +%% ). However, the eprof testing of leveled indicates that this is not a +%% relatively significant overhead. -module(leveled_log). @@ -9,6 +28,8 @@ log_timer/3, log_randomtimer/4]). +-export([log/5, log_timer/6]). + -export([set_loglevel/1, set_databaseid/1, add_forcedlogs/1, @@ -24,8 +45,9 @@ -type log_level() :: debug | info | warning | error | critical. -type log_options() :: #log_options{}. +-type log_base() :: #{atom() => {log_level(), binary()}}. --export_type([log_options/0, log_level/0]). +-export_type([log_options/0, log_level/0, log_base/0]). -define(LOG_LEVELS, [debug, info, warning, error, critical]). -define(DEFAULT_LOG_LEVEL, error). @@ -273,7 +295,7 @@ ic012 => {warning, <<"Tag ~w not found in Strategy ~w - maybe corrupted">>}, ic013 => - {warning, "File with name ~s to be ignored in manifest as scanning for first key returned empty - maybe corrupted"}, + {warning, <<"File with name ~s to be ignored in manifest as scanning for first key returned empty - maybe corrupted">>}, ic014 => {info, <<"Compaction to be run with strategy ~w and max_run_length ~w">>}, cdb01 => @@ -390,12 +412,13 @@ return_settings() -> -spec log(atom(), list()) -> ok. log(LogReference, Subs) -> - log(LogReference, Subs, ?LOG_LEVELS). + log(LogReference, Subs, ?LOG_LEVELS, ?LOGBASE, backend). -log(LogRef, Subs, SupportedLogLevels) -> - {LogLevel, Log} = maps:get(LogRef, ?LOGBASE), +-spec log(atom(), list(), list(log_level()), log_base(), atom()) -> ok. +log(LogRef, Subs, SupportedLevels, LogBase, Tag) -> + {LogLevel, Log} = maps:get(LogRef, LogBase), LogOpts = get_opts(), - case should_i_log(LogLevel, SupportedLogLevels, LogRef, LogOpts) of + case should_i_log(LogLevel, SupportedLevels, LogRef, LogOpts) of true -> DBid = LogOpts#log_options.database_id, Prefix = @@ -405,7 +428,7 @@ log(LogRef, Subs, SupportedLogLevels) -> LogLevel, unicode:characters_to_list([Prefix, Log, Suffix]), Subs, - #{log_type => backend} + #{log_type => Tag} ); false -> ok @@ -430,10 +453,13 @@ is_active_level([_|T], C, L) -> is_active_level(T, C, L). -spec log_timer(atom(), list(), erlang:timestamp()) -> ok. log_timer(LogReference, Subs, StartTime) -> - log_timer(LogReference, Subs, StartTime, ?LOG_LEVELS). + log_timer(LogReference, Subs, StartTime, ?LOG_LEVELS, ?LOGBASE, backend). -log_timer(LogRef, Subs, StartTime, SupportedLevels) -> - {LogLevel, Log} = maps:get(LogRef, ?LOGBASE), +-spec log_timer( + atom(), list(), erlang:timestamp(), list(log_level()), log_base(), atom()) + -> ok. +log_timer(LogRef, Subs, StartTime, SupportedLevels, LogBase, Tag) -> + {LogLevel, Log} = maps:get(LogRef, LogBase), LogOpts = get_opts(), case should_i_log(LogLevel, SupportedLevels, LogRef, LogOpts) of true -> @@ -446,13 +472,13 @@ log_timer(LogRef, Subs, StartTime, SupportedLevels) -> LogLevel, unicode:characters_to_list([Prefix, Log, Duration, Suffix]), Subs, - #{log_type => backend} + #{log_type => Tag} ); false -> ok end. --spec log_randomtimer(atom(), list(), erlang:timestamp(), float()) -> ok. +-spec log_randomtimer(atom(), list(), erlang:timestamp(), float()) -> term(). log_randomtimer(LogReference, Subs, StartTime, RandomProb) -> R = rand:uniform(), case R < RandomProb of @@ -488,14 +514,27 @@ duration_text(StartTime) -> should_i_log(LogLevel, Levels, LogRef) -> should_i_log(LogLevel, Levels, LogRef, get_opts()). - -log_test() -> - log(d0001, []), - log_timer(d0001, [], os:timestamp()). log_warning_test() -> - ok = log(g0001, [], [warning, error]), - ok = log_timer(g0001, [], os:timestamp(), [warning, error]). + ok = log(g0001, [], [warning, error], ?LOGBASE, backend), + ok = + log_timer( + g0001, [], os:timestamp(), [warning, error], ?LOGBASE, backend + ). + +log_wrongkey_test() -> + ?assertException( + error, + {badkey, wrong0001}, + log(wrong0001, [],[warning, error], ?LOGBASE, backend) + ), + ?assertException( + error, + {badkey, wrong0001}, + log_timer( + wrong0001, [], os:timestamp(), [warning, error], ?LOGBASE, backend + ) + ). shouldilog_test() -> ok = set_loglevel(debug),