Chaos testing module #130

IgorPerikov · 2018-05-23T17:31:23Z

According to #118
That's what I came up with - a draft implementation of latency injection filter

I also added header setting, so one can understand whether the request is slow because of chaos injection or some other reasons

cc @daviddenton

coveralls · 2018-05-23T17:44:55Z

Coverage decreased (-0.3%) to 92.425% when pulling 124c552 on IgorPerikov:#118 into 9fcb431 on http4k:master.

IgorPerikov · 2018-05-25T08:54:06Z

about failed checks:

1 introduced from codebeat - duplicate code, it's ok since it's a draft to show what I had in mind
coverage decreased - same, one of the filters is not covered
travis docs section failed - I don't understand yet what this about, I guess I'll figure out later on with that

daviddenton · 2018-05-25T09:06:29Z

@IgorPerikov Don't worry about the docs thing - it's a known issue that the build attempts to perform the build docs step on the PR branches. We've tried to fix that a couple of times already but it seems to be elusive! Haven't had a chance to look over your code yet - will try to have a look soon. :)

daviddenton · 2018-05-26T11:54:00Z

Obviously this is still WIP, but a couple of things about how we would envisage this going. Overall you're on the right track - just pointers really - not criticism! :) :

Like the idea of SysEnv instead of sysprops - more 12 factor app etc.. :)
We'd want this stuff be a new module - maybe http4k-testing-chaos?. Obviously that's a bit of work to add in all the right places, but you can follow the examples for something simple like the webdriver module - just search for occurances of webdriver and go from there. :)
It would be nice to separate the algorithm for injecting random behaviour (maybe a ChaosPolicy typealias/interface?), away from the Filters injecting that behaviour. This would accomplish 2 things: leaning on delegation allows users to develop their own policies/behaviours (one of which is the system environmental variable option that you've already come up with), and it makes things easier to test (rather than conflating the randomisation and the behavioural effect). So taking this further, you'd probably get 3 overall concepts:
a. The "ChaosPolicy" to define a couple of predicates which determine if a behaviour should be applied for a request or response (this allows you to do it before or after the downstream call). One of the policy implementations (the default) would be to never apply any behaviours.
b. "Behaviours" - these apply the actual behaviour - e.g latency or blowing up etc.
c. A filter which coordinates #a and #b - so it checks to see if a behaviour should be activated for a request (or a response).
With regards to the environmental version, we generally try to lean on simple domain types rather than primitives. so instead of using an int, you'd use a Duration and get the millisecond delay from that. This is obviously more of a challenge (to deserialise from environmental vars etc), but we can also use the policy idea from Work on refactoring lens #3 to develop this in isolation from the behaviours so that can be done later :)

That was a bit of a braindump as well - if it doesn't make sense then just yell. :)

IgorPerikov · 2018-05-27T13:42:33Z

if it doesn't make sense then just yell. :)

the points you mentioned are really valuable, I agree with all of them, I'll get back with a new stuff later

IgorPerikov · 2018-05-28T20:13:01Z

A few things I'd like to emphasize here:

I'm not feeling good about ChaosBehavioursTest, it's really clumsy, especially the one that checks delay, but I decided to show what is the best thing I came up with
But what is more clumsy is potential test for chaos policies, especially randomized one, I decided not to write test here at all, since it involves a lot of stuff like making a lot of iterations, calculating result distribution, picking right epsilon since nothing is ideal, etc.

IgorPerikov · 2018-06-04T06:42:18Z

@daviddenton kindly ping?

daviddenton · 2018-06-04T12:26:59Z

release-maven-central.sh

@@ -65,5 +65,6 @@ maven_publish "http4k-server-jetty"
 maven_publish "http4k-server-netty"
 maven_publish "http4k-server-undertow"
 maven_publish "http4k-serverless-lambda"
+maven_publish "http4k-testing-chaos"


For the moment we need to remove this line from the commit - because there is a cycle for when we create a new module - it needs to be uploaded to bintray, then we have to ask permission for it to be linked to JCenter, and only then can we sync it to maven. If we add this line now, it'll fail build that we release it in! :)

daviddenton · 2018-06-04T12:29:10Z

http4k-testing-chaos/src/main/kotlin/org/http4k/chaos/ChaosFilters.kt

+            { request ->
+                if (chaosPolicy.shouldInject(request)) {
+                    behaviour.inject(request)
+                    next(request).header(LATENCY_HEADER, behaviour.javaClass.name)


This is no longer LATENCY only - just call it X-http4k-chaos or similar. We can also add the description of the behaviour as a method on the behaviour instance, and then use it to generate dynamic names based on what we did! :)

daviddenton · 2018-06-04T12:30:46Z

http4k-testing-chaos/src/main/kotlin/org/http4k/chaos/ChaosFilters.kt

+const val LATENCY_HEADER = "X-Chaos-Behaviour"
+
+object ChaosFilters {
+    object ChaosPreFilter {


Can probably combine these into a single filter and then have 2 methods on the Policy shouldInject(Request) and shouldInject(Response) - both of which can default to false.

daviddenton · 2018-06-04T12:38:39Z

@IgorPerikov sorry for going quiet - given you comments I wasn't sure if you were still actively committing to the branch so left you to it! :)

Anyway - it's a good start - I've left a couple of comments - the other thing that might help to drive out an API (specifically around the configuration) is to introduce further behaviours now:

Kill the process!
Block the current thread entirely (thread starvation)
Send back an HTTP 500.

* merge pre and post filters into single one * add inject(Response) & inject(Request) methods to behaviour interface * provide more chaos behaviours

IgorPerikov · 2018-06-05T19:15:16Z

@daviddenton new stuff is ready to review!

When I was working under it, I realised, that it would be extremely conventional to have two methods for ChaosBehaviour as well - one for request and one for response

latest changelist:

merge pre and post filters into single one
add inject(Response) & inject(Request) methods to behaviour interface
provide more chaos behaviours

daviddenton · 2018-06-08T16:27:53Z

delete_version.sh

@@ -48,5 +48,6 @@ maven_publish "http4k-server-jetty"
 maven_publish "http4k-server-netty"
 maven_publish "http4k-server-undertow"
 maven_publish "http4k-serverless-lambda"
+maven_publish "http4k-testing-chaos"


this needs to be removed as we won't publish the module until it's in jcenter..

daviddenton · 2018-06-08T16:32:13Z

http4k-testing-chaos/src/main/kotlin/org/http4k/chaos/ChaosBehaviours.kt

+}
+
+class ChaosException(message: String) : Exception(message)
+


generally, the pattern within the project is for this kind of thing to appear as an extension function of the parent type. So, for instance fun ChaosBehaviour.ThrowException(). This one in particular could take an exception as an arg, and default to throw ChaosException as the default.

daviddenton · 2018-06-08T16:32:58Z

http4k-testing-chaos/src/main/kotlin/org/http4k/chaos/ChaosPolicies.kt

+import org.http4k.core.Response
+import java.util.concurrent.ThreadLocalRandom
+
+interface ChaosPolicy {


ditto the remark about extension functions (from above)

daviddenton

Have a look at my suggestions and let me know what you think :)

IgorPerikov · 2018-06-09T11:02:40Z

let me know what you think

sure I will follow the project rules, but tbh I'm not really following this design principle - not a kotlin native speaker yet :). Is this what are you talking here?

class ExceptionThrowingBehaviour : ChaosBehaviour {
    override val description = "Exception"
    override fun inject(response: Response) = throwException()
}

fun ChaosBehaviour.throwException(exception: Exception = ChaosException("Chaos behaviour injected!")): Response {
    throw exception
}

I guess my assumption is wrong, since it seems really weird for me, but I decided to show it just in case :)

At first I thought about moving all the subtypes to be a companion object functions(pretty much like ContentNegotiation interface in body.kt), and it seems more convenient for me, but AFAIK it has nothing in common with extension functions 😮

interface ChaosBehaviour {
    ... // signatures omitted for brevity
    companion object {
        fun throwException(exception: Exception = ChaosException("Chaos behaviour injected!")) = object : ChaosBehaviour {
            override val description = "Exception"

            override fun inject(response: Response): Response {
                throw exception
            }
        }
    }
}

In addition to explanation, I'd be really grateful, if you could point me a few places in codebase where I could find an examples of this pattern for better understanding

@daviddenton

daviddenton · 2018-06-13T15:56:52Z

Hi @IgorPerikov - first off, I'm really, really sorry for the slow turnaround in this - I've been working up against a deadline so am completely flooded at the moment. It should be better by the end of next week so bear with me! Startups are hard! :)

And whilst I'm waiting for a build to run....

Your intuition was completely right about ContentNegotiation being the template for what I meant - the companion object of the ChaosBehaviour interface would be where we would put these core behaviours. Then further behaviours can be added as extension methods TO the companion object, and hence we keep it all nice and discoverable in the IDE.

This also means that you can drop the name "Behaviour" from each of the implementing classes, so you'd have: ChaosBehaviour.ThrowException**. I'd also maybe give the API use the option to use their own exception and keep the ChaosException as a default parameter in the function

** note the capitalization here - we follow a pattern where you can't really tell if a function is a constructor OR a class. This means that if we move from a function to a class at a later date, it doesn't break the API for the user.

IgorPerikov · 2018-06-13T17:33:09Z

Hi, no need for apologize at all, no matter what is the reason. Thanks for clarification - clear and neat, will come back with update when it's ready

IgorPerikov · 2018-06-13T18:40:47Z

seems like I am already back :) don't hesitate to mark/correct names in the code - I got used to them plus english is not my mother tongue

