polly-contrib / simmy Goto Github PK

Hello, I tried to check this out in the slack channel but was not able to get in there. I have been trying to figure out if there are any samples of how to add simmy policies to polly v8 pipelines?

For example, I have created a ResiliencePipelineRegistry and added a Retry and NoOp pipeline to the registry. These items, since they are v8 polly contructs use a ResilienceContext.

Later on down in the code, if my app is in development mode, I try to AddChaosInjectors to the registry using an extension method that should iterate the pipelines in the registry and add MonkeyPolicies dynamically based on configuration values that are stored in the Context.

v7 of Polly uses Context. v8 of Polly uses ResilienceContext. Do the config values added to the ReslienceContext somehow get copied to a Context and should the expectation be that polly v8 can work with ResilienceContext and MonkeyPolices can still use Context?

Its just unclear how to attach MonkeyPolicy to v8 ResiliencePipelines so are there any samples for that or is it just not supported yet.

Create initial, empty Solution (.sln) and Projects to hold Simmy code.

We should avoid the 'Shared Projects' (.shproj) approach of current Polly, and go straight to the flattened structure exemplified in this PR on Polly. IE Could copy an (empty) structure from there, and just rename everything Polly to Simmy. This is also the MS recommendation for cross-platform targeting

I'm trying to follow the README's example on using the Latency handler and am receiving this error with the new syntax.

'InjectLatencyAsyncOptions' does not contain a definition for 'Latency' and no accessible extension method 'Latency' accepting a first argument of type 'InjectLatencyAsyncOptions' could be found (are you missing a using directive or an assembly reference?)

Here is my code for reference:

var latencyPolicy = MonkeyPolicy.InjectLatencyAsync<HttpResponseMessage>(with =>
    with.Latency(TimeSpan.FromSeconds(5))
        .InjectionRate(1.0)
        .Enabled(true)
    );

For what it's worth, the obsolete syntax works, though it warns of CS0618 when compiled.

var latencyPolicy = MonkeyPolicy.InjectLatencyAsync<HttpResponseMessage>(TimeSpan.FromSeconds(5), 1.0, () => true);

I am using Polly.Contrib.Simmy version 0.3.0

We need to update the project file to include all the metadata that we have into the .nuspec file, in order to remove the duplication with the releases notes, that way we only need to update the ChangeLog.md for new releases. Probably, we need to update the build.cake file.

https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#pack-target

Compare: App-vNext/Polly#632

Fix: App-vNext/Polly@a57f85f

It would be nice to be able to run code when chaos is injected (by any of the simmy injection policies).

Example use-case:

