Add some further logging #358
Conversation
…nd 'trace' level as I find I often have to add such logging in to debug proposed changes
| case Some(message) => | ||
| Sync[F].pure(new JmsMessage(message)) <* Logger[F].debug(s"Received message: $message") | ||
| case None => | ||
| Spawn[F].cede >> Async[F].sleep(pollingInterval) >> Logger[F].trace( |
There was a problem hiding this comment.
My only doubt will be this log.
I mean, if you are not receving nothing, you are polling.. and the pollingInterval is a static configuration. So right now i cant see where this can be helpful, can you give me an example?
also, yeah it's in trace but this could be a very hot path that could lead to a large amounts of logs and noise.
There was a problem hiding this comment.
@faustin0 Sure. Several times whilst trying to make changes to jms4s I have come across the situation where my tests just hang. Retrieving the jstack showed nothing useful, after debugging line by line, I came to realise that this is because the test is trying to read something from either an empty or wrong queue, and so the test will never complete. After fixing the root cause issue, all was fine, however it was impossible to spot with a standard jstack dump due to the nature of cats-effect.
There was a problem hiding this comment.
Ok, got it. However, I don't think this is the right place to put these logs:
- if you want to log every consumed message, I will leave this choice to the user, and the most appropriate place is in the
handlemethod exposed by the various consumers, where you have the same message you would log here. - the logged message will be inconsistent between the different implementations.
- logging all the body(that can be large) can lead to some issues and i prefer leave this choiche to the client instead of the library.
- as for the "polling" logs, yeah maybe they are useful when debugging the libray, but for the users of the library this dont give any useful information so i would remove it and just place a log during development if needed.
This is purposefully done only at 'debug' and 'trace' level as I find I often have to repeatedly add such logging in temporarily in different branches to debug proposed changes that I am trying to make. Having such logging at a log-level which is switched off by default will make it easier for both myself and others to debug issues.