Simmy API review Part 1

[peter-csala]

Pull Request

The issue or feature being addressed

#1901

Details on the issue fix or feature implementation

Latency

• Renamed OnLatency to OnLatencyInjected
  • Justification: Every other chaos strategy defines its success hook as OnXYZInjected

Fault

• Removed obsolete documentation comment from the FaultGenerator of the FaultStrategyOptions
  • Justification: Do not encourage re-throwing the same exception instance #1900 has removed Fault from the FaultStrategyOptions

All chaos strategies

• Unified OnXYZInjectedEvent constant's value
  • Justification: All of them start with Chaos so, it will be easier to spot these events in the telemetry logs
• Set the Name property for each options to a default value
  • Justification: All resilience strategy specifies a default Name. Chaos strategies should not be an exception.

Misc

• Fixed copy-paste issue by renaming optionsOnBehaviorInjected variable to optionsOnLatencyInjected in Chaos.Latency

Confirm the following

• I started this PR by branching from the head of the default branch
• I have targeted the PR to merge into the default branch
• I have included unit tests for the issue/feature
• I have successfully run a local build

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cf00835) 84.79% compared to head (c3f20d6) 84.80%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1909   +/-   ##
=======================================
  Coverage   84.79%   84.80%           
=======================================
  Files         312      312           
  Lines        6893     6897    +4     
  Branches     1056     1056           
=======================================
+ Hits         5845     5849    +4     
  Misses        839      839           
  Partials      209      209           

Flag	Coverage Δ
linux	84.80% <100.00%> (+<0.01%)	⬆️
macos	84.80% <100.00%> (+<0.01%)	⬆️
windows	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[martintmk]

Couple of ideas:

How about we rename the following options:

• BehaviorStrategyOptions -> ChaosBehaviorStrategyOptions
• FaultStrategyOptions -> ChaosFaultStrategyOptions
• LatencyStrategyOptions -> ChaosLatencyStrategyOptions

The reasoning is to be consistent with other strategies and extensions:

i.e. :

• AddRetry -> RetryStrategyOptions
• AddChaosXYZ -> ChaosXYZStrategyOptions

I am also thinking whether we should also rename:

• MonkeyStrategyOptions -> ChaosStrategyOptions
• MonkeyStrategyOptions<T> -> ChaosStrategyOptions<T>

The reason is also consistency, we are adding Chaos strategies, not Monkey strategies.

Finally, let's rename AddChaosResult to AddChaosOutcome (because we do allow injecting both results and exceptions)

[peter-csala]

Couple of ideas:

How about we rename the following options:

* `BehaviorStrategyOptions` -> `ChaosBehaviorStrategyOptions`

* `FaultStrategyOptions` -> `ChaosFaultStrategyOptions`

* `LatencyStrategyOptions` -> `ChaosLatencyStrategyOptions`

The reasoning is to be consistent with other strategies and extensions:

i.e. :

* `AddRetry` -> `RetryStrategyOptions`

* `AddChaosXYZ` -> `ChaosXYZStrategyOptions`

I am also thinking whether we should also rename:

* `MonkeyStrategyOptions` -> `ChaosStrategyOptions`

* `MonkeyStrategyOptions<T>` -> `ChaosStrategyOptions<T>`

The reason is also consistency, we are adding Chaos strategies, not Monkey strategies.

Finally, let's rename AddChaosResult to AddChaosOutcome (because we do allow injecting both results and exceptions)

I think what we should decide here whether we want to keep the Monkey anywhere or prefer something else (like Chaos) everywhere. If we chose the latter one then I would prefer to have a separate PR just for that renaming.

I went through several chaos engineering glossaries (like 1, 2) and Chaos Fault or simply just Fault seems to be an accepted term. But sometimes they used the Experiment as an overloaded term: refers to the process, refers to the injected fault. In short I did not find an industry standard naming for this.

cc: @martintmk , @martincostello, @vany0114
What do you guys think?

[martintmk]

My preference would be to keep the Simmy namespace (for branding and "googlability" for anyone familiar with older Simmy), but to use Chaos instead of Monkey. For anyone new to chaos engineering, seeing Chaos gives more clarity compared to Monkey.

Just to compare:

• AddMonkeyFault
• AddMonkeyBehavior
• AddMonkeyLatency

vs

• AddChaosFault
• AddChaosBehavior
• AddChaosLatency

Either way, consistency should be the key and would be nice not to use too many terms:

So either:

• Simmy, Chaos
• Simmy, Monkey

[martincostello]

Agreed - that was one of the comments in the docs PR. We should be consistently in terminology. I would also prefer Chaos, as that has a defined meaning rather than being a personification of it (a rogue monkey).

[peter-csala]

Okay, I'll do the renaming in a new PR.

In order to minimize merge conflicts, I'll not extend this PR's scope further.