daviddenton · 2018-06-22T07:47:24Z

Sorry (again) - I totally missed your message that you were done. Good news is that stupid deadline is done, so I can start to concentrate on other things! :)

IgorPerikov · 2018-06-22T09:56:27Z

Glad that you meet your deadline! 🎆

IgorPerikov added 2 commits May 23, 2018 10:15

work in progress

3d0d3f6

introduce latency injection filter

6801db1

introduce policy and behaviour abstractions

01f0ae6

IgorPerikov changed the title ~~Latency injection filter~~ Chaos testing module May 30, 2018

daviddenton reviewed Jun 4, 2018

View reviewed changes

review fixes:

4da62dc

* merge pre and post filters into single one * add inject(Response) & inject(Request) methods to behaviour interface * provide more chaos behaviours

daviddenton reviewed Jun 8, 2018

View reviewed changes

daviddenton requested changes Jun 8, 2018

View reviewed changes

make all class implementations to be methods of their interface

124c552

daviddenton merged commit a5ef09f into http4k:master Jun 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chaos testing module #130

Chaos testing module #130

IgorPerikov commented May 23, 2018

coveralls commented May 23, 2018 •

edited

IgorPerikov commented May 25, 2018

daviddenton commented May 25, 2018

daviddenton commented May 26, 2018

