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

Replace Logging by Tinylog #7

Open
koppor opened this issue Apr 19, 2020 · 19 comments
Open

Replace Logging by Tinylog #7

koppor opened this issue Apr 19, 2020 · 19 comments

Comments

@koppor
Copy link
Member

koppor commented Apr 19, 2020

Tinylog: https://tinylog.org/v2/getting-started/

Because it's very simple to use.

Alterantives:

@Stephen-AI
Copy link
Contributor

I can work on this issue

@marko-radosavljevic
Copy link
Collaborator

Sure! ^^

Few rules tho:

  1. Skip every println() in try-catch blocks. (PR PR for #18 #19 is still WIP, we do not want conflicts with it)
  2. Do not use auto-formatting.

Other than that, yeah, replace every println() with appropriate tinylog equivalent. Generic println() should be tinylog.Logger.info().

@Stephen-AI
Copy link
Contributor

Stephen-AI commented Oct 2, 2020

2. Do not use auto-formatting.

To clarify, what do you mean auto formatting? Do you mean like from the IDE?

@Stephen-AI
Copy link
Contributor

Stephen-AI commented Oct 2, 2020

Sure! ^^

Few rules tho:

1. Skip every `println()` in try-catch blocks. (PR #19 is still WIP, we do not want conflicts with it)

I'm assuming I shouldn't change replace println logs in try-catch blocks either as this would also cause a conflict. For example, Logger.getLogger.log(...).log()

@marko-radosavljevic
Copy link
Collaborator

Yes, IDE auto-formatting.
Correct, but its probably smartest to wait for all the big PR's merge, especially #26. That's for sure the easiest way to avoid conflicts.

@weisJ
Copy link
Contributor

weisJ commented Oct 3, 2020

I think we should use slf4j/jdk-logging with tiny log bindings. Also I’m not a big fan of using the global logger. Classes should use their own logger. This way it’s easier to trace back the origin of message.

@marko-radosavljevic
Copy link
Collaborator

marko-radosavljevic commented Oct 3, 2020

I don't care much about logger choice, but I agree that the classes should use their own.
tinylog output is nice tho:

2020-10-03 19:11:20 [main] VisualLogic.FrameMain.<init>()
INFO: test

Btw, @koppor, I decided to donate 1 Hacktober PR to JabRef! ❤️

@koppor
Copy link
Member Author

koppor commented Oct 3, 2020

@weisJ The recommendation to use TinyLog was that the usage is very Java-beginner friendly:

import org.tinylog.Logger;

public class Application {

    public static void main(String[] args) {
        Logger.info("Hello World!");
    }

}

No need to instantiate the Logger variable. - In other projects, I am mostly using SLF4J. I also saw successful usage of Apache Log4j 2.

My proposal of Tinylog was only to be friendly to guys coming from python (I look at you @Foadsf). I am aware that Java-homies are more SLF4-friends (consistency with other Java-projects).

@MaSven
Copy link
Contributor

MaSven commented Oct 6, 2020

Ok if we can change the logger i would take log4j2. It is by far the best of all loggers. SLF4J was the one used for every project. The API of log4j2 is much more jdk8 friendly e.g. you can do Logger.withException().log(). This avoids the problem slf4j has with throwable and a message. I assumed Tinylog doesn't have instance logging?

@marko-radosavljevic
Copy link
Collaborator

marko-radosavljevic commented Oct 6, 2020

Sure, I'm fine with it. Our decision to go with tinylog wasn't really thought out anyway.

It was dead simple, small size, and performant. But I agree that it's maybe too simple ^^

@koppor
Copy link
Member Author

koppor commented Oct 7, 2020

@MaSven Not sure what you mean by "instance logging". The current method and thread are logged. See at the intro of tiny log: https://tinylog.org/v2/getting-started/

Seeing that are Java professionals here used to SLF4J, I don't want to put cognitive load on them to think about another logging framework, I would also opt for SLF4J. - The issues it this project are at so many other places...

@koppor
Copy link
Member Author

koppor commented Oct 7, 2020

Quote from the docs:

When you run this small application, you will see the following output in the console:

2018-03-31 18:15:32 [main] Application.main() INFO: Hello World! 

@MaSven
Copy link
Contributor

MaSven commented Oct 7, 2020

OK and which logging implementation should be used? Slf4j is only the facade. With instance logging I mean per class logging. I think this is not possible with tinylog. I can't filter out a class for example in the config. Also I think log4j2 has the easier api for beginners. But I can also live with slf4j.

@koppor
Copy link
Member Author

koppor commented Oct 7, 2020

@MaSven You are right. I would in call cases log errors.

https://tinylog.org/v2/configuration/#severity-level and it reads

If required, particular severity levels for packages or classes can be set to override the default severity level for them. Severity levels for packages or classes can only be defined globally, but not on a writer.

Example:

level@org.tinylog = debug

Thus level@io.github.mylibrelab = error should avoid logging trace, info, ... on all io.github.mylibrelab packages.

@MaSven
Copy link
Contributor

MaSven commented Oct 7, 2020

Then we should stick with tinylog it has enough features and we already use it now in many places. The project also looks decent enough and they take performance serious. Just my thought about it.

@weisJ
Copy link
Contributor

weisJ commented Oct 7, 2020

Tinylog has sl4j bindings

@MaSven
Copy link
Contributor

MaSven commented Oct 7, 2020

As I said the slf4j api has many problems. I would suggest to using tinylog directly.

@marko-radosavljevic
Copy link
Collaborator

I love that the logger choice is the most debated topic currently ^^

So I guess we have consensus, we are using tinylog for now. It's a very easy thing to replace anyway.

@MaSven
Copy link
Contributor

MaSven commented Oct 9, 2020

So this is partially solved by #57 . What is missing are proper settings for where to store the logs and also what roation we use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants