-
Notifications
You must be signed in to change notification settings - Fork 459
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.
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
Metric refactor #242
Metric refactor #242
Changes from 7 commits
21f6348
ed9a52c
6465cf3
92758a1
4fd402c
51bf9d8
fceb8fd
c846e96
c4cbbed
38649fd
6813915
9cf8447
f3f2140
cb39a25
62b519e
56c3e52
d03eba2
a3b6752
feaa224
2d88c77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ import ( | |
|
||
pb_struct "github.com/envoyproxy/go-control-plane/envoy/extensions/common/ratelimit/v3" | ||
pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v3" | ||
stats "github.com/lyft/gostats" | ||
"github.com/envoyproxy/ratelimit/src/stats" | ||
logger "github.com/sirupsen/logrus" | ||
"golang.org/x/net/context" | ||
"gopkg.in/yaml.v2" | ||
|
@@ -39,8 +39,8 @@ type rateLimitDomain struct { | |
} | ||
|
||
type rateLimitConfigImpl struct { | ||
domains map[string]*rateLimitDomain | ||
statsScope stats.Scope | ||
domains map[string]*rateLimitDomain | ||
manager stats.Manager | ||
} | ||
|
||
var validKeys = map[string]bool{ | ||
|
@@ -53,30 +53,15 @@ var validKeys = map[string]bool{ | |
"requests_per_unit": true, | ||
} | ||
|
||
// Create new rate limit stats for a config entry. | ||
// @param statsScope supplies the owning scope. | ||
// @param key supplies the fully resolved key name of the entry. | ||
// @return new stats. | ||
func newRateLimitStats(statsScope stats.Scope, key string) RateLimitStats { | ||
ret := RateLimitStats{} | ||
ret.TotalHits = statsScope.NewCounter(key + ".total_hits") | ||
ret.OverLimit = statsScope.NewCounter(key + ".over_limit") | ||
ret.NearLimit = statsScope.NewCounter(key + ".near_limit") | ||
ret.OverLimitWithLocalCache = statsScope.NewCounter(key + ".over_limit_with_local_cache") | ||
ret.WithinLimit = statsScope.NewCounter(key + ".within_limit") | ||
return ret | ||
} | ||
|
||
// Create a new rate limit config entry. | ||
// @param requestsPerUnit supplies the requests per unit of time for the entry. | ||
// @param unit supplies the unit of time for the entry. | ||
// @param key supplies the fully resolved key name of the entry. | ||
// @param scope supplies the owning scope. | ||
// @param rlStats supplies the stats structure associated with the RateLimit | ||
// @return the new config entry. | ||
func NewRateLimit( | ||
requestsPerUnit uint32, unit pb.RateLimitResponse_RateLimit_Unit, key string, scope stats.Scope) *RateLimit { | ||
requestsPerUnit uint32, unit pb.RateLimitResponse_RateLimit_Unit, rlStats stats.RateLimitStats) *RateLimit { | ||
|
||
return &RateLimit{FullKey: key, Stats: newRateLimitStats(scope, key), Limit: &pb.RateLimitResponse_RateLimit{RequestsPerUnit: requestsPerUnit, Unit: unit}} | ||
return &RateLimit{FullKey: rlStats.String(), Stats: rlStats, Limit: &pb.RateLimitResponse_RateLimit{RequestsPerUnit: requestsPerUnit, Unit: unit}} | ||
} | ||
|
||
// Dump an individual descriptor for debugging purposes. | ||
|
@@ -104,10 +89,8 @@ func newRateLimitConfigError(config RateLimitConfigToLoad, err string) RateLimit | |
// @param config supplies the config file that owns the descriptor. | ||
// @param parentKey supplies the fully resolved key name that owns this config level. | ||
// @param descriptors supplies the YAML descriptors to load. | ||
// @param statsScope supplies the owning scope. | ||
func (this *rateLimitDescriptor) loadDescriptors( | ||
config RateLimitConfigToLoad, parentKey string, descriptors []yamlDescriptor, | ||
statsScope stats.Scope) { | ||
// @param manager that owns the stats.Scope. | ||
func (this *rateLimitDescriptor) loadDescriptors(config RateLimitConfigToLoad, parentKey string, descriptors []yamlDescriptor, manager stats.Manager) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: rename arg name manager ->statsManager |
||
|
||
for _, descriptorConfig := range descriptors { | ||
if descriptorConfig.Key == "" { | ||
|
@@ -138,8 +121,7 @@ func (this *rateLimitDescriptor) loadDescriptors( | |
} | ||
|
||
rateLimit = NewRateLimit( | ||
descriptorConfig.RateLimit.RequestsPerUnit, pb.RateLimitResponse_RateLimit_Unit(value), newParentKey, | ||
statsScope) | ||
descriptorConfig.RateLimit.RequestsPerUnit, pb.RateLimitResponse_RateLimit_Unit(value), manager.NewStats(newParentKey)) | ||
rateLimitDebugString = fmt.Sprintf( | ||
" ratelimit={requests_per_unit=%d, unit=%s}", rateLimit.Limit.RequestsPerUnit, | ||
rateLimit.Limit.Unit.String()) | ||
|
@@ -148,8 +130,7 @@ func (this *rateLimitDescriptor) loadDescriptors( | |
logger.Debugf( | ||
"loading descriptor: key=%s%s", newParentKey, rateLimitDebugString) | ||
newDescriptor := &rateLimitDescriptor{map[string]*rateLimitDescriptor{}, rateLimit} | ||
newDescriptor.loadDescriptors( | ||
config, newParentKey+".", descriptorConfig.Descriptors, statsScope) | ||
newDescriptor.loadDescriptors(config, newParentKey+".", descriptorConfig.Descriptors, manager) | ||
this.descriptors[finalKey] = newDescriptor | ||
} | ||
} | ||
|
@@ -229,24 +210,10 @@ func (this *rateLimitConfigImpl) loadConfig(config RateLimitConfigToLoad) { | |
|
||
logger.Debugf("loading domain: %s", root.Domain) | ||
newDomain := &rateLimitDomain{rateLimitDescriptor{map[string]*rateLimitDescriptor{}, nil}} | ||
newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.statsScope) | ||
newDomain.loadDescriptors(config, root.Domain+".", root.Descriptors, this.manager) | ||
this.domains[root.Domain] = newDomain | ||
} | ||
|
||
func (this *rateLimitConfigImpl) descriptorToKey(descriptor *pb_struct.RateLimitDescriptor) string { | ||
rateLimitKey := "" | ||
for _, entry := range descriptor.Entries { | ||
if rateLimitKey != "" { | ||
rateLimitKey += "." | ||
} | ||
rateLimitKey += entry.Key | ||
if entry.Value != "" { | ||
rateLimitKey += "_" + entry.Value | ||
} | ||
} | ||
return rateLimitKey | ||
} | ||
|
||
func (this *rateLimitConfigImpl) Dump() string { | ||
ret := "" | ||
for _, domain := range this.domains { | ||
|
@@ -268,13 +235,12 @@ func (this *rateLimitConfigImpl) GetLimit( | |
} | ||
|
||
if descriptor.GetLimit() != nil { | ||
rateLimitKey := domain + "." + this.descriptorToKey(descriptor) | ||
rateLimitKey := stats.DescriptorKey(domain, descriptor) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the reasoning behind moving There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to client package (where it is used currently). |
||
rateLimitOverrideUnit := pb.RateLimitResponse_RateLimit_Unit(descriptor.GetLimit().GetUnit()) | ||
rateLimit = NewRateLimit( | ||
descriptor.GetLimit().GetRequestsPerUnit(), | ||
rateLimitOverrideUnit, | ||
rateLimitKey, | ||
this.statsScope) | ||
this.manager.NewStats(rateLimitKey)) | ||
return rateLimit | ||
} | ||
|
||
|
@@ -316,9 +282,9 @@ func (this *rateLimitConfigImpl) GetLimit( | |
// @param stats supplies the stats scope to use for limit stats during runtime. | ||
// @return a new config. | ||
func NewRateLimitConfigImpl( | ||
configs []RateLimitConfigToLoad, statsScope stats.Scope) RateLimitConfig { | ||
configs []RateLimitConfigToLoad, manager stats.Manager) RateLimitConfig { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same as above, rename |
||
|
||
ret := &rateLimitConfigImpl{map[string]*rateLimitDomain{}, statsScope} | ||
ret := &rateLimitConfigImpl{map[string]*rateLimitDomain{}, manager} | ||
for _, config := range configs { | ||
ret.loadConfig(config) | ||
} | ||
|
@@ -329,9 +295,9 @@ func NewRateLimitConfigImpl( | |
type rateLimitConfigLoaderImpl struct{} | ||
|
||
func (this *rateLimitConfigLoaderImpl) Load( | ||
configs []RateLimitConfigToLoad, statsScope stats.Scope) RateLimitConfig { | ||
configs []RateLimitConfigToLoad, manager stats.Manager) RateLimitConfig { | ||
|
||
return NewRateLimitConfigImpl(configs, statsScope) | ||
return NewRateLimitConfigImpl(configs, manager) | ||
} | ||
|
||
// @return a new default config loader implementation. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,14 @@ package main | |
import ( | ||
"flag" | ||
"fmt" | ||
"github.com/envoyproxy/ratelimit/src/settings" | ||
"github.com/envoyproxy/ratelimit/src/stats" | ||
"io/ioutil" | ||
"os" | ||
"path/filepath" | ||
|
||
"github.com/envoyproxy/ratelimit/src/config" | ||
"github.com/lyft/gostats" | ||
gostats "github.com/lyft/gostats" | ||
) | ||
|
||
func loadConfigs(allConfigs []config.RateLimitConfigToLoad) { | ||
|
@@ -19,9 +21,9 @@ func loadConfigs(allConfigs []config.RateLimitConfigToLoad) { | |
os.Exit(1) | ||
} | ||
}() | ||
|
||
dummyStats := stats.NewStore(stats.NewNullSink(), false) | ||
config.NewRateLimitConfigImpl(allConfigs, dummyStats) | ||
settingStruct := settings.NewSettings() | ||
manager := stats.NewStatManager(gostats.NewStore(gostats.NewNullSink(), false), settingStruct) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: inline creation of settings here instead |
||
config.NewRateLimitConfigImpl(allConfigs, manager) | ||
} | ||
|
||
func main() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import ( | |
pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v3" | ||
"github.com/envoyproxy/ratelimit/src/assert" | ||
"github.com/envoyproxy/ratelimit/src/config" | ||
"github.com/envoyproxy/ratelimit/src/stats" | ||
"github.com/envoyproxy/ratelimit/src/utils" | ||
logger "github.com/sirupsen/logrus" | ||
"math" | ||
|
@@ -18,6 +19,7 @@ type BaseRateLimiter struct { | |
cacheKeyGenerator CacheKeyGenerator | ||
localCache *freecache.Cache | ||
nearLimitRatio float32 | ||
Manager stats.Manager | ||
} | ||
|
||
type LimitInfo struct { | ||
|
@@ -89,7 +91,7 @@ func (this *BaseRateLimiter) GetResponseDescriptorStatus(key string, limitInfo * | |
responseDescriptorStatus = this.generateResponseDescriptorStatus(pb.RateLimitResponse_OVER_LIMIT, | ||
limitInfo.limit.Limit, 0) | ||
|
||
checkOverLimitThreshold(limitInfo, hitsAddend) | ||
this.checkOverLimitThreshold(limitInfo, hitsAddend) | ||
|
||
if this.localCache != nil { | ||
// Set the TTL of the local_cache to be the entire duration. | ||
|
@@ -109,25 +111,26 @@ func (this *BaseRateLimiter) GetResponseDescriptorStatus(key string, limitInfo * | |
limitInfo.limit.Limit, limitInfo.overLimitThreshold-limitInfo.limitAfterIncrease) | ||
|
||
// The limit is OK but we additionally want to know if we are near the limit. | ||
checkNearLimitThreshold(limitInfo, hitsAddend) | ||
this.checkNearLimitThreshold(limitInfo, hitsAddend) | ||
limitInfo.limit.Stats.WithinLimit.Add(uint64(hitsAddend)) | ||
} | ||
return responseDescriptorStatus | ||
} | ||
|
||
func NewBaseRateLimit(timeSource utils.TimeSource, jitterRand *rand.Rand, expirationJitterMaxSeconds int64, | ||
localCache *freecache.Cache, nearLimitRatio float32, cacheKeyPrefix string) *BaseRateLimiter { | ||
localCache *freecache.Cache, nearLimitRatio float32, cacheKeyPrefix string, manager stats.Manager) *BaseRateLimiter { | ||
return &BaseRateLimiter{ | ||
timeSource: timeSource, | ||
JitterRand: jitterRand, | ||
ExpirationJitterMaxSeconds: expirationJitterMaxSeconds, | ||
cacheKeyGenerator: NewCacheKeyGenerator(cacheKeyPrefix), | ||
localCache: localCache, | ||
nearLimitRatio: nearLimitRatio, | ||
Manager: manager, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again this field name is quite generic, unless looking at the type, it would be hard to figure out where does this manager belong to |
||
} | ||
} | ||
|
||
func checkOverLimitThreshold(limitInfo *LimitInfo, hitsAddend uint32) { | ||
func (this *BaseRateLimiter) checkOverLimitThreshold(limitInfo *LimitInfo, hitsAddend uint32) { | ||
// Increase over limit statistics. Because we support += behavior for increasing the limit, we need to | ||
// assess if the entire hitsAddend were over the limit. That is, if the limit's value before adding the | ||
// N hits was over the limit, then all the N hits were over limit. | ||
|
@@ -140,12 +143,11 @@ func checkOverLimitThreshold(limitInfo *LimitInfo, hitsAddend uint32) { | |
|
||
// If the limit before increase was below the over limit value, then some of the hits were | ||
// in the near limit range. | ||
limitInfo.limit.Stats.NearLimit.Add(uint64(limitInfo.overLimitThreshold - | ||
utils.Max(limitInfo.nearLimitThreshold, limitInfo.limitBeforeIncrease))) | ||
limitInfo.limit.Stats.NearLimit.Add(uint64(limitInfo.overLimitThreshold - utils.Max(limitInfo.nearLimitThreshold, limitInfo.limitBeforeIncrease))) | ||
} | ||
} | ||
|
||
func checkNearLimitThreshold(limitInfo *LimitInfo, hitsAddend uint32) { | ||
func (this *BaseRateLimiter) checkNearLimitThreshold(limitInfo *LimitInfo, hitsAddend uint32) { | ||
if limitInfo.limitAfterIncrease > limitInfo.nearLimitThreshold { | ||
// Here we also need to assess which portion of the hitsAddend were in the near limit range. | ||
// If all the hits were over the nearLimitThreshold, then all hits are near limit. Otherwise, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: update function documentation with new parameter