Improve logging #2558

eddiemundo · 2022-01-01T21:43:56Z

This PR tries to implement the contravariant logging/tracing library described by #2492 (comment)

Currently the most straightforward way of contravariant logging that this PR does:

If module has no Log datatype then add one
Invent a constructor (name) for the Log datatype taking the values you want to log
Log using it, e.g. logWith recorder Warning $ LogConstructorName value1 value2
If a function needs a logger you pass a logger to it, but if this function is in another module you contramap to the Log type of that module e.g.

-- Shake.hs
data Log = LogStuff

function :: Recorder (WithPriority Log) -> IO Thing

-- MyModule.hs
import qualified Shake

data Log = LogShake Shake.Log

myFunc :: Recorder (WithPriority Log) -> IO Thing
myFunc recorder = Shake.function (cmapWithPrio LogShake recorder)

cont. This means each logged value when fully constructed is a sequence of constructor wrappers of different modules
Log datatypes.
e.g.

WithPriority Info (MainModule.LogAnotherModule (AnotherModule.LogMyModule (MyModule.LogShake (Shake.LogStuff))))

Currently each module is able to convert its Log value to Docs by a Pretty Log instance defined in it. The final log value is converted to a Doc by that module's Pretty Log instance which delegates to other Pretty Log instances if necessary.
We don't convert Log values to priorities. Instead they are specified at the log call site (like how every other log framework does it), and stored alongside the log value using WithPriority a. The "first/final" logger is a Recorder (WithPriority (Doc ann)). This allows us to more easily alter priorities when contramapping the logger which is more flexible than converting a log value to a priority.

Cons

Logging is not very casual. If you want to log random messages you can add a LogOther Text constructor to the Log type of the current module, but if you are logging "for real" then you invent a constructor that takes the values you want to log, and then add to the Pretty Log instance (add a pattern matching branch) to convert the Log to a Doc
Logging in a plugin means also you need to pass a logger to the plugin descriptor function which means in exe/Plugins.hs, a Log constructor should be added to the existing Log datatype for the plugin if it doesn't exist, and a contramapped recorder should be passed to the plugin's descriptor function. Additionally, you need to add entries for the pretty pattern match. This pattern matching needs CPP like everything else in that module. This should be simple to do by following the existing example, but still an extra step.
Contravariant logging composes functions to build new loggers, so there isn't an easy way of taking a logger and decomposing it to get the base version of the logger (to say, remove a priority filter).
Requires understanding contramap

Pros

The logger can do pretty much anything depending on how its logging function is defined. So this means we can inspect log values in tests, convert log values to LSP messages, text, structured logging, etc.
Conversion to backend values is more cohesive because it's done in one place per module.
The source is less cluttered with formatting details, value conversions, checking of test flags, but sometimes logging text can serve as comments
Can do more fine-grained filtering, like filtering specific log messages based on their values, but that is often not ergonomic. You can also do filtering on the caller side by contramapping loggers before passing them on.

Questions (for the future):

To implement other backends like LSP messages, there seem to be multiple ways of doing it:
1. The Duncan Coutts video way which is to bundle another logger that sends LSP messages with the usual console/file logger. Then at the logging call site you can decide to use which logger depending on which backend you want to send to. This seems like the simplest way.
2. Encode whether the message should go to the LSP backend or console/file backend in the Log data type. In that case the single logger decides how to interpret the message later.
3. Have a separate log datatype per backend.
There is boilerplate when it comes to converting log values to backend values, and when constructing Log values. Maybe generic programming or TH can be useful here.

Todo:

make test utils compile
allow both previous and contravariant style to write same log file
convert log values to match previous text
convert log values to match previous priorities
the final log type should probably be WithPriority (Doc a) instead of WithPriority Text since we're using prettyprinter
fix test loggers to use pretty printer and logToPriority function
delete log comments that helped me remember what the original logging did
ghcide exe default logger doesn't use hslogger and prints to stderr, so match that behaviour (noticeable in CI test logs)
provide log priorities at log call sites instead of separate function

Todo in other PRs:

convert all instances of logInfo, logDebug, etc, to logWith
some kind of generics to reduce boilerplate?
lsp message backend, convert all notifyTesting calls
implement/use tracing functionality where appropriate
eliminate all places where exceptions can be swallowed
add more logging for everything we care about and all the extra stuff @jneira was saying in the original issue.
write docs explaining how to use contravariant logger / what it can do

