Skip to content

Plugin statstore into runner #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 12 commits into from
Jan 10, 2020
Merged

Plugin statstore into runner #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2f381ee
Plugin statstore into runner
Jan 6, 2020
2b75b2c
update the port
Jan 6, 2020
86ab803
verify totalhits stats
Jan 7, 2020
d3dc1b6
add comments
Jan 7, 2020
61b695d
fix format
Jan 7, 2020
8bc2b0a
change to sleep 1s
Jan 7, 2020
916207e
update the port
Jan 7, 2020
a45fde1
update the port
Jan 7, 2020
9ee5637
undo the changes on the testBasicConfigLegacy
Jan 7, 2020
2c5f42a
add newline
Jan 7, 2020
f4a6399
change runner to be the class
Jan 10, 2020
483400a
fix format
Jan 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/server/server_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ func (server *server) Runtime() loader.IFace {
return server.runtime
}

func NewServer(name string, opts ...settings.Option) Server {
return newServer(name, opts...)
func NewServer(name string, store stats.Store, opts ...settings.Option) Server {
return newServer(name, store, opts...)
}

func newServer(name string, opts ...settings.Option) *server {
func newServer(name string, store stats.Store, opts ...settings.Option) *server {
s := settings.NewSettings()

for _, opt := range opts {
Expand All @@ -119,7 +119,7 @@ func newServer(name string, opts ...settings.Option) *server {
ret.debugPort = s.DebugPort

// setup stats
ret.store = stats.NewDefaultStore()
ret.store = store
ret.scope = ret.store.Scope(name)
ret.store.AddStatGenerator(stats.NewRuntimeStats(ret.scope.Scope("go")))

Expand Down
1 change: 1 addition & 0 deletions src/service_cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ package main
import "github.com/lyft/ratelimit/src/service_cmd/runner"

func main() {
runner := runner.NewRunner()
runner.Run()
}
18 changes: 16 additions & 2 deletions src/service_cmd/runner/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"net/http"
"time"

stats "github.com/lyft/gostats"

"github.com/coocood/freecache"

pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v2"
Expand All @@ -19,7 +21,19 @@ import (
logger "github.com/sirupsen/logrus"
)

func Run() {
type Runner struct {
statsStore stats.Store
}

func NewRunner() Runner {
return Runner{stats.NewDefaultStore()}
}

func (runner *Runner) GetStatsStore() stats.Store {
return runner.statsStore
}

func (runner *Runner) Run() {
s := settings.NewSettings()

logLevel, err := logger.ParseLevel(s.LogLevel)
Expand All @@ -29,7 +43,7 @@ func Run() {
logger.SetLevel(logLevel)
}

srv := server.NewServer("ratelimit", settings.GrpcUnaryInterceptor(nil))
srv := server.NewServer("ratelimit", runner.statsStore, settings.GrpcUnaryInterceptor(nil))

var perSecondPool redis.Pool
if s.RedisPerSecond {
Expand Down
23 changes: 19 additions & 4 deletions test/integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,20 @@ func newDescriptorStatusLegacy(
}
}

// TODO: Once adding the ability of stopping the server in the runner (https://github.com/lyft/ratelimit/issues/119),
// stop the server at the end of each test, thus we can reuse the grpc port among these integration tests.
func TestBasicConfig(t *testing.T) {
t.Run("WithoutPerSecondRedis", testBasicConfig("8083", "false", "0"))
t.Run("WithPerSecondRedis", testBasicConfig("8085", "true", "0"))
t.Run("WithoutPerSecondRedisWithLocalCache", testBasicConfig("8083", "false", "1000"))
t.Run("WithPerSecondRedisWithLocalCache", testBasicConfig("8085", "true", "1000"))
t.Run("WithoutPerSecondRedisWithLocalCache", testBasicConfig("18083", "false", "1000"))
Copy link
Contributor Author

@freedomljc freedomljc Jan 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the port, to avoid the statstore gets reused among those tests. It is related to the go routine(https://github.com/lyft/ratelimit/blob/2c5f42aaaa74d5818829ff1473a915edda3e456c/test/integration/integration_test.go#L222-L224) won't exit after the method ends.

Copy link
Member

@mattklein123 mattklein123 Jan 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Can you add a TODO and comment about this? We should fix this so everything gets cleaned up after each test method but we can do that later.

Copy link
Contributor Author

@freedomljc freedomljc Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just added TODO, and also created a github issue on it.

t.Run("WithPerSecondRedisWithLocalCache", testBasicConfig("18085", "true", "1000"))
}

func TestBasicTLSConfig(t *testing.T) {
t.Run("WithoutPerSecondRedisTLS", testBasicConfigAuthTLS("8087", "false", "0"))
t.Run("WithPerSecondRedisTLS", testBasicConfigAuthTLS("8089", "true", "0"))
t.Run("WithoutPerSecondRedisTLSWithLocalCache", testBasicConfigAuthTLS("8087", "false", "1000"))
t.Run("WithPerSecondRedisTLSWithLocalCache", testBasicConfigAuthTLS("8089", "true", "1000"))
t.Run("WithoutPerSecondRedisTLSWithLocalCache", testBasicConfigAuthTLS("18087", "false", "1000"))
t.Run("WithPerSecondRedisTLSWithLocalCache", testBasicConfigAuthTLS("18089", "true", "1000"))
}

func testBasicConfigAuthTLS(grpcPort, perSecond string, local_cache_size string) func(*testing.T) {
Expand Down Expand Up @@ -93,6 +95,7 @@ func testBasicBaseConfig(grpcPort, perSecond string, local_cache_size string) fu

local_cache_size_val, _ := strconv.Atoi(local_cache_size)
enable_local_cache := local_cache_size_val > 0
runner := runner.NewRunner()

go func() {
runner.Run()
Expand Down Expand Up @@ -128,6 +131,10 @@ func testBasicBaseConfig(grpcPort, perSecond string, local_cache_size string) fu
response)
assert.NoError(err)

// store.NewCounter returns the existing counter.
key1HitCounter := runner.GetStatsStore().NewCounter(fmt.Sprintf("ratelimit.service.rate_limit.basic.%s.total_hits", getCacheKey("key1", enable_local_cache)))
assert.Equal(1, int(key1HitCounter.Value()))

// Now come up with a random key, and go over limit for a minute limit which should always work.
r := rand.New(rand.NewSource(time.Now().UnixNano()))
randomInt := r.Int()
Expand All @@ -151,6 +158,8 @@ func testBasicBaseConfig(grpcPort, perSecond string, local_cache_size string) fu
newDescriptorStatus(status, 20, pb.RateLimitResponse_RateLimit_MINUTE, limitRemaining)}},
response)
assert.NoError(err)
key2HitCounter := runner.GetStatsStore().NewCounter(fmt.Sprintf("ratelimit.service.rate_limit.another.%s.total_hits", getCacheKey("key2", enable_local_cache)))
assert.Equal(i+1, int(key2HitCounter.Value()))
}

// Limit now against 2 keys in the same domain.
Expand Down Expand Up @@ -180,6 +189,11 @@ func testBasicBaseConfig(grpcPort, perSecond string, local_cache_size string) fu
newDescriptorStatus(status, 10, pb.RateLimitResponse_RateLimit_HOUR, limitRemaining2)}},
response)
assert.NoError(err)
key2HitCounter := runner.GetStatsStore().NewCounter(fmt.Sprintf("ratelimit.service.rate_limit.another.%s.total_hits", getCacheKey("key2", enable_local_cache)))
assert.Equal(i+26, int(key2HitCounter.Value()))

key3HitCounter := runner.GetStatsStore().NewCounter(fmt.Sprintf("ratelimit.service.rate_limit.another.%s.total_hits", getCacheKey("key3", enable_local_cache)))
assert.Equal(i+1, int(key3HitCounter.Value()))
}
}
}
Expand All @@ -205,6 +219,7 @@ func testBasicConfigLegacy(local_cache_size string) func(*testing.T) {
local_cache_size_val, _ := strconv.Atoi(local_cache_size)
enable_local_cache := local_cache_size_val > 0

runner := runner.NewRunner()
go func() {
runner.Run()
}()
Expand Down