Trace the ProcessRunner #20354
Trace the ProcessRunner #20354
Conversation
| @@ -25,6 +25,7 @@ internal sealed class ProcessRunner : IDisposable | |||
|
|
|||
| public ProcessRunner(IProcess process, TimeSpan timeout, CancellationToken cancellationToken) | |||
| { | |||
| AzureIdentityEventSource.Singleton.ProcessRunnerInformational($"Running process `{process.StartInfo.FileName}' with arguments {string.Join(", ", process.StartInfo.Arguments)}"); | |||
There was a problem hiding this comment.
I believe the arguments will contains a resource string in most cases, as well as tenant id if it was specified by the user. We've talked before about whether these are considered PII in the past, but I don't recall the outcome. @g2vinay are we ok to log resource strings and tenant ids or should they be redacted?
There was a problem hiding this comment.
I believe last we decided these were not PII.
In Java scopes are logged in other credentials when a token is retrieved.
@catalinaperalta can you confirm this ?
There was a problem hiding this comment.
If I recall correctly, tenantID was not PII but resource strings do get close to being PII depending on the resource name. I'll try to find my notes about this.
There was a problem hiding this comment.
Just checked tenantID is OII (organization identifiable data) so not PII, but I think resources (and therefore their resource strings?) do fall under customer content. I can share the link I have for the data governance site on our teams channel.
There was a problem hiding this comment.
Maybe, just to be extra cautious, this change should be coupled with the upcoming setting to log PII information (previously only relevant for MSAL logging).
ghost
added
the
no-recent-activity
|
Hi @christothes. Thank you, for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
|
You're welcome, msftbot. |
ghost
removed
the
no-recent-activity
|
dependent on #22852 |
christothes
force-pushed
the
chriss/ProcesRunnerLogging
branch
from
39b9e0f to
6db08cb
Compare
To better troubleshoot process based credentials like AzureCli, VS, and VSCode.
related to #20210