Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove user-provided details from various logs #1100

Merged
merged 16 commits into from
Jun 10, 2024
Merged
19 changes: 11 additions & 8 deletions src/DurableTask.Core/Logging/LogEvents.cs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ void IEventSourceEvent.WriteEventSource() =>
StructuredEventSource.Log.TerminatingInstance(
this.InstanceId,
this.ExecutionId,
this.Details,
Details: string.Empty, // User-provided details may contain sensitive data, so we don't log it.
davidmrdavid marked this conversation as resolved.
Show resolved Hide resolved
Utils.AppName,
Utils.PackageVersion);
}
Expand Down Expand Up @@ -668,7 +668,7 @@ void IEventSourceEvent.WriteEventSource() =>
StructuredEventSource.Log.SuspendingInstance(
this.InstanceId,
this.ExecutionId,
this.Details,
Details: string.Empty, // User-provided details may contain sensitive data, so we don't log it.
Utils.AppName,
Utils.PackageVersion);
}
Expand Down Expand Up @@ -704,7 +704,7 @@ void IEventSourceEvent.WriteEventSource() =>
StructuredEventSource.Log.ResumingInstance(
this.InstanceId,
this.ExecutionId,
this.Details,
Details: string.Empty, // User-provided details may contain sensitive data, so we don't log it.
Utils.AppName,
Utils.PackageVersion);
}
Expand Down Expand Up @@ -1054,8 +1054,11 @@ public OrchestrationCompleted(
this.RuntimeStatus = action.OrchestrationStatus.ToString();
this.Details = action.Details;
this.SizeInBytes = Encoding.UTF8.GetByteCount(action.Result ?? string.Empty);
this.sanitizedDetails = action.FailureDetails != null ? $"{action.FailureDetails.ErrorType}\n{action.FailureDetails.StackTrace}" : string.Empty;
}

private string sanitizedDetails { get; }

[StructuredLogField]
public string InstanceId { get; }

Expand Down Expand Up @@ -1085,7 +1088,7 @@ void IEventSourceEvent.WriteEventSource() =>
this.InstanceId,
this.ExecutionId,
this.RuntimeStatus,
this.Details,
Details: this.sanitizedDetails, // Failure details may contain sensitive information, so we sanitize them
this.SizeInBytes,
Utils.AppName,
Utils.PackageVersion);
Expand Down Expand Up @@ -1494,8 +1497,6 @@ void IEventSourceEvent.WriteEventSource() =>

internal class TaskActivityFailure : StructuredLogEvent, IEventSourceEvent
{
readonly Exception exception;

public TaskActivityFailure(
OrchestrationInstance instance,
string name,
Expand All @@ -1507,9 +1508,11 @@ public TaskActivityFailure(
this.Name = name;
this.TaskEventId = taskEvent.EventId;
this.Details = exception.ToString();
jviau marked this conversation as resolved.
Show resolved Hide resolved
this.exception = exception;
this.sanitizedDetails = $"{exception.GetType().FullName}\n{exception.StackTrace}";
}

private string sanitizedDetails { get; }

[StructuredLogField]
public string InstanceId { get; }

Expand Down Expand Up @@ -1540,7 +1543,7 @@ void IEventSourceEvent.WriteEventSource() =>
this.ExecutionId,
this.Name,
this.TaskEventId,
Utils.RedactUserCodeExceptions ? LogHelper.GetRedactedExceptionDetails(this.exception) : this.Details, // We have to be extra guarded about logging user exceptions to EventSource (ETW)
Details: sanitizedDetails,
Utils.AppName,
Utils.PackageVersion);
}
Expand Down
33 changes: 15 additions & 18 deletions src/DurableTask.Core/TaskOrchestrationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -615,8 +615,8 @@ public void FailOrchestration(Exception failure)

string reason = failure.Message;

// string details is legacy, FailureDetails is the newer way to share failure information
string details = null;
// failureDetailsString is legacy, FailureDetails is the newer way to share failure information
string failureDetailsString = null;
FailureDetails failureDetails = null;

// correlation
Expand All @@ -626,16 +626,13 @@ public void FailOrchestration(Exception failure)
CorrelationTraceClient.TrackException(failure);
});

// In this block, we always create a failureDetails variable, which is needed to produce sanitized telemetry
if (failure is OrchestrationFailureException orchestrationFailureException)
{
if (this.ErrorPropagationMode == ErrorPropagationMode.UseFailureDetails)
failureDetails = orchestrationFailureException.FailureDetails;
cgillum marked this conversation as resolved.
Show resolved Hide resolved
if (this.ErrorPropagationMode == ErrorPropagationMode.SerializeExceptions)
{
// When not serializing exceptions, we instead construct FailureDetails objects
failureDetails = orchestrationFailureException.FailureDetails;
}
else
{
details = orchestrationFailureException.Details;
failureDetailsString = orchestrationFailureException.Details;
}
}
else if (failure is TaskFailedException taskFailedException &&
Expand All @@ -646,20 +643,19 @@ public void FailOrchestration(Exception failure)
}
else
{
if (this.ErrorPropagationMode == ErrorPropagationMode.UseFailureDetails)
failureDetails = new FailureDetails(failure);
if (this.ErrorPropagationMode == ErrorPropagationMode.SerializeExceptions)
{
failureDetails = new FailureDetails(failure);
}
else
{
details = $"Unhandled exception while executing orchestration: {failure}\n\t{failure.StackTrace}";
failureDetailsString = $"Unhandled exception while executing orchestration: {failure}\n\t{failure.StackTrace}";

}
}

CompleteOrchestration(reason, details, OrchestrationStatus.Failed, failureDetails);
CompleteOrchestration(reason, failureDetailsString, OrchestrationStatus.Failed, failureDetails);
}

public void CompleteOrchestration(string result, string details, OrchestrationStatus orchestrationStatus, FailureDetails failureDetails = null)
#nullable enable
public void CompleteOrchestration(string result, string failureDetailsString, OrchestrationStatus orchestrationStatus, FailureDetails? failureDetails = null)
{
int id = this.idCounter++;
OrchestrationCompleteOrchestratorAction completedOrchestratorAction;
Expand All @@ -678,14 +674,15 @@ public void CompleteOrchestration(string result, string details, OrchestrationSt

completedOrchestratorAction = new OrchestrationCompleteOrchestratorAction();
completedOrchestratorAction.Result = result;
completedOrchestratorAction.Details = details;
completedOrchestratorAction.Details = failureDetailsString;
completedOrchestratorAction.OrchestrationStatus = orchestrationStatus;
completedOrchestratorAction.FailureDetails = failureDetails;
}

completedOrchestratorAction.Id = id;
this.orchestratorActionsMap.Add(id, completedOrchestratorAction);
}
#nullable disable

class OpenTaskInfo
{
Expand Down