var  chaosExceptionPolicy = MonkeyPolicy
    .InjectExceptionAsync(with => with
        .Fault(new TimeoutException())
        .InjectionRate(0.2)
        .Enabled()
        .OnInject((context, exception, cancellation) => logger.LogInformation("Injecting Chaos"));

Alternative:
Using InjectBehavior rather than InjectException, and throwing the exception myself would also work without changes in Simmy.

Currently, all the monkeys are cancellable, which is fine, but we want to allow the user to make the latency monkey no cancellable for those scenarios where even if a token is signaled, they don't want to cancel the latency.

Proposal: add a new property to LatencyOptions/AyncLatencyOptions like this:

internal Func<Context, CancellationToken, Task<bool>> IsCancellable { get; set; }

Might be true by default, then validate it before to honor the token.

We need to add support for .Net Standard 2.1 since it's already released.
https://docs.microsoft.com/en-us/dotnet/standard/net-standard

A PR typically builds twice - for the branch and then the PR. The second build often fails with:

========================================
__UpdateAppVeyorBuildNumber
========================================
Executing task: __UpdateAppVeyorBuildNumber
Build with specified version already exists.

The cake build script intends to set the build number from the git metadata FullSemVer and clearly this repeats for the second build.

The following range of git metadata is typically available via the GitVersion tool used by the build (obtained by running GitVersion.exe in the repository root):

{
  "Major":0,
  "Minor":2,
  "Patch":1,
  "PreReleaseTag":"reisenberger-optionsSyntaxNamespace.1",
  "PreReleaseTagWithDash":"-reisenberger-optionsSyntaxNamespace.1",
  "PreReleaseLabel":"reisenberger-optionsSyntaxNamespace",
  "PreReleaseNumber":1,
  "BuildMetaData":14,
  "BuildMetaDataPadded":"0014",
  "FullBuildMetaData":"14.Branch.reisenberger-optionsSyntaxNamespace.Sha.7741373763fbb2a63c08229c6ccb5db99257e2b0",
  "MajorMinorPatch":"0.2.1",
  "SemVer":"0.2.1-reisenberger-optionsSyntaxNamespace.1",
  "LegacySemVer":"0.2.1-reisenberger-option1",
  "LegacySemVerPadded":"0.2.1-reisenberger-opt0001",
  "AssemblySemVer":"0.2.1.0",
  "AssemblySemFileVer":"0.2.1.0",
  "FullSemVer":"0.2.1-reisenberger-optionsSyntaxNamespace.1+14",
  "InformationalVersion":"0.2.1-reisenberger-optionsSyntaxNamespace.1+14.Branch.reisenberger-optionsSyntaxNamespace.Sha.7741373763fbb2a63c08229c6ccb5db99257e2b0",
  "BranchName":"reisenberger-optionsSyntaxNamespace",
  "Sha":"7741373763fbb2a63c08229c6ccb5db99257e2b0",
  "ShortSha":7741373,
  "NuGetVersionV2":"0.2.1-reisenberger-opt0001",
  "NuGetVersion":"0.2.1-reisenberger-opt0001",
  "NuGetPreReleaseTagV2":"reisenberger-opt0001",
  "NuGetPreReleaseTag":"reisenberger-opt0001",
  "CommitsSinceVersionSource":14,
  "CommitsSinceVersionSourcePadded":"0014",
  "CommitDate":"2019-11-30"
}

Suggested resolution

Change that line in the build script to use a more specific metadata like InformationalVersion (if not too long for AppVeyor)

• The two builds of the PR do have different SHAs - one for the head-of-the-branch commit, one for the merge-commit; confirmed by the event metadata in AppVeyor. So this should provide distinct build numbers.

However, when we deploy releases directly out of AppVeyor, that "Appveyor build number" may also be tagged to the commit and be used as the release number (think that's the case). And InformationalVersion seems overkill for releases ...

So we possibly want something like:

appveyorBuildNumber = gitVersionOutput["BranchName"].ToString() == "master" ? gitVersionOutput["FullSemVer"].ToString() : gitVersionOutput["InformationalVersion"].ToString();

(A suggestion ... open to anyone refining this!)

All current overloads are of the form:

MonkeyPolicy.InjectX(/* what to inject */, Double injectionRate, bool enabled) // or similar Func variants for injectionRate and enabled

Unit tests are likely to always want to inject the fault/response (ie injectionRate: 1 and enabled: true). Propose: Add syntax overloads throughout, of the form:

MonkeyPolicy.InjectX(/* what to inject */)

which chain on to injectionRate: 1 and enabled: true.

Timing

Would be nice to have this before launch, as stubbing for unit-tests is a major use case we can advertise, as part of the launch

See also #16 . Makes sense to implement #16 before this.

Alongside chaos-engineering, another significant use-case for Simmy is setting up calls to return stub/fake results, for unit tests.

We already have the functionality, InjectResultImplementation, but all our syntax uses the wording InjectFault.

Not all results one might want to inject, in unit testing, are faults. Success cases are valid to test. A more neutral syntax to support this would be InjectResult, eg:

MonkeyPolicy.InjectResult<HttpResponseMessage>(new HttpResponseMessage(HttpStatusCode.OK))

Proposal I

• Rename the Fault namespaces to be Outcomes (ie Simmy.Outcomes), in both Simmy and Simmy.Specs,
• Add files InjectResultSyntax and AsyncInjectResultSyntax, giving various overloads InjectResult<TResult>(...) and InjectResultAsync<TResult>(...)
• These overloads would be identical to the existing InjectFault<TResult>(...) overloads. To avoid code duplication, likely the InjectResult(...) overloads should be the master code, and InjectFault(...) overloads should simply chain to the corresponding InjectResult(...) overload.

Advantages/disadvantages: Retains existing InjectFault<TResult>(...) syntax, but leaves some minor duplication, as there would be two syntaxes (InjectFault<TResult>(...) and InjectResult<TResult>(...)) which cause identical behaviour. But gives nice-reading syntax for each use case.

Proposal II

• Rename the Fault namespaces to be Outcomes (ie Simmy.Outcomes), in both Simmy and Simmy.Specs,
• Rename the existing InjectFault<TResult>(...) overloads to be InjectResult(...).
• Rename the existing InjectFault(Exception ...) overloads to be InjectException(...).

Advantages/disadvantages: No duplication. Loss of InjectFault(...) syntax.

Timing

Would be nice to have this before launch, as stubbing for unit-tests is a major use case we can advertise, as part of the launch

It seems reasonable that the delegates used to configure async MonkeyPolicy instances allow users to provide an async lambda. This permits users make async calls to consult some config source, for the enabled toggle, the injectionRate, or indeed what Exception, TResult, TimeSpan is going to be injected in fault scenarios. Currently the signatures we provide for such delegates are of the form Func<..., Task<bool>>, Func<..., Task<double>>, Func<..., Task<Exception>>, Func<..., Task<TResult>> etc.

That said, many (perhaps even a majority) usages might be sync. Simmy offers overloads taking constant values. Even for config-driven cases, it might be preferable for looking up the config to be a rapid in-memory check, with new values being pushed into that in-memory store by some other process (callback/monitor) when the underlying config source changes.

We want users to be able to include MonkeyPolicy in prod/whatever systems with minimal perf impact on executions, so want to optimise to minimise allocations and execution time.

Propose: Consider the use of ValueTask<>. Refs;

• Stephen Toub blog
• Szymon Kulec blog
• SO question with eminent contributions from eg Eric Lippert and Stephen Cleary

I'm thrilled to announce that Simmy is now part of Polly, so starting with version 8.3.0, Polly has integrated Simmy into its core. For more information, please refer to the dedicated chaos engineering documentation.

Implement the new syntax to avoid the proliferation of overloads in order to allow Simmy can grow easier and the new features can be added more flexibly avoiding breaking changes and deprecate too much code.

Syntax proposal:

.PolicyType(Action<IPolicyTypeConfiguration>)

Example:

var policy = MonkeyPolicy.InjectLatency(options => 
   {
        options.Latency = TimeSpan.FromMilliseconds(5),
        options.InjectionRate = 0.1,
        options.Enabled = true
   };
);

As @martincostello mentions here, given the Simmy API does offer async overloads to obtain configuration values in the async cases, those APIs might want take to a CancellationToken.

We would need to agree what the assumptions should be if cancellation was signalled for a config-obtaining delegate:

• for delegates returning bool enabled, we would probably have no option but to assume cancellation means we return false
• for delegates returning injectionRate or Timespan we would probably likewise have no better option than to assume cancellation means the fault-injection operation should default to not injecting the fault

Thoughst?

x-ref: #18 affects same APIs

Throwing this here to take into consideration, according to Microsoft, we should consider adopting NRT by next year:
https://devblogs.microsoft.com/dotnet/embracing-nullable-reference-types/

There are some caveats tho regarding C# 8 and multitargeting that I'm not sure if we have to care about (I have to dig into it) but as I said, just to consider it, let me know your thoughts @reisenberger please.

Hi,
I've wrote an sample Azure Function App to start learning about resilient systems using your cool project.

I inject a latency and a fault chaos policy but it seems that the raised exception is swallowed by the function runtime. From these specs I can see that the consumer code should be able to catch the exception.

This is the relevant part of my code:

static class PolicyBuilder
{
    public static Policy CreateLatency() => MonkeyPolicy.InjectLatency(with =>
        with.Latency(TimeSpan.FromSeconds(Settings.Latency.Seconds))
        .InjectionRate(Settings.Latency.InjectionRate)
        .Enabled(Settings.Latency.Enabled));

    public static Policy CreateFault() => MonkeyPolicy.InjectException(with =>
        with.Fault(new Exception("Something gone wrong."))
            .InjectionRate(Settings.Fault.InjectionRate)
            .Enabled(Settings.Fault.Enabled)
        );
}

public class ProductsFunction
{
    readonly ILogger _logger;
    readonly IBackend _backend = new FakeBackend();
    readonly Policy _latencyPolicy = PolicyBuilder.CreateLatency();
    readonly Policy _faultPolicy = PolicyBuilder.CreateFault();

    public ProductsFunction(ILoggerFactory loggerFactory)
    {
        _logger = loggerFactory.CreateLogger($"Acme.{nameof(ProductsFunction)}");
    }

    [FunctionName("ProductsFunction")]
    public Task<IActionResult> Run(
        [HttpTrigger(AuthorizationLevel.Anonymous, "get", "post", Route = null)] HttpRequest req)
    {
        _logger.LogInformation($"{nameof(ProductsFunction)} trigger function processed a request.");

        var sku = string.IsNullOrEmpty(req.Query["sku"])
            ? (int?)null
            : Convert.ToInt32(req.Query["sku"]);

        var policyWrap = Policy.Wrap(_latencyPolicy, _faultPolicy);
            
        try {
            var products = policyWrap.ExecuteAndCapture(() => _backend.GetProducts(sku));
            return Task.FromResult<IActionResult>(new OkObjectResult(products));
        }
        catch (Exception e) {
            _logger.LogCritical(e, "Exception"); // breakpoint set here doesn't stop
            return Task.FromResult<IActionResult>(new BadRequestObjectResult(e.Message));
        }
    }
}

As commented in the code the catch block is never executed. Anyway when fault occurs instead of fake data the exception is rendered in JSON (with wrong 200 OK status code):

{
    "outcome": 1,
    "finalException": {
        "ClassName": "System.Exception",
        "Message": "Something gone wrong.",
        "Data": null,
        "InnerException": null,
        "HelpURL": null,
        "StackTraceString": "   at Polly.Contrib.Simmy.MonkeyEngine.InjectExceptionImplementation[TResult](Func`3 action, Context context, CancellationToken cancellationToken, Func`3 injectedException, Func`3 injectionRate, Func`3 enabled)\r\n   at Polly.Contrib.Simmy.Outcomes.InjectOutcomePolicy.Implementation[TResult](Func`3 action, Context context, CancellationToken cancellationToken)\r\n   at Polly.Policy.Execute[TResult](Func`3 action, Context context, CancellationToken cancellationToken)\r\n   at Polly.Wrap.PolicyWrapEngine.<>c__DisplayClass3_0`1.<Implementation>b__0(Context ctx, CancellationToken ct)\r\n   at Polly.Contrib.Simmy.MonkeyEngine.InjectBehaviourImplementation[TResult](Func`3 action, Context context, CancellationToken cancellationToken, Action`2 injectedBehaviour, Func`3 injectionRate, Func`3 enabled)\r\n   at Polly.Contrib.Simmy.Latency.InjectLatencyPolicy.Implementation[TResult](Func`3 action, Context context, CancellationToken cancellationToken)\r\n   at Polly.Policy.Execute[TResult](Func`3 action, Context context, CancellationToken cancellationToken)\r\n   at Polly.Wrap.PolicyWrapEngine.Implementation[TResult](Func`3 func, Context context, CancellationToken cancellationToken, ISyncPolicy outerPolicy, ISyncPolicy innerPolicy)\r\n   at Polly.Wrap.PolicyWrap.Implementation[TResult](Func`3 action, Context context, CancellationToken cancellationToken)\r\n   at Polly.Policy.Execute[TResult](Func`3 action, Context context, CancellationToken cancellationToken)\r\n   at Polly.Policy.ExecuteAndCapture[TResult](Func`3 action, Context context, CancellationToken cancellationToken)",
        "RemoteStackTraceString": null,
        "RemoteStackIndex": 0,
        "ExceptionMethod": null,
        "HResult": -2146233088,
        "Source": "Polly.Contrib.Simmy",
        "WatsonBuckets": null
    },
    "exceptionType": 1,
    "result": null,
    "finalHandledResult": null,
    "faultType": 1,
    "context": {}
}

Any help really appreciated.

Regards,
Giacomo S. S.

I'm new to C#, Polly and Simmy but after reading all the docs and looking at the samples, I thought I'd try to update the Polly sample to use Simmy to generate failures instead of the custom throttling handler.

I copy/pasted the fault injection samples from https://github.com/Polly-Contrib/Simmy/blob/master/README.md ("Step 1") into ValuesController.cs. I then tried to wrap them together using PolicyWrap() before calling Execute but am getting a compiler error:

Severity Code Description Project File Line Suppression State
Error CS1503 Argument 1: cannot convert from 'Polly.Contrib.Simmy.Fault.InjectOutcomePolicy<System.Net.Sockets.SocketException>' to 'Polly.ISyncPolicy' PollyTestApp C:\src\Polly-Samples\PollyTestApp\Controllers\ValuesController.cs 39 Active

What am I missing? Why can't the fault policy be wrapped like the other policy types?

I forked the Polly-Sample repo and created a PR with my attempted changes which should make figuring out what I'm attempting easier to see:

https://github.com/App-vNext/Polly-Samples/pull/22/files

Decide whether we launch Simmy as a semver version number:

• 0.1 : if we consider 'early' version
• 0.9 ; if we are holding off a 1.0 at launch, but to indicate we are close to a 1.0 and confident in the product and public API (as feels like we will be)
• 1.0 : to indicate this is the public API, we are confident in the public API not changing.

For decision as we go over any final clean-up of the code.

Comment welcome.

ReadMe

Should:

• provide brief overview of what the project is
• how to install it
• supported targets
• immediate code examples

See Polly ReadMe for examples.

Wiki

Branching to a wiki can allow us to keep the ReadMe focused, and use a wiki page to deep-dive on any concept/example if it deserves it.

Start wiki when we feel there is critical mass to deserve it.

Changelog

Change log in Changelog.md and Simmy.nuspec files should also be filled out to describe v0.1.0

The MonkeyPolicy implementations validate injectionRate at run-time.

For those policy configuration overloads which take a Double injectionRate at configuration-time, it would be nice to have the same validation at configuration-time rather than at run-time.

Example overload; there are many others.

Nice-to-have: not essential for go-live

We have an offer from a member of the App-vNext team to create a logo ...

Currently, latency injected by MonkeyPolicy.InjectLatency(...) or MonkeyPolicy.InjectLatencyAsync(...) is not cancellable, in either the sync (1; 2) or async (1; 2) implementations.

Async case

Not allowing cancellation doesn't seem right in the async case. Async APIs of typical systems called through Polly typically support cancellation. By providing a MonkeyPolicy.InjectLatencyAsync(...) that doesn't support cancellation, we don't allow users to test how their production system would respond if there was (say) an extra 20ms latency.

For example, if the call is to some Azure SDK API which accepts a CancellationToken and the user is using an TimeoutPolicyAsync() or their own timing-out CancellationToken, then the call would typically cancel when the token is signalled. To allow users to simulate (in prod; or unit tests) that that behaviour works, the injected latency would want to be cancellable too.

Recommend: Change MonkeyPolicy.InjectLatencyAsync(...) to honour the CancellationToken passed to the .ExecuteAsync(...) call through the policies.

Option (now or later): Add an bool isCancellable parameter to (some) overloads of MonkeyPolicy.InjectLatencyAsync(...), allowing the user choice over whether they are injecting cancellable or non-cancellable latency.

Sync case

The sync case seems opposite. Sync APIs typically don't support cancellation. Therefore, it would be a more realistic simulation for the sync MonkeyPolicy.InjectLatency(...) to not support cancellation, at least by default.

Recommend: Leave MonkeyPolicy.InjectLatency(...) not honouring the CancellationToken passed to the .Execute(...) call through the policies.

Option (now or later): Add an bool isCancellable parameter to (some) overloads of MonkeyPolicy.InjectLatency(...), allowing the user choice over whether they are injecting cancellable or non-cancellable latency.

We need to increase the coverage for InjectBehaviour policies, there are overloads that don't even have unit tests.

Update Simmy to reference Polly v7.1.0

Bring Simmy code over from the Polly repo

Bring exactly the Monkey folders (code, and tests) over from App-vNext/Polly#553 . (Code may not need to be in Monkey sub-folders when part of the Simmy repo.)

After #2. After Polly v7.0 released.

New code will need to take a dependency (stated in nuspec; csprojs) on Polly >= v7.0

Add cake build and nuget packaging

Add cake build and packaging. After #2 .

Can just copy the cake build from App-vNext/Polly#504 and rename everything Polly to Simmy.

Also set up automated deployment to GitHub and nuget.org.