boulder icon indicating copy to clipboard operation
boulder copied to clipboard

Document logging best practices

Open aarongable opened this issue 2 years ago • 2 comments

aarongable avatar Nov 16 '23 19:11 aarongable

I feel like logging in the OCSP responder falls under "one log line per request"; the OCSP responder is just so high-volume that we also have to sample there.

logDNSError similarly falls under the "work that happens asynchronously" exception I discuss in the last paragraph. But also it's not clear to me why we log that line at all: wouldn't it be better to include that level of detail in the single "validation failed" audit log object that the VA produces?

I'm fairly opposed to "at most one log line per error" because we have plenty of codepaths that can encounter multiple errors and keep going (or keep retrying). I think the goal I'm trying to accomplish here is to lay out guidelines for us to think about when we're writing new code or making big changes, not to document exactly how things look today.

aarongable avatar Nov 16 '23 20:11 aarongable

I'm fairly opposed to "at most one log line per error" because we have plenty of codepaths that can encounter multiple errors and keep going (or keep retrying).

It sounds like you're arguing against "at least one log line per error," right? "At most" means those codepaths are fine.

But also it's not clear to me why we log that line at all: wouldn't it be better to include that level of detail in the single "validation failed" audit log object that the VA produces?

Yes. FWIW we introduced this log line specifically to debug some weird issues we were seeing (id mismatch) and it could probably be removed. The reason it's not logged by the VA is that the information it logs is not part of the interface exposed by the DNS package. It includes things like the encoded query and encoded response. Though perhaps we could smuggle those to the VA inside private fields of an error object.

jsha avatar Nov 27 '23 19:11 jsha