weissi / swift-server-logging-api-proposal

SSWG Proposal for Logging API
9 stars 8 forks source link

remove error parameter from trace, debug & info #19

Closed weissi closed 5 years ago

weissi commented 5 years ago

CC @tomerd

ktoso commented 5 years ago

(also: very good to allow attaching error in warning level, was a mistake we had in some apis in the past)

weissi commented 5 years ago

@tomerd I'd be in favour of not having it everywhere because if people really want to, they can create that through an extension, using the logger.log method which does take an error.

But I'm not super strong here. @ktoso ?

ktoso commented 5 years ago

No strong opinion, as long as warning has it that’s good. (That’s a mistake Akka made where it’s logging api did not allow exceptions to be passed in warning level which really annoyed a lot of people)

I suppose there could be uses to log an error even on trace for things like “this failed but we’re trying again” only to after some errors log an actual errror...

ktoso commented 5 years ago

But people could add these with an extension if it is needed hm

dhoepfl commented 5 years ago

Could you explain why you want to remove it? After all, you can just leave it out on call side.

I, too, think that it is not a uncommon use case to include errors in your log that do not pose a problem:

storeEventLocally(…)
do {
   try sendPendingEventsToServer()
} catch {
   logger.debug("Failed to sync to server, will be sent as part of next sync", error: error)
   // Could also be trace or info.
}
ktoso commented 5 years ago

I think I'm happy to keep the param around, good point @dhoepfl. Let's see what Johannes thinks.

My main concern was to have ti available on warning at least; I don't mind having it on others.

weissi commented 5 years ago

@dhoepfl / @ktoso yeah, let's keep it around, doesn't hurt.