ghcide/src/Development/IDE/Types/Logger.hs

pepeiborra · 2022-01-01T23:11:34Z

This is an example of a custom logger used in an in-process test:

 where
    -- a logger that intercepts and delays Buck targets fetching for 20s
    -- to ensure that the test is not sensitive to timing issues
    delayBuckInit =
      mempty
        { traceMsg_ = \msg act -> case msg of
            LogKnownTargets (KnownTargets.LogBuck Buck.Log {}) -> do
              liftIO $ sleep 20
              act
            _ -> act
        }

pepeiborra · 2022-01-01T23:12:49Z

This is an example of a test that parses (strongly typed) log messages to prove invariants:

   let eitherFormatOrInit =
          asum
            [ Left
                <$> satisfyLog
                  ( \case
                      KnownTargets.LogGetSigmaTargetsResult {} -> pure ()
                      _ -> empty
                  )
            , Right <$> responseForId STextDocumentFormatting rid
            ]
    res <- skipManyTill anyMessage eitherFormatOrInit
   ...

…ded log file to see it side by side with original logging

…server into improve-logging2

pepeiborra · 2022-01-06T21:42:40Z

Just wanted to say that this is going in the right direction and very impressively so, in case you still had any doubts.

And a suggestion - instead of a Big Bang, try to land it incrementally. Don't make any more changes, polish what you have and land it before converting any more modules and plugins.

… to same log file

michaelpj · 2022-01-08T12:48:58Z

Converting these log values to text/priorities/other is done separately and maybe in a different location. I think the idea is that each entry point will have a table converting Logs to priorities, and/or text, and or other things. Or some other centralized place.

I think this may become unwieldy.

Perhaps we could provide Pretty instances for Log types? That at least gives a straightforward route to "normal" logging.
I'm not sure what to do with priorities. It does seem that within a module we usually have information about the relative priorities of logs at least. Perhaps we could always log with WithPriority, and then allow callers to rearrange the priorities if they care (e.g. "demote all info messages from plugins to debug" would be fairly doable, I think)

eddiemundo · 2022-01-08T22:33:08Z

Yeah I was thinking with the various Main modules, concentrating logging stuff in each them it would be messy. If instead we have some concentration point before them that they import it would still bit weird because this concentration point would have to also contain any Log types for the Main modules themselves used to have or there would be circular imports, another strategy is we can also just not log things in Main modules but that is a weird restriction, and hard to realize I think.

So like you said the easiest thing would to have each module know how to convert its Log into a final log type which currently is WithPriority Text. So Pretty instances would work because currently I'm using Show and that's pretty convenient, or we could just have a function per module that converts log values to Doc.
For the priorities we could also have a logToPriority function in each module to handle priorities. I don't think that is too bad.

However, in the Duncan Coutts video, the impression I got was they already have a log value data type that is independent of the module structure. If that's the case then centralization is a lot easier. That's technically possible... I just look at every module that logs stuff and gather all the logging types in one place. It would still mimic the module structure but not literally needing to import each module aliased. It's also more flexible because well a module doesn't actually have to use a Log type specific to the module its in. The disadvantage is rearranging things to avoid circular imports, maybe a lot.

exe/Plugins.hs

…er and hls-plugin-api dependencies

…have default logger be mutex stderr or file handle, use stderr if failing to open log file

…server into improve-logging2

berberman · 2022-02-05T19:30:22Z

Can we re-export prettyprinter stuffs in Development.IDE.Types.Logger, so that downstream packages don't have to explicitly depend on this library?

eddiemundo · 2022-02-05T19:32:53Z

Can we re-export prettyprinter stuffs in Development.IDE.Types.Logger, so that downstream packages don't have to explicitly depend on this library?

Yeah that sounds like a good idea. Gonna do that.

pepeiborra

Reading the PR description, this is pretty much what Sigma IDE does as described in #2492. Approving early as I don't want to hold back the PR leading to inevitable merge conflicts.

For future work, we should have a logging backend that forwards some LSP messages (encoded as JSON values) to the LSP stream for testing invariants.

pepeiborra · 2022-02-05T20:58:47Z

ghcide/src/Development/IDE/Main.hs

-defaultArguments :: Priority -> Arguments
-defaultArguments priority = Arguments
+defaultArguments :: Recorder (WithPriority Log) -> Logger -> Arguments
+defaultArguments recorder logger = Arguments


