hazelcast / hazelcast

Hazelcast is a unified real-time data platform combining stream processing with a fast data store, allowing customers to act instantly on data-in-motion for real-time insights.
https://www.hazelcast.com
Other
5.99k stars 1.81k forks source link

Implement own default logger #16640

Open cangencer opened 4 years ago

cangencer commented 4 years ago

Hazelcast currently ships with java.util.logging as the default. Besides badly formatted default output, it also makes synchronized calls for each log message which incurs some performance cost and might lead to unexpected behaviour (i.e. hiding data races, etc)

KaairaGupta commented 4 years ago

I would like to work on this issue.

@mmedenjak I am currently trying to understand where the changes need to be made and also, how would we want to solve this issue.

I noticed that Logger.getLogger() and ILogger are being used in the whole project to register logs, so that would be a good start to understand where I need to make changes to solve this issue. Am I right?

Also, what would be a good practice to handle the formatting issue?

mmedenjak commented 4 years ago

Hi @KaairaGupta !

Sorry, I don't have any ideas at the moment, maybe @cangencer has some suggestions.

KaairaGupta commented 4 years ago

@mmedenjak @cangencer If we will log asynchronously, then possibly logs will not print in correct order, and thus will remain of no use for debugging.

Have I understood the task wrong?

cangencer commented 4 years ago

@KaairaGupta LoggingServiceImpl is a good place to start. That's where the logger is created.

You can't rely on ordering of log messages in a distributed/multi-threaded system. It should preserve order within the same thread, though. Have a look at https://logging.apache.org/log4j/2.x/manual/async.html

KaairaGupta commented 4 years ago

Sorry, for the late reply. I could not work on this issue till now, but will start now. Give me a day to understand the code-base and process this recourse before coming up with a solution.

alimalek71 commented 3 years ago

LoggingServiceImpl is a good place to start. That's where the logger is created.

Have a look at https://logging.apache.org/log4j/2.x/manual/async.html

I read the codes and the document about log4j2 async logging It seems it just needs to add one dependency (com.lmax.disruptor) which is needed for log4j2 async working and the other things we should do by this document is to provide a log4j2 configuration file.

@cangencer Am I correct?

mmedenjak commented 2 years ago

@fbarotov @frant-hartm not really sure, but I know we did some updates in 5.0, did we switch to slf4j2 or something else, or are we still on JUL?

fbarotov commented 2 years ago

afaik, jdk logging is still the default on Hazelcast, but on package and zip / tar distributions it is overridden to use slf4j2 / log4j2 when instances are created through (hz, hz-start) commands

frant-hartm commented 2 years ago

What @fbarotov says is correct:

Imho it doesn't make sense to implement own logger, anyone embedding us will want us to plug into their existing logging infrastructure. Maybe we could print a warning if jul is used and provide guidance to use a better logging framework, or even default to no op logger.

mmedenjak commented 2 years ago

Ok, so I guess there's still some improvements to be done on our side.

there is no such thing as slf4j2

Oh no, my ignorance has been exposed! 😄

edysli commented 1 year ago

Would you (project maintainers) like to completely move away from java.util.logging and replace it with SLF4J? In this case, the whole com.hazelcast.logging package can be deleted. A comment in com.hazelcast.logging.ILogger suggests otherwise though:

The Hazelcast logging interface. It exists because Hazelcast doesn't want any dependencies on concrete logging frameworks, so it creates its own meta logging framework behind which existing frameworks can be placed.

So is a dependency on SLF4J acceptable here?

frant-hartm commented 1 year ago

We don't have a move to slf4j on the roadmap. We don't see much maintenance burden with our current custom logging interface.

edysli commented 1 year ago

Thanks for your reply. I suggest closing this issue then.