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.

Sign up for GitHub

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

Jump to bottom

Document outcome strategy anti-patterns #1994

Merged
merged 11 commits into from
Mar 22, 2024
Merged

Document outcome strategy anti-patterns #1994

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cd82241
* document outcome strategy anti-patterns
vany0114 Feb 25, 2024
c297b96
controlled chaos
vany0114 Feb 25, 2024
aa308c1
Merge branch 'main' into simmy-outcome-antipattern
Mar 2, 2024
262b136
wrapping up
Mar 2, 2024
85d88c4
fix snippet link
Mar 2, 2024
c8b618e
put back description
Mar 2, 2024
e47812c
PR feedback.
vany0114 Mar 3, 2024
4c78245
Merge branch 'main' into simmy-outcome-antipattern
vany0114 Mar 9, 2024
1ecd966
PR feedback
vany0114 Mar 9, 2024
b8fde8d
PR feedback
vany0114 Mar 11, 2024
5a7c5e4
pr feedback
vany0114 Mar 21, 2024
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
144 changes: 139 additions & 5 deletions docs/chaos/outcome.md
View file
Open in desktop
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 @@ -165,13 +165,147 @@ new ResiliencePipelineBuilder<HttpResponseMessage>()
{
< 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 them is the telemetry events, which 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".
vany0114 marked this conversation as resolved.
Show resolved Hide resolved

Also, you end up losing control of how/when to inject outcomes vs. faults since this way does not allow you to separately control when to inject a fault vs. an outcome.
vany0114 marked this conversation as resolved.
Show resolved Hide resolved

<!-- 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
vany0114 marked this conversation as resolved.
Show resolved Hide resolved
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 more succinct, but use fault chaos instead as the [`ChaosFaultStrategy`](fault.md) correctly tracks telemetry events effectively as faults not just as any other outcome. Also 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.
vany0114 marked this conversation as resolved.
Show resolved Hide resolved

<!-- 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.
137 changes: 131 additions & 6 deletions src/Snippets/Docs/Chaos.Outcome.cs
View file
Open in desktop
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 @@ -103,17 +106,139 @@ public static void OutcomeGeneratorDelegates()
{
< 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);
}
});

#endregion
}

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
vany0114 marked this conversation as resolved.
Show resolved Hide resolved
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
}

private static HttpResponseMessage CreateResultFromContext(ResilienceContext context) => new(HttpStatusCode.TooManyRequests);
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;
}