Did you consider adding an argsRecorder field to store the recorder argument?

I thought about it briefly but wasn't sure how it would play out. The recorder would be for the Log type at the point it gets passed to Argument, and then it gets put into a ShakeExtras. When we want to use it we'd take it out and contramap it right there? I think that would mean each module would need to import the Log of the recorder when it was put into the Argument or ShakeExtras. It seemed messy but maybe my mental simulation was just wrong.

Other ways is that existential-ish typeclass method?

pepeiborra · 2022-02-05T21:01:33Z

ghcide/src/Development/IDE/Main.hs

        { argsProjectRoot = Nothing
        , argsOTMemoryProfiling = False
        , argCommand = LSP
-        , argsLogger = stderrLogger priority
-        , argsRules = mainRule def >> action kick
+        , argsLogger = pure logger


What is the old Logger type used for now? If it is still being used, can it be subsumed with a LogLegacy Priority recorder?

The old logger type is still being used, it's just being initialized at various entry points and passed in.

Not exactly sure what the LogLegacy Priority means, but the original plan was to remove argsLogger completely eventually.

pepeiborra · 2022-02-05T21:08:07Z

ghcide/src/Development/IDE/Types/Logger.hs

+import qualified System.Log.Formatter       as HSL
+import qualified System.Log.Handler         as HSL
+import qualified System.Log.Handler.Simple  as HSL
+import qualified System.Log.Logger          as HsLogger


This is a significant change in behaviour. Switching to HsLogger seems unrelated to the contravariant logging changes, and better left for another PR

The only reason for the hslogger stuff is so we can still see them in the logs because hie-bios and a few other places in the code base haven't been converted to the contravariant logging system.

I don't follow this. Are you saying that the old stderr logger implementation didn't work with the contravariant system?

Although now that I look, it seems the only place in hls that uses hslogger is the Hlint plugin, but I'm pretty sure I need the hslogger stuff to get the hiebios output for log files. The reason to copypaste the hslogger stuff from lsp was so the outputs wouldn't interleave improperly when we write to the handle.

But the old stderr logger was already using locking to prevent undesired interleaving. I'm not keen on adding a new dependency in ghcide, specially as @michaelpj is planning to drop the hslogger dependency in lsp

Oh OK didn't see the newer message.

Previously HLS used both hslogger, and a plain stderr logger depending on the entry point. If ghcide process was run it would use stderr logger, and otherwise I think it would overwrite the default stderr logger with the hslogger logger.
Additionally hie-bios uses hslogger to write compilation logs to output. The hslogger was initialized by calling setupLogger from lsp. If I kept using the setupLogger from lsp then the logs would get interleaved weirdly because they don't have access to the lock, and so I decided to copy-paste that code with some modifications so we could still get any logs written to some handle using hslogger, but also not have them displayed weirdly.

Maybe the confusion is thinking that we're using hslogger as the console/file backend. We're not... we're using just hslogger to setup a hslogger so we can pick up hie-bios logs to the right handle, and give it a lock so it doesn't do funny things with messages we write to the actual handle backend.

So, if I understand your analysis right, the hie-bios logs were actually broken for ghcide, since hie-bios unconditionally uses hslogger, and the ghcide entry point wasn't setting it up? That would also be true for the logs from lsp, which currently use hslogger (although I'm getting rid of it).

I'm okay with putting this in there as a temporary measure until we can get rid of hslogger in our dependencies. We should just clearly explain why we're doing it.

Yes, I'll add a better comment.

… on prettyprinter and hslogger

isovector

idk what's going on but lgtm!

…w Show instances

michaelpj · 2022-02-06T11:35:29Z

This forces any conversion of the final Log value to backend value (Doc, or some lsp message, or something else) to torturously delegate to other modules conversion functions.

This isn't so bad, right? Typically this means writing a Pretty instance and then calling pretty on the sub-logs. So in particular it's not like you need to remember lots of conversion functions.

The advantage of building Log values such that they have a module constructor chain is to be able to consider the module chain information when doing the logging action, but I can't see that as being really too useful.

I think the advantage is rather locality of reasoning: you can see in that module what logs it emits and they're just there. We could imagine instead having a GlobalLog type that had one alternative for every log in the entire codebase. Then we would only have to do one conversion, but everything would depend on this type, and all the logs for each module would be mixed up together. So I think it's a maintainability thing rather than that it provides useful runtime information.

