hashicorp / consul

Consul is a distributed, highly available, and data center aware solution to connect and configure applications across dynamic, distributed infrastructure.
https://www.consul.io
Other
28.45k stars 4.43k forks source link

Consul Connect Envoy request mirroring #9827

Open chuckyz opened 3 years ago

chuckyz commented 3 years ago

Feature Description

I would like to enable Envoy's request mirroring onto listeners (LDS) for silent traffic mirroring between clusters (CDS), while using Connect's XDS provider.

Use Case(s)

Note

I am more than willing to PR this myself, I have looked through the Connect XDS codebase a bit but I am not confident on where exactly this code should go. If there are any pointers I will attempt to kick-start a PR on this.

Edit: As a small note "envoy request mirroring" brings up this blog post: https://blog.markvincze.com/shadow-mirroring-with-envoy/ -- from a requirements perspective I want to state that what we need is just about everything that's covered there. We could even cut out the % based traffic and just have "blast service (x) with 100% traffic, go for it."

freddygv commented 3 years ago

Hi @chuckyz,

I took a look at the required Envoy configuration and this that this kind of setting could make sense in a few places:

  1. In our service-splitter definition. So like how there is currently an array of Splits, there would be an additional array of mirror policies with configurable weights.
  2. In a new service-mirror config entry, since mirroring is similar to splitting but doesn't actually reduce the number of requests to another service.
  3. As a destination in a service-router. This would allow users to apply the mirror policies to individual routes, but would make it more verbose to apply to all routes. Though this is how Envoy exposes it as well, since it is configured on route actions.

I think I lean toward #3 because options 1 and 2 prevent users from specifying these policies on individual routes.

What do you think? Were you looking to add this config on individual routes?

Also, there's one thing I'm wondering about. request_mirroring_policies are configured on route actions alongside weighted_clusters, which are used for traffic splitting. So what happens if both are specified? If you're splitting 50/50 between two versions service, v1 and v2, is it fine to add a 10% shadow policy to some other set of endpoints? Or is there an error when the config is loaded?

Andy-Gong commented 3 years ago

Hi, @freddygv

We also have the same requirement which mirrors all requests to another service.

In #3, is there any doc saying how to config the mirror policies in a consul service route? I go through consul docs, but do not find how to do this.

chuckyz commented 3 years ago

@freddygv I'm keen on number 3, I favor explicitness over "easy."

I genuinely don't know about request mirroring and weighted clusters. One easy way to find out.... :)

For our use-case we'll want "10% shadow, period" I believe, e.g. some kind of "for all traffic to baz, send 10% to foo."

rajatvig commented 2 years ago

Is there any progress on this feature?

chrisboulton commented 2 years ago

@freddygv I have a pass of this that I put together the other night sitting out in a fork, it seems to work well and I'd love to tidy it up and contribute back. Before I do, I'd love your thoughts on how you'd like the mirroring to be configured. I've got the following so far: (going with #3)

Kind = "service-router"
Name = "catalog"
Routes = [
  {
    ..
    Destination {
        Service = "catalog"
        MirrorPolicy {
            Service = "catalog-testing"
            # ServiceSubset, Namespace, Partition all optional but configurable

            # Envoy lets you confiture a numerator and a denominator
            # I've just left this as the numerator for now (and the default denominator of 100)
            # Should we allow this to be configurable to the same extent?
            Percent = 25

        }
    }
  }
]
zackangelo commented 2 years ago

@blake did you have a chance to check out this proposal for mirroring? If there's anything I can do to help push this forward let me know!

Amier3 commented 2 years ago

Sorry @zackangelo looks like this fell through the cracks a bit, would like to revive this issue if possible.

@chrisboulton your approach looks good if you're still interested in working on this issue. For now just allowing the numerator to be set sounds good. Was the previous comment by Freddy enough context for you to start working o this?

shubhamsre commented 2 years ago

Can we please have any updates on it?

https://istio.io/latest/docs/tasks/traffic-management/mirroring/

It's already part of istio from a long time.

shubhamsre commented 2 years ago

@chrisboulton @freddygv ^^

chrisboulton commented 2 years ago

@shubhamsre Feel free to take the commit I have in our fork of Consul here, and build it into a version you wish to deploy if you want to test or mess with the functionality: https://github.com/bigcommerce/consul/commit/c20c1d499673d32f08234c7ba390299c862beb48. We're yet to deploy it into a real environment, so I haven't tidied it up and contributed it upstream as yet.