Skip to content
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

[DONT MERGE] Add logic to get backoff metrics #1666

Closed
wants to merge 44 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
421501d
add logic to get backoff metrics
hlts2 May 20, 2022
e98669a
add backoff metrics view
hlts2 May 23, 2022
3320531
rename variable name
hlts2 May 23, 2022
57fb78c
add logic to set value into the context
hlts2 May 23, 2022
6f549bb
fix bug of context value
hlts2 May 23, 2022
9315c0e
add option for enable/disable metrics
hlts2 May 23, 2022
2a98372
fix fails test
hlts2 May 23, 2022
88b2cce
fix deepsource warning
hlts2 May 23, 2022
307e271
Merge branch 'master' into feature/add-backoff-metrics
hlts2 May 24, 2022
c432624
fix golangcilint warning
hlts2 May 24, 2022
eb6f54a
Merge branch 'master' into feature/add-backoff-metrics
hlts2 May 24, 2022
2893de0
add field of values for backoff metrics
hlts2 May 24, 2022
66009b6
make helm/schema/vald & make helm/schema/crd/vald
hlts2 May 24, 2022
d4a4cbd
add metrics configuration
hlts2 May 24, 2022
0a7133d
refactor and added logic to get metrics
hlts2 May 24, 2022
7fcbe9d
add logic to get backoff metrics
hlts2 May 25, 2022
9534597
add grpc method propagation logic
hlts2 May 25, 2022
db7bd9d
fix nil pointer bug
hlts2 May 25, 2022
bf913f9
fix lint warning
hlts2 May 26, 2022
d94cdae
Merge branch 'master' into feature/add-backoff-metrics
hlts2 May 26, 2022
702ece8
fix lint warning
hlts2 May 26, 2022
999dde4
fix deepsource error
hlts2 May 26, 2022
54f3ab3
fix metrics view creation error
hlts2 May 26, 2022
014b128
fix aggregation method
hlts2 May 26, 2022
ea9e40e
fix aggregation type
hlts2 May 26, 2022
66e5eac
change from map delete to zero store
hlts2 May 26, 2022
e1e0e27
Revert "change from map delete to zero store"
hlts2 May 26, 2022
f4899ef
zero store and fix aggregation method
hlts2 May 26, 2022
5c7af01
add debug log
hlts2 May 26, 2022
00d9988
fix bug of map initialize logic
hlts2 May 26, 2022
f98545a
change from Count to lastValue
hlts2 May 26, 2022
012b8b3
change to sum aggregation
hlts2 May 27, 2022
cbffcda
fix backoff metrics type
hlts2 May 30, 2022
ed8ff25
fix aggregation type
hlts2 May 30, 2022
1fbe737
Update internal/observability/metrics/backoff/backoff.go
hlts2 May 30, 2022
acc4711
deleted import path
hlts2 May 30, 2022
5003cbb
fix backoff counting point
hlts2 May 30, 2022
1a90ecc
fix lint warning
hlts2 May 30, 2022
e4fc27f
deleted ctxkey package
hlts2 May 30, 2022
d1c5e7e
deleted space
hlts2 May 30, 2022
e1dc9d5
store grpc method name to context
hlts2 May 30, 2022
a925c68
fix countup bug
hlts2 May 30, 2022
6d5261b
Update pkg/manager/index/service/indexer.go
hlts2 May 31, 2022
e92afd7
make format
hlts2 Jun 1, 2022
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
20 changes: 20 additions & 0 deletions charts/vald-helm-operator/crds/valdrelease.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -1284,6 +1286,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -2152,6 +2156,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -4193,6 +4199,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -4357,6 +4365,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -4516,6 +4526,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -5648,6 +5660,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -5804,6 +5818,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -6956,6 +6972,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down Expand Up @@ -7112,6 +7130,8 @@ spec:
type: string
enable_error_log:
type: boolean
enable_metrics:
type: boolean
initial_duration:
type: string
jitter_limit:
Expand Down
2 changes: 1 addition & 1 deletion charts/vald/values.schema.json

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions charts/vald/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,9 @@ defaults:
# @schema {"name": "defaults.grpc.client.backoff.enable_error_log", "type": "boolean"}
# defaults.grpc.client.backoff.enable_error_log -- gRPC client backoff log enabled
enable_error_log: true
# @schema {"name": "defaults.grpc.client.backoff.enable_metrics", "type": "boolean"}
# defaults.grpc.client.backoff.enable_metrics -- gRPC client backoff metrics enabled
enable_metrics: false
# @schema {"name": "defaults.grpc.client.call_option", "type": "object"}
call_option:
# @schema {"name": "defaults.grpc.client.wait_for_ready", "type": "boolean"}
Expand Down
27 changes: 27 additions & 0 deletions internal/backoff/backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,16 @@ type backoff struct {
maxRetryCount int
backoffTimeLimit time.Duration
errLog bool
metricsEnabled bool

mu sync.RWMutex
metrics map[string]int64
}

