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

Document outcome strategy anti-patterns #1994

Merged
merged 11 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 140 additions & 6 deletions docs/chaos/outcome.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
OutcomeGenerator = static args =>
{
var response = new HttpResponseMessage(HttpStatusCode.InternalServerError);
return new ValueTask<Outcome<HttpResponseMessage>?>(Outcome.FromResult(response));
return ValueTask.FromResult<Outcome<HttpResponseMessage>?>(Outcome.FromResult(response));
},
InjectionRate = 0.1
})
Expand Down Expand Up @@ -143,13 +143,13 @@ new ResiliencePipelineBuilder<HttpResponseMessage>()
OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
.AddResult(() => new HttpResponseMessage(HttpStatusCode.InternalServerError)) // Result generator
.AddResult(() => new HttpResponseMessage(HttpStatusCode.TooManyRequests), weight: 50) // Result generator with weight
.AddResult(context => CreateResultFromContext(context)) // Access the ResilienceContext to create result
.AddResult(context => new HttpResponseMessage(CreateResultFromContext(context))) // Access the ResilienceContext to create result
.AddException<HttpRequestException>(), // You can also register exceptions
});
```
<!-- endSnippet -->

### Use delegates to generate faults
### Use delegates to generate outcomes

Delegates give you the most flexibility at the expense of slightly more complicated syntax. Delegates also support asynchronous outcome generation, if you ever need that possibility.

Expand All @@ -159,19 +159,153 @@ new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
// The same behavior can be achieved with delegates
OutcomeGenerator = args =>
OutcomeGenerator = static args =>
{
Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
{
< 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
< 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
< 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
< 250 => Outcome.FromResult(new HttpResponseMessage(CreateResultFromContext(args.Context))),
< 350 => Outcome.FromException<HttpResponseMessage>(new TimeoutException()),
_ => null
_ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
};

return ValueTask.FromResult(outcome);
}
});
```
<!-- endSnippet -->

## Anti-patterns

### Injecting faults (exceptions)

❌ DON'T

Use outcome strategies to inject faults in advanced scenarios which you need to inject outcomes using delegates. This is an opinionated anti-pattern since you can consider an exception as a result/outcome, however, there might be undesired implications when doing so. One of these implications is these is to the telemetry events. Events might end up affecting your metrics as the `ChaosOutcomeStrategy` reports both result and exceptions in the same way. This could pose a problem for instrumentation purposes since it's clearer looking for fault injected events to be 100% sure where/when exceptions were injected, rather than have them mixed in the same "bag".

Another problem is that you end up losing control of how/when to inject outcomes vs. faults. This is because the approach does not allow you to separately control when to inject a fault vs. an outcome.

<!-- snippet: chaos-outcome-anti-pattern-generator-inject-fault -->
```cs
var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
InjectionRate = 0.5, // Same injection rate for both fault and outcome
OutcomeGenerator = static args =>
{
Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
{
< 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
< 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
< 250 => Outcome.FromResult(new HttpResponseMessage(CreateResultFromContext(args.Context))),
< 350 => Outcome.FromException<HttpResponseMessage>(new HttpRequestException("Chaos request exception.")), // ⚠️ Avoid this ⚠️
_ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
};

return ValueTask.FromResult(outcome);
},
OnOutcomeInjected = static args =>
{
// You might have to put some logic here to determine what kind of output was injected. 😕
if (args.Outcome.Exception != null)
{
Console.WriteLine($"OnBehaviorInjected, Exception: {args.Outcome.Exception.Message}, Operation: {args.Context.OperationKey}.");
}
else
{
Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
}

return default;
}
})
.Build();
```
<!-- endSnippet -->

✅ DO

The previous approach is tempting since it looks like less code, but use fault chaos instead as the [`ChaosFaultStrategy`](fault.md) correctly tracks telemetry events as faults, not just as any other outcome. By separating them, you can control the injection rate and enable/disable them separately which gives you more control when it comes to injecting chaos dynamically and in a controlled manner.

