scrive / log

Structured logging solution.
https://hackage.haskell.org/package/log-base
BSD 3-Clause "New" or "Revised" License
35 stars 7 forks source link

Fix issue #4 #10

Closed 23Skidoo closed 8 years ago

23Skidoo commented 8 years ago

This still doesn't completely free the users from the need to think, but should make it harder to get messages silently dropped.

Resolves #4.

arybczak commented 8 years ago

To be fair, if you want to get rid of finalizers then the optimal solution IMO is to rewrite functions that construct loggers to be alloca-like, e.g. withBulkStdoutLogger :: (Logger -> IO r) -> IO r, or withPgLogger :: ConnectionSourceM IO -> (Logger -> IO r) -> IO r. Then you can guarantee that waitForLogger is always run when Logger goes out of scope. This won't prevent anyone from using Logger that was already finalized by returning it outside the scope in which it's valid, but that's fine as you can do the same with alloca, so simply mentioning this in the documentation should be sufficient.

23Skidoo commented 8 years ago

@arybczak Thanks, that's a good idea, I'll implement it.

This won't prevent anyone from using Logger that was already finalized by returning it outside the scope in which it's valid

This can sort of be prevented by marking the logger as closed and raising an exception when it's used outside of scope. There will still be the issue of main thread exiting before the child threads, but that's just something clients need to be aware of.

23Skidoo commented 8 years ago

Implemented new logger creation API as suggested by @arybczak, but not logger invalidation (yet).

23Skidoo commented 8 years ago

Logger invalidation now implemented.

jonathanjouty commented 8 years ago

Followup with ElasticSearch logging

jonathanjouty commented 8 years ago

Followup with ElasticSearch logging

jonathanjouty commented 8 years ago

I've had a quick look and Travis CI is happy, so I'm not going to dig deeper right now.

@arybczak did you want to review changes? If not, @23Skidoo feel free to merge.

23Skidoo commented 8 years ago

I think I'll merge this now and address @arybczak's potential future comments later if/when he decides to review.