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

API Review for Simmy in v8 #1571

Closed
wants to merge 3 commits into from
Closed
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy.Behavior;

public readonly struct BehaviorActionArguments
{
public ResilienceContext Context { get; }
public BehaviorActionArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Assembly 'Polly.Core'

using System;
using System.ComponentModel.DataAnnotations;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy.Behavior;

public class BehaviorStrategyOptions : MonkeyStrategyOptions
{
public Func<OnBehaviorInjectedArguments, ValueTask>? OnBehaviorInjected { get; set; }
[Required]
public Func<BehaviorActionArguments, ValueTask>? BehaviorAction { get; set; }
public BehaviorStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy.Behavior;

public readonly struct OnBehaviorInjectedArguments
{
public ResilienceContext Context { get; }
public OnBehaviorInjectedArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy.Fault;

public readonly struct FaultGeneratorArguments
{
public ResilienceContext Context { get; }
public FaultGeneratorArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Assembly 'Polly.Core'

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy.Fault;

public class FaultStrategyOptions : MonkeyStrategyOptions
{
public Func<OnFaultInjectedArguments, ValueTask>? OnFaultInjected { get; set; }
public Func<FaultGeneratorArguments, ValueTask<Exception?>>? FaultGenerator { get; set; }
public Exception? Fault { get; set; }
public FaultStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Assembly 'Polly.Core'

using System;
using System.Runtime.CompilerServices;

namespace Polly.Simmy.Fault;

public readonly struct OnFaultInjectedArguments
{
public ResilienceContext Context { get; }
public Exception Fault { get; }
public OnFaultInjectedArguments(ResilienceContext context, Exception fault);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy.Latency;

public readonly struct LatencyGeneratorArguments
{
public ResilienceContext Context { get; }
public LatencyGeneratorArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Assembly 'Polly.Core'

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy.Latency;

public class LatencyStrategyOptions : MonkeyStrategyOptions
{
public Func<OnLatencyArguments, ValueTask>? OnLatency { get; set; }
public Func<LatencyGeneratorArguments, ValueTask<TimeSpan>>? LatencyGenerator { get; set; }
public TimeSpan Latency { get; set; }
public LatencyStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Assembly 'Polly.Core'

using System;
using System.Runtime.CompilerServices;

namespace Polly.Simmy.Latency;

public readonly struct OnLatencyArguments
{
public ResilienceContext Context { get; }
public TimeSpan Latency { get; }
public OnLatencyArguments(ResilienceContext context, TimeSpan latency);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy.Outcomes;

public readonly struct OnOutcomeInjectedArguments<TResult>
{
public ResilienceContext Context { get; }
public Outcome<TResult> Outcome { get; }
public OnOutcomeInjectedArguments(ResilienceContext context, Outcome<TResult> outcome);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy.Outcomes;

public readonly struct OutcomeGeneratorArguments
{
public ResilienceContext Context { get; }
public OutcomeGeneratorArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Assembly 'Polly.Core'

using System;
using System.ComponentModel.DataAnnotations;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy.Outcomes;

public class OutcomeStrategyOptions<TResult> : MonkeyStrategyOptions
{
public Func<OnOutcomeInjectedArguments<TResult>, ValueTask>? OnOutcomeInjected { get; set; }
[Required]
public Func<OutcomeGeneratorArguments, ValueTask<Outcome<TResult>?>> OutcomeGenerator { get; set; }
public OutcomeStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Assembly 'Polly.Core'

namespace Polly.Simmy.Outcomes;

public class OutcomeStrategyOptions : OutcomeStrategyOptions<object>
{
public OutcomeStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Assembly 'Polly.Core'

using System;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Polly.Simmy.Behavior;

namespace Polly.Simmy;

public static class BehaviorPipelineBuilderExtensions
{
public static TBuilder AddChaosBehavior<TBuilder>(this TBuilder builder, double injectionRate, Func<ValueTask> behavior) where TBuilder : ResiliencePipelineBuilderBase;
public static TBuilder AddChaosBehavior<TBuilder>(this TBuilder builder, BehaviorStrategyOptions options) where TBuilder : ResiliencePipelineBuilderBase;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy;

public readonly struct EnabledGeneratorArguments
martintmk marked this conversation as resolved.
Show resolved Hide resolved
{
public ResilienceContext Context { get; }
public EnabledGeneratorArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Assembly 'Polly.Core'

using System;
using System.Diagnostics.CodeAnalysis;
using Polly.Simmy.Fault;

namespace Polly.Simmy;

public static class FaultPipelineBuilderExtensions
{
public static TBuilder AddChaosFault<TBuilder>(this TBuilder builder, double injectionRate, Exception fault) where TBuilder : ResiliencePipelineBuilderBase;
public static TBuilder AddChaosFault<TBuilder>(this TBuilder builder, double injectionRate, Func<Exception?> faultGenerator) where TBuilder : ResiliencePipelineBuilderBase;
public static TBuilder AddChaosFault<TBuilder>(this TBuilder builder, FaultStrategyOptions options) where TBuilder : ResiliencePipelineBuilderBase;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Assembly 'Polly.Core'

using System.Runtime.CompilerServices;

namespace Polly.Simmy;

public readonly struct InjectionRateGeneratorArguments
{
public ResilienceContext Context { get; }
public InjectionRateGeneratorArguments(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Assembly 'Polly.Core'

using System;
using System.Diagnostics.CodeAnalysis;
using Polly.Simmy.Latency;

namespace Polly.Simmy;

public static class LatencyPipelineBuilderExtensions
{
public static TBuilder AddChaosLatency<TBuilder>(this TBuilder builder, double injectionRate, TimeSpan latency) where TBuilder : ResiliencePipelineBuilderBase;
public static TBuilder AddChaosLatency<TBuilder>(this TBuilder builder, LatencyStrategyOptions options) where TBuilder : ResiliencePipelineBuilderBase;
}
14 changes: 14 additions & 0 deletions ApiReview/API.Polly.Core/NoDocs/Polly.Simmy/MonkeyStrategy.T.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Assembly 'Polly.Core'

using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy;

public abstract class MonkeyStrategy<T> : ResilienceStrategy<T>
{
protected MonkeyStrategy(MonkeyStrategyOptions options);
protected ValueTask<bool> ShouldInjectAsync(ResilienceContext context);
}
14 changes: 14 additions & 0 deletions ApiReview/API.Polly.Core/NoDocs/Polly.Simmy/MonkeyStrategy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Assembly 'Polly.Core'

using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy;

public abstract class MonkeyStrategy : ResilienceStrategy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Top Level Comment:
There are 3 terms in this PR that basically represent the same thing:

  • Monkey
  • Simmy
  • Chaos

Do you think we can consolidate and align this somehow?

@martintmk I think all of them are still valid tho, I mean from a brand standpoint if you will, back when Simmy was conceived, kinda the reasoning or the slogan was "Simmy is a monkey to create chaos" and it was developed mapping that, meaning that the Monkey is the one in charge to unleash the chaos as the more basic chaos unit let's say, thus the public base class that consumer can extend to implement their own monkies, for instance, I might want a monkey called Caesar that implements a strategy to stop nodes in a K8s cluster. Now, Simmy is our monkey implementation that offers a set of common chaos strategies.

@joelhulen thoughts?

Moved the this from the #1459 PR.

Alternative naming:

  • Polly.Simmy -> Polly.Chaos (namespace)
  • MonkeyStrategy -> ChaosStrategy
  • MonkeyStrategy<T> -> ChaosStrategy<T>

Personally, I feel this is a cleaner naming, but Simmy would lose the branding and effective dissolve into Polly.

@vany0114, @martincostello, @joelhulen

Copy link
Member

Choose a reason for hiding this comment

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

I have no strong thoughts on branded vs. not, but I agree that the naming should be consistent whichever is chosen.

Copy link
Member

Choose a reason for hiding this comment

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

My two cents is that Simmy has been around for a few years now, and there's fairly steady traffic to the repo (11 clones and 147 unique visitors in the past two weeks alone, not to mention almost 500 stars). I know that @vany0114 has done a bit of work raising awareness of the project. I also like the monkey logo :) All said, I think there is a bit of brand awareness that I don't think we necessarily need to throw away. However, I like the clarity of the proposed naming. But because of the previous points, I'm OK with sticking to the branded naming as long as, like @martincostello said, we stay consistent in the naming. We also should be very descriptive in the XML document and related published documentation to prevent any confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If both @martincostello and @joelhulen are fine with current API and don't have any other suggestions I am fine exposing Simmy as it is now.

@vany0114 Can you create a tasks to expose Simmy in 8.2.0?

What needs to be done:

  • Make the API public
  • Prepare docs for new chaos strategies

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create a tasks to expose Simmy in 8.2.0?

@martintmk by "creating the tasks" you mean the PR(s) right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be better to create an issue here:
https://github.com/App-vNext/Polly/issues/new/choose

Something like [Feature] Expose the Simmy APIs

{
protected MonkeyStrategy(MonkeyStrategyOptions options);
protected ValueTask<bool> ShouldInjectAsync(ResilienceContext context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Assembly 'Polly.Core'

using System;
using System.ComponentModel.DataAnnotations;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

namespace Polly.Simmy;

public abstract class MonkeyStrategyOptions<TResult> : ResilienceStrategyOptions
{
[Range(0.0, 1.0)]
public double InjectionRate { get; set; }
public Func<InjectionRateGeneratorArguments, ValueTask<double>>? InjectionRateGenerator { get; set; }
public Func<EnabledGeneratorArguments, ValueTask<bool>>? EnabledGenerator { get; set; }
public bool Enabled { get; set; }
[Required]
public Func<double> Randomizer { get; set; }
protected MonkeyStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Assembly 'Polly.Core'

namespace Polly.Simmy;

public abstract class MonkeyStrategyOptions : MonkeyStrategyOptions<object>
{
protected MonkeyStrategyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Assembly 'Polly.Core'

using System;
using System.Diagnostics.CodeAnalysis;
using Polly.Simmy.Outcomes;

namespace Polly.Simmy;

public static class OutcomePipelineBuilderExtensions
{
public static ResiliencePipelineBuilder<TResult> AddChaosResult<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TResult>(this ResiliencePipelineBuilder<TResult> builder, double injectionRate, TResult result);
public static ResiliencePipelineBuilder<TResult> AddChaosResult<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TResult>(this ResiliencePipelineBuilder<TResult> builder, double injectionRate, Func<TResult?> resultGenerator);
public static ResiliencePipelineBuilder<TResult> AddChaosResult<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] TResult>(this ResiliencePipelineBuilder<TResult> builder, OutcomeStrategyOptions<TResult> options);
}