<!-- snippet: chaos-outcome-pattern-generator-inject-fault -->
```cs
var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosFault(new ChaosFaultStrategyOptions
{
InjectionRate = 0.1, // Different injection rate for faults
EnabledGenerator = static args => ShouldEnableFaults(args.Context), // Different settings might apply to inject faults
FaultGenerator = static args =>
{
Exception? exception = RandomThreshold switch
{
>= 250 and < 350 => new HttpRequestException("Chaos request exception."),
_ => null
};

return ValueTask.FromResult(exception);
},
OnFaultInjected = static args =>
{
Console.WriteLine($"OnFaultInjected, Exception: {args.Fault.Message}, Operation: {args.Context.OperationKey}.");
return default;
}
})
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
InjectionRate = 0.5, // Different injection rate for outcomes
EnabledGenerator = static args => ShouldEnableOutcome(args.Context), // Different settings might apply to inject outcomes
OutcomeGenerator = static args =>
{
HttpStatusCode statusCode = RandomThreshold switch
{
< 100 => HttpStatusCode.InternalServerError,
< 150 => HttpStatusCode.TooManyRequests,
< 250 => CreateResultFromContext(args.Context),
_ => HttpStatusCode.OK
};

return ValueTask.FromResult<Outcome<HttpResponseMessage>?>(Outcome.FromResult(new HttpResponseMessage(statusCode)));
},
OnOutcomeInjected = static args =>
{
Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
return default;
}
})
.Build();
```
<!-- endSnippet -->

❌ DON'T

Use outcome strategies to inject only faults, use the [`ChaosFaultStrategy`](fault.md) instead.

<!-- snippet: chaos-outcome-anti-pattern-only-inject-fault -->
```cs
new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
.AddException<HttpRequestException>(), // ⚠️ Avoid this ⚠️
});
```
<!-- endSnippet -->

✅ DO

Use fault strategies to inject the exception.

<!-- snippet: chaos-outcome-pattern-only-inject-fault -->
```cs
new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosFault(new ChaosFaultStrategyOptions
{
FaultGenerator = new FaultGenerator()
.AddException<HttpRequestException>(),
});
```
<!-- endSnippet -->

> [!NOTE]
> Even though the outcome strategy is flexible enough to allow you to inject outcomes as well as exceptions without the need to chain a fault strategy in the pipeline, use your judgment when doing so because of the caveats and side effects explained regarding telemetry and injection control.
139 changes: 132 additions & 7 deletions src/Snippets/Docs/Chaos.Outcome.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
using System.Net;
using System.Net;
using System.Net.Http;
using Polly.Retry;
using Polly.Simmy;
using Polly.Simmy.Fault;
using Polly.Simmy.Outcomes;

namespace Snippets.Docs;
Expand All @@ -10,6 +11,8 @@ namespace Snippets.Docs;

internal static partial class Chaos
{
private static readonly int RandomThreshold = Random.Shared.Next(350);

public static void OutcomeUsage()
{
#region chaos-outcome-usage
Expand Down Expand Up @@ -63,7 +66,7 @@ public static void OutcomeUsage()
OutcomeGenerator = static args =>
{
var response = new HttpResponseMessage(HttpStatusCode.InternalServerError);
return new ValueTask<Outcome<HttpResponseMessage>?>(Outcome.FromResult(response));
return ValueTask.FromResult<Outcome<HttpResponseMessage>?>(Outcome.FromResult(response));
},
InjectionRate = 0.1
})
Expand All @@ -82,7 +85,7 @@ public static void OutcomeGenerator()
OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
.AddResult(() => new HttpResponseMessage(HttpStatusCode.InternalServerError)) // Result generator
.AddResult(() => new HttpResponseMessage(HttpStatusCode.TooManyRequests), weight: 50) // Result generator with weight
.AddResult(context => CreateResultFromContext(context)) // Access the ResilienceContext to create result
.AddResult(context => new HttpResponseMessage(CreateResultFromContext(context))) // Access the ResilienceContext to create result
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usage samples advocate to use one of the anti-patterns

...
.AddResult(context => new HttpResponseMessage(CreateResultFromContext(context))) // Access the ResilienceContext to create result
.AddException<HttpRequestException>(), // You can also register exceptions

Please update these OutcomeGenerators by removing theAddException clauses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I originally opened the PR I had removed those guys since I had deprecated it, however, I put them back so that we support that scenario (injecting results and exceptions in the same strategy) but under their judgment after knowing the caveats/side effects explained in the anti-pattern sections and in the footer. I can remove that entirely from the docs but it would be hidden. Honestly, while I understand it is handy it feels like we're kinda discouraging people to use the right strategy to inject faults.

We could also add a comment in the samples as I did in some snippets, like this: // ⚠️ Avoid this ⚠️
and maybe add a remarks on the API pointing to the docs? thoughts?

.AddException<HttpRequestException>(), // You can also register exceptions
});