IgorPerikov commented May 27, 2018

IgorPerikov commented May 28, 2018

IgorPerikov commented Jun 4, 2018

daviddenton Jun 4, 2018

daviddenton Jun 4, 2018

daviddenton Jun 4, 2018

daviddenton commented Jun 4, 2018

IgorPerikov commented Jun 5, 2018

daviddenton Jun 8, 2018

daviddenton Jun 8, 2018

daviddenton Jun 8, 2018

daviddenton left a comment

IgorPerikov commented Jun 9, 2018

daviddenton commented Jun 13, 2018

IgorPerikov commented Jun 13, 2018

IgorPerikov commented Jun 13, 2018

daviddenton commented Jun 22, 2018

IgorPerikov commented Jun 22, 2018

Chaos testing module #130

Chaos testing module #130

Conversation

IgorPerikov commented May 23, 2018

coveralls commented May 23, 2018 • edited

IgorPerikov commented May 25, 2018

daviddenton commented May 25, 2018

daviddenton commented May 26, 2018

IgorPerikov commented May 27, 2018

IgorPerikov commented May 28, 2018

IgorPerikov commented Jun 4, 2018

daviddenton Jun 4, 2018

Choose a reason for hiding this comment

daviddenton Jun 4, 2018

Choose a reason for hiding this comment

daviddenton Jun 4, 2018

Choose a reason for hiding this comment

daviddenton commented Jun 4, 2018

IgorPerikov commented Jun 5, 2018

daviddenton Jun 8, 2018

Choose a reason for hiding this comment

daviddenton Jun 8, 2018

Choose a reason for hiding this comment

daviddenton Jun 8, 2018

Choose a reason for hiding this comment

daviddenton left a comment

Choose a reason for hiding this comment

IgorPerikov commented Jun 9, 2018

daviddenton commented Jun 13, 2018

IgorPerikov commented Jun 13, 2018

IgorPerikov commented Jun 13, 2018

daviddenton commented Jun 22, 2018

IgorPerikov commented Jun 22, 2018

coveralls commented May 23, 2018 •

edited