iand675 / hs-opentelemetry

OpenTelemetry support for the Haskell programming language
https://iankduncan.com/projects/opentelemetry
BSD 3-Clause "New" or "Revised" License
73 stars 34 forks source link

inSpan without UnliftIO #76

Open JonathanLorimer opened 1 year ago

JonathanLorimer commented 1 year ago

I am working on an app where the monad doesn't support unliftio. I would like to understand why inSpan requires UnliftIO? It seems like it is just in the bracketError function. It would be great to provide an inSpan function that doesn't require this constraint, but I am not sure where to start.

JonathanLorimer commented 1 year ago

for example, I think this works:

bracketErrorIO :: IO a -> (Maybe SomeException -> a -> IO b) -> (a -> IO c) -> IO c
bracketErrorIO before after thing = EUnsafe.mask $ \restore -> do
  x <- before
  res1 <- EUnsafe.try $ restore $ thing x
  case res1 of
    Left (e1 :: SomeException) -> do
      -- explicitly ignore exceptions from after. We know that
      -- no async exceptions were thrown there, so therefore
      -- the stronger exception must come from thing
      --
      -- https://github.com/fpco/safe-exceptions/issues/2
      _ :: Either SomeException b <-
        EUnsafe.try $ EUnsafe.uninterruptibleMask_ $ after (Just e1) x
      EUnsafe.throwIO e1
    Right y -> do
      _ <- EUnsafe.uninterruptibleMask_ $ after Nothing x
      return y
iand675 commented 1 year ago

This is probably a good overview on the tradeoffs around catching errors for various monads:

https://stackoverflow.com/questions/46425062/should-i-prefer-monadunliftio-or-monadmask-for-bracketting-like-functions

if you know [...] (that you) want compile time guarantees about the correctness of your code vis-a-vis monadic state, use MonadUnliftIO.

This is a particularly important principal for observability tooling– the presence of of tracing should not affect the semantics of the code that's being instrumented. Therefore, we go with the most conservative option for what the library supports by default. If you don't particularly care about maintaining those semantics in your situation, you could use the exceptions-package based implementation provided here: https://github.com/iand675/hs-opentelemetry/tree/main/utils/exceptions

However, there are often ways to make it work for your situation, even with MonadUnliftIO:

Say you're working with ExceptT, for example:

getUser :: MonadIO m => ExceptT HttpResponse m User
getUser = do
  thing1 <- foo
  thing2 <- bar
  thing3 <- baz
  pure $ constructUser thing1 thing2 thing3
  ...
  thing2 <- liftIO $ inSpan "bar" bar
  ...
-- natural transformation
nt :: AppState -> AppM a -> Handler a
nt s x = Handler $ ExceptT $ catch (Right <$> runAppM s x) $ \(e :: ServerError) -> pure $ Left e
JonathanLorimer commented 1 year ago

Oh this is great. I actually ended up implementing exactly the exceptions solution, but I think the natural transformation is the one I want (and I happen to be using servant).

parsonsmatt commented 1 year ago

I do think there's value in providing a class (Monad m) => MonadSpan m where inSpan :: (span arguments) -> m a -> m a, even if that ends up being implemented as something else.