// Backoff represents an interface to handle backoff operation.
type Backoff interface {
Do(context.Context, func(ctx context.Context) (interface{}, bool, error)) (interface{}, error)
Metrics(ctx context.Context) map[string]int64
Close()
}

Expand All @@ -62,6 +67,7 @@ func New(opts ...Option) Backoff {
}
b.durationLimit = b.maxDuration / b.backoffFactor
b.jittedInitialDuration = b.addJitter(b.initialDuration)
b.metrics = make(map[string]int64)

return b
}
Expand All @@ -86,6 +92,7 @@ func (b *backoff) Do(ctx context.Context, f func(ctx context.Context) (val inter

dur := b.initialDuration
jdur := b.jittedInitialDuration
name := FromBackoffName(ctx)

dctx, cancel := context.WithDeadline(sctx, time.Now().Add(b.backoffTimeLimit))
defer cancel()
Expand Down Expand Up @@ -119,6 +126,12 @@ func (b *backoff) Do(ctx context.Context, f func(ctx context.Context) (val inter
if b.errLog {
log.Error(err)
}
// e.g. name = v1.vald.Exists/10.0.0.0 ...etc
if len(name) != 0 && b.metricsEnabled {
b.mu.Lock()
b.metrics[name] += 1
b.mu.Unlock()
}
timer.Reset(time.Duration(jdur))
select {
case <-dctx.Done():
Expand Down Expand Up @@ -149,6 +162,20 @@ func (b *backoff) addJitter(dur float64) float64 {
return dur + float64(rand.LimitedUint32(uint64(hd))) - hd
}

func (b *backoff) Metrics(_ context.Context) map[string]int64 {
b.mu.RLock()
defer b.mu.RUnlock()
if len(b.metrics) == 0 {
return nil
}

m := make(map[string]int64, len(b.metrics))
for name, cnt := range b.metrics {
m[name] = cnt
}
return m
}

// Close wait for the backoff process to complete.
func (b *backoff) Close() {
b.wg.Wait()
Expand Down
1 change: 1 addition & 0 deletions internal/backoff/backoff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func TestNew(t *testing.T) {
maxRetryCount: 50,
errLog: true,
durationLimit: float64(time.Hour) / 1.1,
metrics: make(map[string]int64),
},
checkFunc: func(got *backoff, want *backoff) error {
got.jittedInitialDuration, want.jittedInitialDuration = 1, 1
Expand Down
35 changes: 35 additions & 0 deletions internal/backoff/context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1000: package comment should be of the form "Package backoff ..." (stylecheck)

// Copyright (C) 2019-2022 vdaas.org vald team <[email protected]>
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
package backoff

import "context"

type contextKey string

const backoffNameContextKey contextKey = "backoff_name"

// WithBackoffName returns a copy of parent in which the method associated with key (backoffNameContextKey).
func WithBackoffName(ctx context.Context, name string) context.Context {
return context.WithValue(ctx, backoffNameContextKey, name)
}

// FromBackoffName returns the value associated with this context for key (backoffNameContextKey).
func FromBackoffName(ctx context.Context) string {
if val := ctx.Value(backoffNameContextKey); val != nil {
return val.(string)
}
return ""
}
15 changes: 15 additions & 0 deletions internal/backoff/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var defaultOptions = []Option{
WithBackOffFactor(1.5),
WithRetryCount(50),
WithEnableErrorLog(),
WithDisableMetrics(),
}

// WithInitialDuration returns the option to set the initial duration of backoff.
Expand Down Expand Up @@ -113,3 +114,17 @@ func WithDisableErrorLog() Option {
b.errLog = false
}
}

// WithEnableMetrics returns the option to set the enable for backoff metrics.
func WithEnableMetrics() Option {
return func(b *backoff) {
b.metricsEnabled = true
}
}

// WithDisableMetrics returns the option to set the disable for backoff metrics.
func WithDisableMetrics() Option {
return func(b *backoff) {
b.metricsEnabled = false
}
}
1 change: 1 addition & 0 deletions internal/client/v1/client/discoverer/discover.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ func (c *client) dnsDiscovery(ctx context.Context, ech chan<- error) (addrs []st
}

func (c *client) discover(ctx context.Context, ech chan<- error) (err error) {
ctx = grpc.WithGRPCMethod(ctx, "discoverer.v1.Discoverer/Nodes")
if c.dscClient == nil || (c.autoconn && c.client == nil) {
return errors.ErrGRPCClientNotFound
}
Expand Down
17 changes: 17 additions & 0 deletions internal/config/backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Backoff struct {
BackoffFactor float64 `json:"backoff_factor" yaml:"backoff_factor"`
RetryCount int `json:"retry_count" yaml:"retry_count"`
EnableErrorLog bool `json:"enable_error_log" yaml:"enable_error_log"`
EnableMetrics bool `json:"enable_metrics" yaml:"enable_metrics"`
}

// Bind binds the actual data from the Backoff receiver fields.
Expand Down Expand Up @@ -57,5 +58,21 @@ func (b *Backoff) Opts() []backoff.Option {
)
}

if b.EnableMetrics {
opts = append(opts,
backoff.WithEnableMetrics(),
)
}

return opts
}

func (b *Backoff) CreateBackoff() backoff.Backoff {
if len(b.InitialDuration) != 0 &&
b.RetryCount > 2 {
return backoff.New(
b.Opts()...,
)
}
return nil
}
3 changes: 3 additions & 0 deletions internal/net/grpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,9 @@ func (g *gRPCClient) do(ctx context.Context, p pool.Conn, addr string, enableBac
}
}()
if g.bo != nil && enableBackoff {
if method := FromGRPCMethod(sctx); len(method) != 0 {
sctx = backoff.WithBackoffName(ctx, method+"/"+addr)
}
data, err = g.bo.Do(sctx, func(ictx context.Context) (r interface{}, ret bool, err error) {
err = p.Do(func(conn *ClientConn) (err error) {
if conn == nil {
Expand Down
35 changes: 35 additions & 0 deletions internal/net/grpc/context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//
// Copyright (C) 2019-2022 vdaas.org vald team <[email protected]>
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
package grpc

import "context"

type contextKey string

const grpcMethodContextKey contextKey = "grpc_method"

// WithGRPCMethod returns a copy of parent in which the method associated with key (grpcMethodContextKey).
func WithGRPCMethod(ctx context.Context, method string) context.Context {
return context.WithValue(ctx, grpcMethodContextKey, method)
}

// FromGRPCMethod returns the value associated with this context for key (grpcMethodContextKey).
func FromGRPCMethod(ctx context.Context) string {
if v := ctx.Value(grpcMethodContextKey); v != nil {
return v.(string)
}
return ""
}
2 changes: 1 addition & 1 deletion internal/net/http/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ var (
comparator.AllowUnexported(http.Transport{}),
comparator.IgnoreFields(http.Transport{}, "idleLRU", "altProto", "TLSNextProto"),
comparator.Exporter(func(t reflect.Type) bool {
if t.Name() == "ert" || t.Name() == "backoff" {
if t.Name() == "ert" || t.Name() == "backoff" || t.Name() == "RWMutex" {
return true
}
return false
Expand Down
9 changes: 7 additions & 2 deletions internal/net/http/transport/roundtrip_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,19 @@ func (rm *roundTripMock) RoundTrip(req *http.Request) (*http.Response, error) {
}

type backoffMock struct {
DoFunc func(context.Context, func(context.Context) (interface{}, bool, error)) (interface{}, error)
CloseFunc func()
DoFunc func(context.Context, func(context.Context) (interface{}, bool, error)) (interface{}, error)
MetricsFunc func(ctx context.Context) map[string]int64
CloseFunc func()
}

func (bm *backoffMock) Do(ctx context.Context, fn func(context.Context) (interface{}, bool, error)) (interface{}, error) {
return bm.DoFunc(ctx, fn)
}

func (bm *backoffMock) Metrics(ctx context.Context) map[string]int64 {
return bm.MetricsFunc(ctx)
}

func (bm *backoffMock) Close() {
bm.CloseFunc()
}
Loading