k0001 / di

Easy and powerful typeful logging without monad towers, in Haskell.
26 stars 5 forks source link

Add Semigroup instance for StringPath to fix build with GHC 8.4 #19

Closed cocreature closed 6 years ago

cocreature commented 6 years ago

I tested the build with GHC 8.0, 8.2 and 8.4 which are the only versions supported according to the constraint on base.

k0001 commented 6 years ago

Thank you.

k0001 commented 6 years ago

By the way @cocreature, are you using di at the moment? I am doing a big revamp of the library to pursue a different path, and I am wondering if I should do that as an upgrade to di or as a new library.

cocreature commented 6 years ago

@k0001 Thanks for the heads up! I started using di today in a project. Unless you intend to keep developing (as in actively developing, not just bumping bounds to prevent the worst bitrot) both libraries, I’d say making it an upgrade to di is preferable. We already have a ton of logging packages on Hackage and adding one more will only make things more confusing.

That said, I took a short look at the df1 branch assuming that this is where the revamp is taking place and it looks like this will probably result in me looking for a different library (which is totally fine, I’m not at all trying to complain! I just thought a bit of feedback might be helpful). The two primary reasons why I like di are that it doesn’t force me to use some MTL style abstraction and more importantly that it has a very simple yet very flexible API. It looks like both of these things are going away, i.e., there is now a MonadDi typeclass and I can’t use things like custom log level types.

k0001 commented 6 years ago

@cocreature thanks for all the comments!

Yes, I'm working on the df1 branch.

Regarding MonadDi: You will still be able to use Di directly, passing it around, etc. I haven't done that work in that branch yet though, you still need to go through DiT at least.

Regarding custom log levels: I am not sure if keeping these is particularly useful, but I am open to suggestions. Do you have some use case in mind?

Don't look to much at the code in the df1 branch yet, it's still a mess. Much of this effort in the df1 branch has to do introducing a new structured logging format (which I'm calling “df1”) and command line tools for dealing with it. So, it's not so much about the di library itself, and when the time comes I'll probably move all of the df1 related stuff to a different package, and perhaps there I'll restrict log levels and such to a monomorphic type.

k0001 commented 6 years ago

@cocreature by the way, I just uploaded this change and other fix to Hackage. Thanks.

cocreature commented 6 years ago

Regarding MonadDi: You will still be able to use Di directly, passing it around, etc. I haven't done that work in that branch yet though, you still need to go through DiT at least.

Alright sounds good!

Regarding custom log levels: I am not sure if keeping these is particularly useful, but I am open to suggestions. Do you have some use case in mind?

I do have a usecase, namely log levels that are not ordered hierarchically: E.g. let’s say I have three parts A, B and C in my codebase. I’d like to be able to be able to control separately for each part if logs should be shown. So I’d have something like data LogLevel = FromA | FromB | FromC and could then filter messages to a subset of those (in addition to that each part can emit messages of different severity but that’s orthogonal to this problem). Thinking about this I realized that paths would probably be a better fit for this than log levels and the reason why I have been using log levels is that filtering based on the path is not possible atm. So if there was an option to filter based on the path, that would be fine for me!

So, it's not so much about the di library itself, and when the time comes I'll probably move all of the df1 related stuff to a different package, and perhaps there I'll restrict log levels and such to a monomorphic type.

Great! Hopefully this should also allow the removal of some of the deps added in that branch :)

k0001 commented 6 years ago

@cocreature thanks for your feedback!

Yes, I think paths solve the problem nicely. And, actually, the introduction of MonadDi mostly has to do with making your particular kind of situation easier to handle.

Paths are already present in the di version up in Hackage, but it's kind of tricky working with them without (a) monadic vocabulary, and (b) a opinionated recommendation of how to use them for these scenarios. Otherwise, as you just described, people may flock to levels or messages where paths would have been a better approach.

In the df1 branch, we mandate that paths be constructed of segments and attributes. A segment conveys the idea that you are going "deeper" into a different sector of your application, and attributes add metadata to the location from where you are logging stuff. Your parts A, B and C would be best represented by a new segment (named "A", "B" or "C", respectively), and optionally some additional attributes describing how your execution path in A is unique (e.g., if A is handling a request with id 1234, then an attribute saying "request-id=1234" would be appropriate).

Consider this toy example:

