Happy hello logging #69
Conversation
| -define(LOG_WARNING_reason(CallbackModule, HandlerId, Msg, Args, Reason, LogId), | ||
| lager:info(?DEFAULT_META( [ {hello_handler_callback, CallbackModule}, {hello_error_reason, Reason}, | ||
| {hello_service_id, HandlerId}], | ||
| LogId)), Msg, Args). |
There was a problem hiding this comment.
Seems there is a problem with brackets
There was a problem hiding this comment.
If you find the reason for that compiler warning I'll spend you a beer when you come here ;P. Sasha and I couldn't find it, its probably related to parse_transform.
There was a problem hiding this comment.
-define(LOG_WARNING_reason(CallbackModule, HandlerId, Msg, Args, Reason, LogId),
lager:info(?DEFAULT_META( [ {hello_handler_callback, CallbackModule},
{hello_error_reason, Reason},
{hello_service_id, HandlerId}], LogId), Msg, Args)).look on last bracket
There was a problem hiding this comment.
@GalaxyGorilla we have fixed it, there are no warnings;) and you have one bracket again)
There was a problem hiding this comment.
Uuups... So I guess pt prevented a compiler error?
| {noreply, Table}; | ||
| handle_info({'DOWN', _MRef, process, Pid, Reason}, Table) -> | ||
| Objects = ets:match(Table, {'$1', Pid, '_', '_'}), | ||
| ?LOG_WARNING("~p: down ~p with reason ~p", [Pid, Objects, Reason]), | ||
| ?LOG_INFO("Hello registry received 'DOWN' signal from monitored process '~p' with reason '~p'. | ||
| Going to clean up associated processes '~p'.", [Pid, Reason, Objects], |
There was a problem hiding this comment.
whitespaces and new line. do you need it?
There was a problem hiding this comment.
Very good point, I'll work this over.
|
I wasn't a part of discussion about logging but on my first view there are so many ids: 6 digits status code, very long hex for message and .hrl file with conjunction of it. Do we really need to put all of it to small open source framework? May we generate some of it automatically or do something for improve understanding of these ids? |
| @@ -72,7 +72,8 @@ init(URL) -> | |||
| State = #state{socket = Socket, url = URL}, | |||
| {ok, State}; | |||
| {error, Error} -> | |||
| ?LOG_ERROR("ezmq_bind_url error: ~p", [Error]), | |||
| ?LOG_INFO("Hello ZeroMQ listener was unable to bind on '~p' because of reason '~p'.", [URL, Error], | |||
| [{hello_transport, zmtp}, {hello_transport_url, ex_uri:encode(URL)}], ?LOGID47), | |||
There was a problem hiding this comment.
Can we save encoded URL in State and use it when it needed?
There was a problem hiding this comment.
I have no problem with that, I'll include that in the following commit.
|
@surik this is our internal logging standard, a short explanation what's what:
both of these are yielding binding elements which makes it easier for OPS to see what's going on in the application |
- introduction to status codes and message ids
- human readable log messages
- log level is configured to info or debugfor every log
- documentation of log messages with status codes
- more logs in hello client
- introduction of several tracing possibilities
- more meta fields for journald
- no functional changes should be included
GalaxyGorilla
force-pushed
the
tp_hello_pr
branch
from
a885f9b
to
833b40e
Compare
|
Fixed code formatting, squashed commits. |
This commit aims to enhance the logging in hello regarding aspects:
- introduction to status codes and message ids
- human readable log messages
- log level is configured to info or debugfor every log
- documentation of log messages with status codes
- more logs in hello client
- introduction of several tracing possibilities
- more meta fields for journald
- no functional changes should be included