Unleash / unleash-proxy

Unleash Proxy is used to safely integrate frontend application with Unleash in a secure and scaleable way.
https://docs.getunleash.io/sdks/unleash-proxy
Apache License 2.0
53 stars 43 forks source link

Support for refresh interval jitter #108

Closed Meemaw closed 1 year ago

Meemaw commented 1 year ago

Describe the feature request

Add support to configure refresh interval jitter.

Background

Having all proxy instances send metrics at the same time can put uneven load on the server, that would much easier to handle if there was a jitter.

Solution suggestions

Add a refreshIntervalJitter config option.

ivarconr commented 1 year ago

Have you seen this as an actual problem today? Could you share some traffic charts @Meemaw?

Our experience running the proxy for customers in Unleash Cloud is that they usually will never start at the same time, just because of how the scheduler will provision instances.

But, I think actually the main problem is to not have them start sending metrics at the same time, so the first time metrics are sent should actually be random number in the defined metrics interval.

What do you think?

Meemaw commented 1 year ago

Actually, it seems posting metrics is the main issue. Here is a chart

Screenshot 2022-12-13 at 10 48 25
thomasheartman commented 1 year ago

Hey! Sorry for the long delay here 🙋

Yeah, we agree that this sounds like a sensible thing to add. Whether it'd be better to add jitter only for the first send and then send at regular intervals thereafter or to add jitter for every request is still uncertain, but that can be discussed and decided.

Anyway, would you be willing to make a PR for this? We'd be happy to provide guidance, of course 😄

Meemaw commented 1 year ago

@thomasheartman this would have to be implemented in each client separately, right? For proxy in particular, that would be node client.

guscost-opensea commented 1 year ago

I think if we only jitter on the initial metrics startup, we would only have to update the unleash-proxy package. PR above.

thomasheartman commented 1 year ago

@Meemaw @guscost-opensea :

Yeah, if you only add it to the startup, then it should suffice to only update the proxy, but as you mentioned in the PR: it might make more sense to add jitter for every request. That would require updates both to the Node SDK and to the proxy.

ivarconr commented 1 year ago

Node SDK v3.18.0-beta.0 introduced support for jitter on the metrics interval.

We need to be mindful that the proxy customize the metrics sligthly (see code)

In addition we need to expose the same jitter option in the proxy (also as an environment variable).

ivarconr commented 1 year ago

Released as part of v0.14.0-beta.0.

Do you mind testing it a bit @Meemaw ?

Meemaw commented 1 year ago

@ivarconr We still use fairly old version of proxy, mainly due to https://github.com/Unleash/unleash-proxy/issues/86