search

envoyproxy / ratelimit

Go/gRPC service designed to enable generic rate limit scenarios from different types of applications.
Apache License 2.0
2.21k stars 428 forks source link

fix(dogstatsd_sink): switch from pure map to list of key/value pairs #627

Closed JDeuce closed 2 weeks ago

JDeuce commented 2 weeks ago

Flaky build was observed in this run https://github.com/envoyproxy/ratelimit/actions/runs/9600573727/job/26477125368

--- FAIL: TestLoadMogrifiersFromEnv (0.00s)
    --- FAIL: TestLoadMogrifiersFromEnv/Two_mogrifiers:_First_match (0.00s)
        mogrifier_map_test.go:146: 
                Error Trace:    /workdir/src/godogstats/mogrifier_map_test.go:146
                Error:          Not equal: 
                                expected: "custom.foo"
                                actual  : "ratelimit.service.rate_limit.foo"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -custom.foo
                                +ratelimit.service.rate_limit.foo
                Test:           TestLoadMogrifiersFromEnv/Two_mogrifiers:_First_match

The test for Two mogrifiers: First match has flaky results because the underlying iteration order from the map is not guaranteed. Sometimes matching first and sometimes matching second mogrifier.

Having a consistent mogrifier order is required for some types of priority based manipulation, and that is how the keys are defined in the environment, so changing the overall implementation to a slice to maintain ordered iteration is the best option. And can be done by only changing the internal private types.

Repro'd originally bug on main and verified results are no longer flaky by running several iterations of the tests:

go test -timeout 30s -count=5000 github.com/envoyproxy/ratelimit/src/godogstats

See https://go.dev/blog/maps

Iteration order When iterating over a map with a range loop, the iteration order is not specified and is not guaranteed to be the same from one iteration to the next. If you require a stable iteration order you must maintain a separate data structure that specifies that order.