Define middleware type and make a basic logging middleware

[martyall]

we mentioned defining a middleware type as App m -> App m

We're about to start having to write tests and a small example app, so we should go ahead and put in a basic logger middleware. Something that allows for location, severity, and some loggable fields. It should take LoggingOptions that control the severity / level of info / color?

[martyall]

I assigned this to @charlescrain because he was talking about making use of katip instead of rolling our own whatever, which seems like the better option for getting something up and running.

however, I think this needs to be used in it's own cabal project inside this repo, something like hs-abci-katip-logger or something like this, since people might not want to depend on the katip library. It would depend on hs-abci-server for the middleware type alias

type Middleware = App IO -> App IO

which isn't defined yet, as well as the message types I guess.

[charlescrain]

Great point, @blinky3713 the wai example you gave me for middleware also has the wai-extra library that has a bunch for different middleware available. We could do a similar thing and it could be a place for anyone to add useful middleware modules.

[kejace]

FWIW I found this that seems like a good logging library: http://hackage.haskell.org/package/co-log

[martyall]

katip looks pretty sweet cause it can also push to datadog, which we LOVE

[charlescrain]

It's probably faster for me to use Katip since I'm more familiar with it but I can do whichever.

[kejace]

My apologies I thought this issue was about middle-management not middleware. What am I even doing here.

[charlescrain]

@blinky3713 I've built out some basic scaffolding for the logger. I was planning on making the logger for Requests and modeling it after wai-request-logger.

I began making the instances to log the requests but wanted to ask if you guys are okay with JSON and other necessary orphan instances or if you'd prefer making newtype wrappers.

Here's the branch I've started.

[martyall]

Looks good to me. I think that if we can get around orphans that's best, newtyping sounds good to me.

Also I think we should go ahead and nest hs-abci-server into a directory as well, so that only the stack yaml, license, and various READMEs Makefiles etc are top level. What do you think?

[charlescrain]

Sounds good!

[charlescrain]

After some experimentation, newtypes won't work as the types depend on each other and I'd need to redefine many of the types just to give them these instances.

I think orphans is the best solution, we could put them in an internal file to make it obvious that these instances shouldn't be used outside the lib.

[charlescrain]

@blinky3713 Assuming the decision to use orphans is good, I need some help with GADT stuff so I can pull out the request type for making the ToJSON/FromJSON instances for the actual request type. i.e. data Request (m :: MessageType) :: * where

[martyall]

A few initial comments:

1. Do we need the FromJSON instances?
2. Do we need the ToJSON instances? It seems like you're just using them to get the default implementation for ToObject, so why don't we just skip that step and invoke genericParseJSON in the ToObject instance.
3. Can we pull out the Options for each type into a separate value? It generally makes it easier to see what's going on imo. For example:

defaultABCIOptions :: String -> Options
defaultABCIOptions prefix = 
  defaultOptions & aesonDrop (length prefix) camelCase

...

instance ToObject VoteInfo where
  toObject = genericToJSON voteInfoOptions
    where
      voteInfoOptions :: Options
      voteIntoOptions = defaultABCIOptions "voteInfo"

...

It should be possible to use newtypes to avoid orphans, even if it means being extremely verbose. If you newtyped every Request and Response message type from with some newtype wrapper, something like

-- | Loggable newtype wrapper
newtype L a = L a

instance ToObject (L Request.VoteInfo) where
  toObject (L a) = handDefinedInstance

Obviously we don't want to do that if possible because it sucks, but I guess I don't understand the part about it being impossible because of the types depending on each other. So yeah, I'd rather just put it in a non-exposed module (making the distinction in the package.yaml file)

The other thing we can do here to avoid is work purely with generics. I will write a gist and post it in a comment below.

[charlescrain]

@blinky3713

Ahh boom good point, we can avoid the To/From instances since it's just for ToObject. And yeah I can pull out the Options.

Yeah I didn't mean that it'd be impossible, just that it would require manual implementations for the instances that have nested types.

You mentioned it'd be fine to make it non-exposed modules which would be great.

If we can just write a ToObject instance based on just generics, that'd be awesome. Looking forward to seeing the gist :D

[martyall]

apparently I can't write the function I want to write because of this issue but depending on what happens you can do something like this:

import Data.Aeson (ToJSON(..), Value, Options, GToJSON, ToArgs(NoToArgs), Zero)
import GHC.Generics

class GToLoggableValue (f :: (* -> *)) where
  type GLoggableValue f :: (* -> *)
  gToLoggableValue :: f a -> (GLoggableValue f) a

newtype LF a = LF a

instance GToLoggableValue (K1 i c) where
  type GLoggableValue (K1 i c) = K1 i (LF c)
  gToLoggableValue (K1 a) = K1 (LF a)

instance ( GToLoggableValue f
         , GToLoggableValue g
         ) => GToLoggableValue (f :*: g)  where
  type GLoggableValue (f :*: g) = GLoggableValue f :*: GLoggableValue g
  gToLoggableValue (a :*: b) = gToLoggableValue a :*: gToLoggableValue b

instance (GToLoggableValue f) => GToLoggableValue (M1 i c f) where
  type GLoggableValue (M1 i c f) = M1 i c (GLoggableValue f)
  gToLoggableValue (M1 a) = M1 $ gToLoggableValue a

-- You could use this as proof that the function below is working, because
-- all of the values in the hashmaps would be prefaced by "LF"
instance  Show a => ToJSON (LF a) where
  toJSON (LF a) = toJSON $ "LF " <> show a

-- can't actually write this function because gToJSON is not exposed :/
genericToLoggableValue
  :: ( Generic a
     , Rep a ~ rep
     , GToLoggableValue rep
     , GToJSON Zero (GLoggableValue rep)
     )
  => Options
  -> a
  -> Value
genericToLoggableValue opts = gToJSON opts NoToArgs . toLoggableValue . from

[safareli]

What if we also derive toJSON instances for all our types? what's the problem of depending on aeson or whatever? or why not use show instance which is present for all types?