main_example :: forall m. (Di.MonadDi m, MonadIO m) => m ()
main_example = Di.push "example" $ Di.attr "version" "123.35" $ do
   Di.notice "Hello, welcome to the example project"
   Di.info "Let's initialize some stuff now"
   yport <- initStuff
   case yport of
      Nothing -> do
         Di.critical "Couldn't initialize. Can't do anything useful. Bye"
      Just port -> do
         Di.push "server" $ do
            Di.attr "port" (fromString (show port)) $ do
               Di.notice "All set. Listening for incoming connections."
               handler "yahoo.com" -- just pretending yahoo hits us
         Di.notice "Program finished successfully, we'll exit now."
 where
   initStuff :: m (Maybe Int)
   initStuff = Di.push "init" $ do
      Di.push "frobizer" $ do
        Di.attr "remote-addr" "google.com" $ do
           Di.notice "Connecting to remote frobizer"
           Di.attr "error" "division by 7" $ do
              Di.error "Could not connect."
           Di.warning "Using fallback in memory frobizer"
      Di.push "wat" $ do
        Di.info "The wat has been setup"
        return (Just 1234)
   handler :: String -> m ()
   handler remoteEnd = Di.push "handler" $ do
      Di.attr "remote-end" (fromString remoteEnd) $ do
         Di.notice "Incoming connection"
         Di.warning "Remote host is trying to hack us!"

When that program runs, things will render like this (or some variation of it, remember, all is in flux):

2018-05-03T14:07:19.975269373Z /example version=123%2e35 NOTICE Hello, welcome to the example project
2018-05-03T14:07:19.975279235Z /example version=123%2e35 INFO Let's initialize some stuff now
2018-05-03T14:07:19.975284892Z /example version=123%2e35 /init /frobizer remote%2daddr=google%2ecom NOTICE Connecting to remote frobizer
2018-05-03T14:07:19.975292413Z /example version=123%2e35 /init /frobizer remote%2daddr=google%2ecom error=division%20by%207 ERROR Could not connect.
2018-05-03T14:07:19.975294487Z /example version=123%2e35 /init /frobizer remote%2daddr=google%2ecom WARNING Using fallback in memory frobizer
2018-05-03T14:07:19.975297270Z /example version=123%2e35 /init /wat INFO The wat has been setup
2018-05-03T14:07:19.975303320Z /example version=123%2e35 /server port=1234 NOTICE All set. Listening for incoming connections.
2018-05-03T14:07:19.975311846Z /example version=123%2e35 /server port=1234 /handler remote%2dend=yahoo%2ecom NOTICE Incoming connection
2018-05-03T14:07:19.975313744Z /example version=123%2e35 /server port=1234 /handler remote%2dend=yahoo%2ecom WARNING Remote host is trying to hack us!
2018-05-03T14:07:19.975315930Z /example version=123%2e35 NOTICE Program finished successfully, we'll exit now.

Doing that thing with di today is already possible if you use something like Path (as it appears in the df1 branch) as your path (not exactly, but sorta), but without a monadic interface it's kind of tricky because instead of saying

push "foo" $ do
  log Debug "yeh"
  attr "bar" "qux" $ do
     log Info "nayy"
  log Warning "back"

You need to say:

\di0 -> do
   let di1 = push (Segment "foo") di0     --  Segment constructor for some `path` monoid
   log di1 Debug "yeh"
   let di2 = push (Attribute "bar" "qux") di1
   log di2 Info "nay"
   log di1 Warning "back"

That's manageable to some extent, but... it's not as nice as the monadic interface if you are going to be a heavy path user. That's why I'm exploring the MonadDi road.

Bonus track: Colors :)

colo

k0001 commented 6 years ago

By the way, mkDiStringHandle shows a small example of using paths https://hackage.haskell.org/package/di-0.3/docs/Di.html#v:mkDiStringHandle

cocreature commented 6 years ago

The problem with paths in the current version is that there is no easy way to filter based on the path. I suppose I could put the filtering directly in the low level logging function but that’s kind of ugly.

k0001 commented 6 years ago

Good point (see #4). I don't remember the exact details now, but filtering on path is currently tricky because mappend on path is performed way too early in the process, so by the time you try to filter that path, it has already been transformed to something else. Perhaps mandating [] as the outer path Monoid, and keeping individual path elements separated for as long as possible is the way to go.

cocreature commented 6 years ago

One last question: Does it make sense to make PRs/issues for the current version of Di that do something more substantial than just fixing the build with a new version of GHC (e.g. attempt to add path-based filtering) or should I wait until you’re done with the revamp?

k0001 commented 6 years ago

@cocreature Oh, please, send PRs :) It's likely that most/all of what you see in the df1 branch now will end up in a different library, so don't worry about it.

k0001 commented 6 years ago

What I'll try to do is probably this: Keep di as it is today as much as possible, and build the df1 stuff on top of it, in a different library.