This makes me think that there are 3 (?) reasonable points on the spectrum:

Per-module Log types as now, embedded into each other.
Per-package Log types, with everything in that package. Still need to be embedded into each other e.g. to get plugin logs embedded into the HLS logs.
Truly global Log type, by picking a final representation type and just using that, e.g. Doc.

I think 3 would actually be quite annoying if we ever do want to emit a different type. The most practical example I can think of is providing a --json-log option to HLS that would emit all logs as JSON objects to make them easier to analyze. There it might actually be nice to go via the ToJSON instance of the log objects rather than having to post-process a Doc.

Maybe we should consider 2 instead? It's a little ugly, but given the number of packages we have, perhaps things still get scoped somewhat reasonably?

michaelpj

I think this is great, and we should merge it soon and iterate from there.

michaelpj · 2022-02-06T12:12:20Z

ghcide/src/Development/IDE/Types/Logger.hs

+cmapWithPrio :: (a -> b) -> Recorder (WithPriority b) -> Recorder (WithPriority a)
+cmapWithPrio f = cmap (fmap f)
+
+cmapIO :: (a -> IO b) -> Recorder b -> Recorder a


I'd probably call this cmapM (to match co-log), with the implicit constraint that we fix m ~ IO in our recorder. This is also fine though.

I think in order to get cmapM working with the rank N type of logger_ in Recorder I'd need an UnliftIO or MonadBaseControl constraint, and I remember having some issue there too.

I'm not sure if the rank N type of the logger_ field is worth, but it does save carrying around a type parameter. Without the logger_ having rank N type then using co-log directly would probably be better, although Pepe's initial example had a tracer_ field bundled in which would be different from co-log.

ghcide/src/Development/IDE/Types/Logger.hs

michaelpj · 2022-02-06T12:16:17Z

ghcide/src/Development/IDE/Types/Logger.hs

+  { logger_ :: forall m. (HasCallStack, MonadIO m) => msg -> m () }
+
+logWith :: (HasCallStack, MonadIO m) => Recorder (WithPriority msg) -> Priority -> msg -> m ()
+logWith recorder priority msg = withFrozenCallStack $ logger_ recorder (WithPriority priority msg)


If you switch the order of the constructors of WithPriority then you could just make this

logWith :: ... => Recorder msg -> msg -> m ()

and have the call sites do this

logWith logger $ "hello" `WithPriority` Info

which is a bit more verbose but kind of cute :) Probably this is better, given this gets called a lot.

It is cooler to have logWith not know anything about Priority. I think in the end going back to

logDebug logger log = logWith logger $ WithPriority Debug log

will be better since I expect everything to use priorities/severities

michaelpj · 2022-02-06T12:18:48Z

ghcide/test/exe/HieDbRetry.hs

+  = LogSession Session.Log
+  deriving Show
+
+makeLogger :: Var [Log] -> Recorder (WithPriority Log)


Probably not for this PR to keep it minimal, but there's a nice generic version of this backed by a TQueue or something, so you could have

mkTQueueLogger :: IO (Recorder msg, TQueue msg)

or something.

That's more convenient. It never occurred to me that the recorder is kind of like the in pipe so you can create them both at same time.

ghcide/test/exe/Main.hs

michaelpj · 2022-02-06T18:22:40Z

Ignore my previous message, it was in fact just wrong! 🙈

ghcide/src/Development/IDE/Types/Logger.hs

pepeiborra · 2022-02-06T19:35:57Z

This makes me think that there are 3 (?) reasonable points on the spectrum:

Per-module Log types as now, embedded into each other.

Per-package Log types, with everything in that package. Still need to be embedded into each other e.g. to get plugin logs embedded into the HLS logs.

Truly global Log type, by picking a final representation type and just using that, e.g. Doc.

Option 3 defeats the point of contra variant logging, doesn't it? You can no longer use the trick to test invariants as in #2558 (comment)