Expand All @@ -97,15 +100,15 @@ public static void OutcomeGeneratorDelegates()
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
// The same behavior can be achieved with delegates
OutcomeGenerator = args =>
OutcomeGenerator = static args =>
{
Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
{
< 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
< 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
< 250 => Outcome.FromResult(CreateResultFromContext(args.Context)),
< 250 => Outcome.FromResult(new HttpResponseMessage(CreateResultFromContext(args.Context))),
vany0114 marked this conversation as resolved.
Show resolved Hide resolved
< 350 => Outcome.FromException<HttpResponseMessage>(new TimeoutException()),
_ => null
_ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
};

return ValueTask.FromResult(outcome);
Expand All @@ -115,5 +118,127 @@ public static void OutcomeGeneratorDelegates()
#endregion
}

private static HttpResponseMessage CreateResultFromContext(ResilienceContext context) => new(HttpStatusCode.TooManyRequests);
public static void AntiPattern_GeneratorDelegateInjectFault()
{
#region chaos-outcome-anti-pattern-generator-inject-fault
var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
InjectionRate = 0.5, // Same injection rate for both fault and outcome
OutcomeGenerator = static args =>
{
Outcome<HttpResponseMessage>? outcome = Random.Shared.Next(350) switch
{
< 100 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)),
< 150 => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.TooManyRequests)),
< 250 => Outcome.FromResult(new HttpResponseMessage(CreateResultFromContext(args.Context))),
< 350 => Outcome.FromException<HttpResponseMessage>(new HttpRequestException("Chaos request exception.")), // ⚠️ Avoid this ⚠️
_ => Outcome.FromResult(new HttpResponseMessage(HttpStatusCode.OK))
};

return ValueTask.FromResult(outcome);
},
OnOutcomeInjected = static args =>
{
// You might have to put some logic here to determine what kind of output was injected. 😕
if (args.Outcome.Exception != null)
{
Console.WriteLine($"OnBehaviorInjected, Exception: {args.Outcome.Exception.Message}, Operation: {args.Context.OperationKey}.");
}
else
{
Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
}

return default;
}
})
.Build();

#endregion
}

public static void Pattern_GeneratorDelegateInjectFaultAndOutcome()
{
#region chaos-outcome-pattern-generator-inject-fault
var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosFault(new ChaosFaultStrategyOptions
{
InjectionRate = 0.1, // Different injection rate for faults
EnabledGenerator = static args => ShouldEnableFaults(args.Context), // Different settings might apply to inject faults
FaultGenerator = static args =>
{
Exception? exception = RandomThreshold switch
{
>= 250 and < 350 => new HttpRequestException("Chaos request exception."),
_ => null
};

return ValueTask.FromResult(exception);
},
OnFaultInjected = static args =>
{
Console.WriteLine($"OnFaultInjected, Exception: {args.Fault.Message}, Operation: {args.Context.OperationKey}.");
return default;
}
})
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
InjectionRate = 0.5, // Different injection rate for outcomes
EnabledGenerator = static args => ShouldEnableOutcome(args.Context), // Different settings might apply to inject outcomes
OutcomeGenerator = static args =>
{
HttpStatusCode statusCode = RandomThreshold switch
{
< 100 => HttpStatusCode.InternalServerError,
< 150 => HttpStatusCode.TooManyRequests,
< 250 => CreateResultFromContext(args.Context),
_ => HttpStatusCode.OK
};

return ValueTask.FromResult<Outcome<HttpResponseMessage>?>(Outcome.FromResult(new HttpResponseMessage(statusCode)));
},
OnOutcomeInjected = static args =>
{
Console.WriteLine($"OnBehaviorInjected, Outcome: {args.Outcome.Result}, Operation: {args.Context.OperationKey}.");
return default;
}
})
.Build();
#endregion
}

public static void AntiPattern_OnlyInjectFault()
{
#region chaos-outcome-anti-pattern-only-inject-fault

new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosOutcome(new ChaosOutcomeStrategyOptions<HttpResponseMessage>
{
OutcomeGenerator = new OutcomeGenerator<HttpResponseMessage>()
.AddException<HttpRequestException>(), // ⚠️ Avoid this ⚠️
});

#endregion
}

public static void Pattern_OnlyInjectFault()
{
#region chaos-outcome-pattern-only-inject-fault

new ResiliencePipelineBuilder<HttpResponseMessage>()
.AddChaosFault(new ChaosFaultStrategyOptions
{
FaultGenerator = new FaultGenerator()
.AddException<HttpRequestException>(),
});

#endregion
}

private static ValueTask<bool> ShouldEnableFaults(ResilienceContext context) => ValueTask.FromResult(true);

private static ValueTask<bool> ShouldEnableOutcome(ResilienceContext context) => ValueTask.FromResult(true);

private static HttpStatusCode CreateResultFromContext(ResilienceContext context) => HttpStatusCode.TooManyRequests;
}