Toxiproxy 2.0 RFC

[xthexder]

Proposed API Changes

• Remove enabled field from all toxics
• POST /proxies/{proxy}/{up|down}stream/toxics - Create (enable) a new toxic
• POST /proxies/{proxy}/{up|down}stream/toxics/{toxic} - Update an existing toxic
• DELETE /proxies/{proxy}/{up|down}stream/toxics/{toxic} - Destroy (disable) a toxic

This API matches the current proxy creation/deletion api.

Internal Structure Changes

Current 1.x architecture: https://docs.google.com/drawings/d/1pwuRINbpwyaQeYSgGt8VTEKWEUQAwYpNJIMH1owW3Sc/view

Instead of a static number of ToxicStubs (and a static mapping for each toxic), ToxicStubs will be dynamically added / removed as toxics are created/destroyed. A minimum of 1 ToxicStub will always be present, and the first toxic will always be a NoopToxic so that it can be interrupted for inserting / removing toxics.

Steps for adding a new toxic:

1. Interrupt the last toxic in the chain
2. Reconnect the output channels so that the new toxic is between the last toxic and the end ChanReader
3. Resume both ToxicStubs with their toxics

Steps for removing a toxic:

1. Interrupt the toxic before the target in order to ensure the target's input channel is empty
2. Interrupt the target toxic
3. Connect the previous toxic's output to the target toxic's output
4. Resume the previous toxic / ToxicStub pair

This ends up being very similar to a doubly-linked list, except that we need to ensure no data is in-flight between the links (channels) when they're removed.

Toxic Registration

The main() function and server creation will be simplified so that it can be used as a library.
Instead of having all the toxics hard-coded inside toxiproxy, they will be registered in an init() function: toxiproxy.Register(SomeToxic) (this will have to be done with reflection or something...)

This will allow user-created toxics without having to create a full fork of toxiproxy.

[pushrax]

What's the plan for ensuring the toxic has finished sending output after being interrupted, and before the connections are changed? Wait for Pipe() to return?

Also 👍 for the API. Have you thought about allowing control of the toxic ordering? That might be useful in some cases, but maybe not enough to be worth the extra complexity.

[xthexder]

stub.Interrupt() will not return until Pipe() has returned now, so we are guaranteed that either the data is sent or it won't be until we resume.

Another thing for the implementation:
I'd like to wrap toxics in a struct with some extra fields (like name) so that we can simplify the Toxic interface to just the Pipe() function.
This means much less boilerplate code in the toxic definitions.

[pushrax]

🆒

[sirupsen]

Wouldn't it be more restful to do:

POST /proxies/{proxy}/{up|down}stream/toxics

But only allow certain names?

[xthexder]

👍 @sirupsen That means it also exactly matches how proxies are created.

[eapache]

👍 to the API and design

[sirupsen]

Another interpretation to the API would be that all the toxics are created on the resource by default, and you have to do a PUT to update them. Would that make more sense?

[xthexder]

Missed this before (maybe because it was edited in after): toxic order is determined by the order toxics are created in, since we'll always append to the end of the chain. In the general use case, this works well since I don't see a need to reorder toxics without removing them all first.

I think the idea of adding/removing toxics vs enabling/disabling is what we want. It seems to be what people are expecting, and it will scale better if/when we have many types of toxics (I expect there will end up being many protocol-aware toxics)

[pushrax]

@sirupsen PUT semantics are to replace the whole object, while in this case it would be more convenient to do an incremental update.

[ihsw]

👍

[sirupsen]

We need to remember to remove the toxiproxy binary that doesn't have a -server suffix.