This started as an attempt to bubble up errors from the VFS as actual errors and return them to the user via the LSP response. However, in fact VFS operations occur in response to notifications, which don't have responses. So all we can do is log the error and drop the change, which is okay. However, that made me look at how the logging works. At the moment we use `hslogger`, which is fine, but isn't so great when it's plugging into part of a larger system. For example, we might want to have a global log handler that sends error-level logs to the client as messages, or uses the `logMessage` method of the LSP spec. But there's no way to intercept the messages sent by the VFS currently. So I switched over to using `co-log-core`, which is also the direction that [HLS is going](haskell/haskell-language-server#2558). `co-log-core` is also a lightweight dependency. It's suboptimal for `lsp-types` to depend on a logging library, however, but that should be fixed when we do haskell#394.

…explaining hslogger setup existence

michaelpj · 2022-02-19T11:29:05Z

👋 I'm returning to the lsp part of this work this weekend. I can wire it up to the existing logging, I think, but I'm still excited to get this in!

This started as an attempt to bubble up errors from the VFS as actual errors and return them to the user via the LSP response. However, in fact VFS operations occur in response to notifications, which don't have responses. So all we can do is log the error and drop the change, which is okay. However, that made me look at how the logging works. At the moment we use `hslogger`, which is fine, but isn't so great when it's plugging into part of a larger system. For example, we might want to have a global log handler that sends error-level logs to the client as messages, or uses the `logMessage` method of the LSP spec. But there's no way to intercept the messages sent by the VFS currently. So I switched over to using `co-log-core`, which is also the direction that [HLS is going](haskell/haskell-language-server#2558). `co-log-core` is also a lightweight dependency. It's suboptimal for `lsp-types` to depend on a logging library, however, but that should be fixed when we do haskell#394.

eddiemundo · 2022-02-20T00:17:54Z

Fixed conflicts and if tests are good then I think this can be merged then can iterate

michaelpj · 2022-02-20T17:15:57Z

Failure is just some warnings from building with -Werror.

michaelpj · 2022-02-20T21:50:24Z

Okay, let's do this.

Reading back, I see this from @pepeiborra

For future work, we should have a logging backend that forwards some LSP messages (encoded as JSON values) to the LSP stream for testing invariants.

I'm not sure I quite understood this, could you make an issue so we don't forget?

* convert to contravariant logging style part 1, uses additional hardcoded log file to see it side by side with original logging * convert Session to contravariant logging style * convert Plugin/HLS and FireStore to contravariant logging style * convert Rules (and most of the universe) to contravariant logging style * fix tests, allow old style logging and contravariant logging to write to same log file * fix import inside wrong CPP * add CPP for LogTactic constructor * remove redundant import * fix ghcide tests * remove unused import * fix plugin tests * LSP_TEST_STDERR should apply to contra logger as well * fix tactic plugin test * use CPP for Log datatype plugin constructors, remove unused imports * add a few Pretty instances, add prettyprinter to haskell-language-sever and hls-plugin-api dependencies * add Pretty Log instances for Session, FileStore, Notifications * add remaining Pretty Log instances * add logToPriorities * fix slight interleaving issue with hslogger and logger both logging, have default logger be mutex stderr or file handle, use stderr if failing to open log file * forgot to add .cabal files with hslogger dep * dont use UnliftIO file IO helpers because they are too new * remove log helper comments, use Doc instead of Text as final console/file logger input, renaming, export Log constructors * remove accidentally added useless file, removed prettyprinter dep from hls-plugin-api because stack ghc8.6.5 doesnt have it? * use deprecated prettyprint modules import for the sake of circleci ghc-8.6.5 * use dummy stderr logger for plugin cli commands, use priorityToHsLoggerPriority function instead of manual mapping * remove old plugin detritus that somehow got committed * fix prettyprinter imports for 8.6.5 * try enforcing prettyprinter bounds? * enforcing bound makes no sense * maybe changing stack yamls does trick * filter out warnings when their diags are empty to more closely match original * add ability to select wanted logging columns, match prev ghcide exe logging behaviour * dont log anything when diags are empty in some defineEarlyCutoff versions * use non-deprecated prettyprinter imports * fix ghcide test module * change logWith to accept priority at call site, remove all logToPriority functions, add cmapWithPrio that contramaps through WithPriority * remove useless hiding import list, add comments to default recorder makers * make cradleToOptsAndLibDir take concrete cradle to remove existential type var in Log constructor * Types.Logger now re-exports prettyprinter, remove unused dependencies on prettyprinter and hslogger * existential type var to remove boilerplate in Plugins.hs, remove a few Show instances * add SourceLoc logging column, inline logToDoc functions, add comment explaining hslogger setup existence * qualify a name to match original source * fix -WError

* First go * Match against specific error message. * Basic Change Type Signature implementation. This implementation only matches a single GHC Error message: ``` • Couldn't match type ‘Int’ with ‘Data.HashSet.Internal.HashSet Int’ Expected type: Int -> Int Actual type: Data.HashSet.Internal.HashSet Int -> Int • In the expression: head . toList In an equation for ‘test’: test = head . toList ``` Specifically on `Expected type: ...`, `Actual type:...` and `In an equation ...`. There are plenty of error messages that match this format but aren't actually valid. * GHC 9.2.0 compat change * Lift expectedError message into a separate binding * Move ChangeTypeAction into it's own plugin * Add New Error Message parsing. - Add new regex for matching extra errors message types - Revamp original regex to match more. - Add basic test suite. - Begin adding `tidyActualType` semantics to provide slightly prettier TyVars * Added Error Message Validation to ignore bad Messages. - Add Pretty Printing for Types - Added a few test scenarios * Miscellaneous Cleanup. * Update Tide Type Signature logic. - Be able to tidy signatures with operators in it - Use T.words instead of regex matching to split tyVars * Remove locA (defaults to id in 8.10) to satisfy 9.0+ * Touch up 9.2.1 * Clean up review notes * Update stack.yamls * Fix copy-paste error * Fix Local Signature resolution * Improve logging (#2558) * convert to contravariant logging style part 1, uses additional hardcoded log file to see it side by side with original logging * convert Session to contravariant logging style * convert Plugin/HLS and FireStore to contravariant logging style * convert Rules (and most of the universe) to contravariant logging style * fix tests, allow old style logging and contravariant logging to write to same log file * fix import inside wrong CPP * add CPP for LogTactic constructor * remove redundant import * fix ghcide tests * remove unused import * fix plugin tests * LSP_TEST_STDERR should apply to contra logger as well * fix tactic plugin test * use CPP for Log datatype plugin constructors, remove unused imports * add a few Pretty instances, add prettyprinter to haskell-language-sever and hls-plugin-api dependencies * add Pretty Log instances for Session, FileStore, Notifications * add remaining Pretty Log instances * add logToPriorities * fix slight interleaving issue with hslogger and logger both logging, have default logger be mutex stderr or file handle, use stderr if failing to open log file * forgot to add .cabal files with hslogger dep * dont use UnliftIO file IO helpers because they are too new * remove log helper comments, use Doc instead of Text as final console/file logger input, renaming, export Log constructors * remove accidentally added useless file, removed prettyprinter dep from hls-plugin-api because stack ghc8.6.5 doesnt have it? * use deprecated prettyprint modules import for the sake of circleci ghc-8.6.5 * use dummy stderr logger for plugin cli commands, use priorityToHsLoggerPriority function instead of manual mapping * remove old plugin detritus that somehow got committed * fix prettyprinter imports for 8.6.5 * try enforcing prettyprinter bounds? * enforcing bound makes no sense * maybe changing stack yamls does trick * filter out warnings when their diags are empty to more closely match original * add ability to select wanted logging columns, match prev ghcide exe logging behaviour * dont log anything when diags are empty in some defineEarlyCutoff versions * use non-deprecated prettyprinter imports * fix ghcide test module * change logWith to accept priority at call site, remove all logToPriority functions, add cmapWithPrio that contramaps through WithPriority * remove useless hiding import list, add comments to default recorder makers * make cradleToOptsAndLibDir take concrete cradle to remove existential type var in Log constructor * Types.Logger now re-exports prettyprinter, remove unused dependencies on prettyprinter and hslogger * existential type var to remove boilerplate in Plugins.hs, remove a few Show instances * add SourceLoc logging column, inline logToDoc functions, add comment explaining hslogger setup existence * qualify a name to match original source * fix -WError * Delete the Telemetry log level (#2727) It's a bit non-standard, and moreover it's entirely dead. * Wall and 9.2 fix * Remove unneeded comments/code Co-authored-by: J. S <[email protected]> Co-authored-by: Michael Peyton Jones <[email protected]> Co-authored-by: Pepe Iborra <[email protected]>

pepeiborra reviewed Jan 1, 2022

View reviewed changes

ghcide/src/Development/IDE/Types/Logger.hs Outdated Show resolved Hide resolved

ghcide/src/Development/IDE/Types/Logger.hs Outdated Show resolved Hide resolved

convert to contravariant logging style part 1, uses additional hardco…

671211e

…ded log file to see it side by side with original logging

eddiemundo force-pushed the improve-logging branch from 84afa80 to 671211e Compare January 6, 2022 04:22

eddiemundo added 4 commits January 5, 2022 23:24

Merge branch 'master' of https://github.com/haskell/haskell-language-…

acc1ef4

…server into improve-logging2

convert Session to contravariant logging style

bb2c6a2

convert Plugin/HLS and FireStore to contravariant logging style

235b87d

convert Rules (and most of the universe) to contravariant logging style

b4ebdd7

eddiemundo added 9 commits January 7, 2022 18:21

fix tests, allow old style logging and contravariant logging to write…

c0190d7

… to same log file

fix import inside wrong CPP

9946830

add CPP for LogTactic constructor

20a3761

remove redundant import

528439c

fix ghcide tests

e69839d

remove unused import

e6b4bd7

fix plugin tests

fb5033a

LSP_TEST_STDERR should apply to contra logger as well

4cfa737

fix tactic plugin test

c874657

pepeiborra requested changes Jan 9, 2022

View reviewed changes

exe/Plugins.hs Outdated Show resolved Hide resolved

eddiemundo added 8 commits January 9, 2022 13:52

use CPP for Log datatype plugin constructors, remove unused imports

2b3b3b3

add a few Pretty instances, add prettyprinter to haskell-language-sev…

23a57c2

…er and hls-plugin-api dependencies

add Pretty Log instances for Session, FileStore, Notifications

504a3df

add remaining Pretty Log instances

17937d2

add logToPriorities

a2fe371

fix slight interleaving issue with hslogger and logger both logging, …

5d879b0

…have default logger be mutex stderr or file handle, use stderr if failing to open log file

Merge branch 'master' of https://github.com/haskell/haskell-language-…

9178f1c

…server into improve-logging2

forgot to add .cabal files with hslogger dep

e94fd03

pepeiborra approved these changes Feb 5, 2022

View reviewed changes

pepeiborra reviewed Feb 5, 2022

View reviewed changes

Types.Logger now re-exports prettyprinter, remove unused dependencies…

cc7dd2b

… on prettyprinter and hslogger

This comment was marked as resolved.

Sign in to view

isovector approved these changes Feb 6, 2022

View reviewed changes

existential type var to remove boilerplate in Plugins.hs, remove a fe…

88ae7e2

…w Show instances

This comment was marked as outdated.

Sign in to view

michaelpj approved these changes Feb 6, 2022

View reviewed changes

This comment was marked as resolved.

Sign in to view

pepeiborra reviewed Feb 6, 2022

View reviewed changes

ghcide/src/Development/IDE/Types/Logger.hs Outdated Show resolved Hide resolved

add SourceLoc logging column, inline logToDoc functions, add comment …

1fd42ae

…explaining hslogger setup existence

eddiemundo added 2 commits February 19, 2022 19:10

fix conflicts

5bb4fcf

qualify a name to match original source

c1f1f3d

fix -WError

3d71348

michaelpj mentioned this pull request Feb 20, 2022

Fix and improve server logging #2492

Open

michaelpj merged commit 3b687a5 into haskell:master Feb 20, 2022

eddiemundo mentioned this pull request Feb 21, 2022

A logging backend that sends some logs to client #2729

Closed

Improve logging #2558

Improve logging #2558

Conversation

eddiemundo commented Jan 1, 2022 • edited Loading

Cons

Pros

Questions (for the future):

pepeiborra commented Jan 1, 2022

pepeiborra commented Jan 1, 2022

pepeiborra commented Jan 6, 2022

michaelpj commented Jan 8, 2022

eddiemundo commented Jan 8, 2022

berberman commented Feb 5, 2022

eddiemundo commented Feb 5, 2022

pepeiborra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddiemundo Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddiemundo Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

isovector left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

michaelpj commented Feb 6, 2022

michaelpj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelpj commented Feb 6, 2022

This comment was marked as resolved.

pepeiborra commented Feb 6, 2022 • edited Loading

michaelpj commented Feb 19, 2022

eddiemundo commented Feb 20, 2022

michaelpj commented Feb 20, 2022

michaelpj commented Feb 20, 2022

eddiemundo commented Jan 1, 2022 •

edited

Loading

eddiemundo Feb 5, 2022 •

edited

Loading

eddiemundo Feb 5, 2022 •

edited

Loading

pepeiborra commented Feb 6, 2022 •

edited

Loading