Closed davidmrdavid closed 2 months ago
Just realized a bunch of logging unit tests broke, which is good, but it means I need to update them. Still, I'd appreciate a quick pass on the proposed refactoring in the meantime. Thanks
Tests are passing. I'd appreciate a review here, @cgillum. Thanks!
Under certain conditions (user opting in to tracing raw inputs and outputs), our telemetry may capture sensitive information. For example, through logs of exceptions, inputs, output, etc. T
This PR aims to remove the sensitive information by implementing these guidelines:
(1) User-provided inputs and outputs are never part of telemetry as-is. This includes the "reason" label for operations like terminate or rewind, as well as the simpler cases of inputs and outputs to orchestrators and entities.
(2) When application-level exceptions are logged, we only log a sanitized version of them that includes the exception type, and the exception stack trace. The exception message is absent, as it may contain sensitive information.
And that's it. To this end, I've refactored many methods in the
EndToEndTracer
class, as well as their callers. In most cases, the refactoring is simply to avoid logging sensitive parameters via ETW, but still logging them to the user's Application Insights.The more complicated refactorings are all around logging exceptions. In the easy case - I simply refactored the string parameter containing the exception string into an actual
Exception
type, that I can use to create a sanitized exception string containing only the exception type and stack trace. Other cases where the exception object wasn't available in the immediate caller are handled on a case-by-case basis.Finally - some diffs in this PR are minor improvements like adding nullable analysis and removing build warnings.
resolves N/A
Pull request checklist
pending_docs.md
release_notes.md
/src/Worker.Extensions.DurableTask/AssemblyInfo.cs