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

Feature: Add ability to implement custom loggers #313

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

liss-h
Copy link
Contributor

@liss-h liss-h commented Jan 23, 2024

Hi,
this PR implements the ability to implement custom loggers in consuming applications. I've tried to keep the changes minimal, but there is still some room to make them even more minimal (see notes for details).

Some Notes:

  • The way this is currently implemented you first have to define METALL_LOGGER_EXTERN_C=1
    and then provide a defintion for metall_log somewhere. If you don't like how that works, feel free to suggest something else
  • metall_log is intentionally an extern "C" interface so that it can be easily used in FFI/cross-language contexts, i.e. this gives the ability to implement the logger behaviour in a foreign language that is not C or C++, which has the advantage of having the ability to neatly integrate into the native logging interface of the foreign language
  • There is one interface change: I removed the silent logger level and factored it out into the level_filter. The reason being that silent is not really a log level and there isn't really a point in ever logging silent messages, because they are always ignored. But of course this is an interface change so I would understand if you didn't like it, in that case I can revert this detail back to its original state. Otherwise no change in interface has occured (in the default configuration)

This is related to #297

@KIwabuchi KIwabuchi self-requested a review January 30, 2024 02:55
@KIwabuchi
Copy link
Member

I'm glad that you were able to find a way to support custom loggers keeping the header-only design. I also like the level_filter design.
I'm going to accept this PR.

Future work:
I'm wondering if there is a way to avoid not using logger_interface.h file with C++ so that metall_log or metall_log_level is not declared in the global space. I'll think about it.

@KIwabuchi KIwabuchi merged commit 98fcc95 into LLNL:develop Feb 5, 2024
2 checks passed
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.

2 participants