Add more debug logging #828
Conversation
Will log as soon as possible in the operation. Whether the operation actually failed or not should be deduced from other sources.
There is no codepath where the argument to simpleSettings is not isJust. Also use qualified imports. bit more pretty
e.g. we're not logging the log level
arianvp
force-pushed
the
more-debug-logging
branch
from
f7e424a to
3b98bd7
Compare
| @@ -42,6 +42,10 @@ createSettings (zusr ::: tid ::: req ::: _) = do | |||
| assertLegalHoldEnabled tid | |||
|
|
|||
| membs <- Data.teamMembers tid | |||
| let zothers = map (view userId) membs | |||
| Log.debug $ Log.field "targets" (toByteString (show zothers)) | |||
There was a problem hiding this comment.
| Log.debug $ Log.field "targets" (toByteString (show zothers)) | |
| Log.debug $ Log.field "targets" (show $ toByteString <$> zothers) |
There was a problem hiding this comment.
(you're still using show on the Id type, which is fine for debugging, but the ToByteString instance is more concise.)
(speaking of which: why debug? are customers going to want to use this? then it should at least be Info.)
There was a problem hiding this comment.
Don't do show . toByteString, toByteString is sufficient.
Also, how does the output of this line look like with multiple uuids?
There was a problem hiding this comment.
It's fine to log all these details on debug level, but we may have to be more sophisticated than that in the future. Here, ideally we would want something like:
data LogFilter = LogFilter Level UseCase
data UseCase = MaxPrivacy | EnterpriseAuditable@arianvp insists that colog would be a good replacement for tinylog, and I'm tempted... :-)
At least we should check that it's ok for the customers who want this to run with log level Debug for the foreseeable future. If so, we're good. If not, we should elaborate on this PR elsewhere.
| @@ -18,12 +18,19 @@ import qualified Data.ByteString.Lazy.Char8 as L | |||
| import qualified Data.ByteString.Lazy.Builder as B | |||
| import qualified System.Logger.Class as LC | |||
|
|
|||
| -- TODO(arianvp): Get rid of boolean blindness | |||
There was a problem hiding this comment.
These TODOs are removed in the next PR btw
* Allow to set log format The old netstrings option is now deprecated, use logFormat instead. logFormat takes presedence of netstrings if both are set. If none are set, log format defaults to Plain * Implement json renderer for tinylog
* Add json logging and logging (#828) (#836) * Add logs for team operations * Add logging for legalhold * Json logging (#836) * Allow to set log format The old netstrings option is now deprecated, use logFormat instead. logFormat takes presedence of netstrings if both are set. If none are set, log format defaults to Plain * Implement json renderer for tinylog * Specialize the error cases on conversation lookup. (#841) Getting a conversation you left will now give 403 access-denied, not 404 not-found. * Feature Flags in galley options. (#825) * Feature flags in galley options file. * Honour sso feature flag. * Honour legalhold feature flag (test). * Honour legalhold feature flag (handler). * Disable all LH tests if feature is disabled. * Check options settings for feature flag. * Avoid catch-all pattern. Catch-all patterns are not future-proof if constructors for new falvors of "disabled" are added.
This adds debug logs for operations that directly affect a user which are useful to have. when running in info log level, these actions aren't logged.
This is part 1 of a a series of pull requests.
In part 2 i'm planning to add a structured json log format (next to netstr and plan). Which should ease integration of log aggregators like fluent-bit.
I thought it's a good idea to get this part reviewed already as the second part is clearly standalone.