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

Use structured logging #29

Merged
merged 12 commits into from
May 30, 2022
Merged

Use structured logging #29

merged 12 commits into from
May 30, 2022

Conversation

mmzeeman
Copy link
Member

@mmzeeman mmzeeman commented Apr 13, 2022

This PR changes log messages to structured logging format.

@@ -196,7 +196,7 @@ block_lookup({ok, TplFile}, BlockMap, ExtendsStack, DebugTrace, Options, Vars, R
case lists:member(Module, ExtendsStack) of
true ->
FileTrace = [Module:filename() | [ M:filename() || M <- ExtendsStack ]],
?LOG_ERROR("[template_compiler] Template recursion: ~p", [FileTrace]),
?LOG_ERROR(#{ text => "Template recursion.", trace => FileTrace}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea, could we transform the trace to a stack trace format? Then the logger can show it as a stack trace.

Copy link
Member Author

@mmzeeman mmzeeman Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably.. I want to change the yecc errors top maps as well. That will probably improve the readability. Not sure what a filetrace looks like tbh.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great, the Yecc errors are quite confusing. We should be able to see in one go what the problem is and where.

Error
end;
error ->
?LOG_ERROR("Error compiling forms for ~p", [Filename]),
?LOG_ERROR(#{ text => "Error compiling forms.", filename => Filename}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the other logging I always added result => error and then, if known, reason => ...

src/template_compiler.erl Outdated Show resolved Hide resolved
src/template_compiler.erl Outdated Show resolved Hide resolved
src/template_compiler.erl Outdated Show resolved Hide resolved
src/template_compiler.erl Outdated Show resolved Hide resolved
src/template_compiler_admin.erl Outdated Show resolved Hide resolved
src/template_compiler_runtime_internal.erl Outdated Show resolved Hide resolved
src/template_compiler_runtime_internal.erl Outdated Show resolved Hide resolved
src/template_compiler_runtime_internal.erl Outdated Show resolved Hide resolved
@mworrell
Copy link
Member

Rebased on master

Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
@mmzeeman
Copy link
Member Author

Yeah nice... Only the yecc errors are still confusing. :p

mworrell and others added 10 commits May 30, 2022 15:33
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
Co-authored-by: Viacheslav Katsuba <v.katsuba.dev@gmail.com>
@mworrell mworrell merged commit a600e32 into master May 30, 2022
@mworrell mworrell deleted the structured-logging branch May 30, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants