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 8 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
30 changes: 29 additions & 1 deletion internal/backoff/backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,29 @@ type backoff struct {
maxRetryCount int
backoffTimeLimit time.Duration
errLog bool
metricsEnabled bool
metrics sync.Map
}

// 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]int
Close()
}

const traceTag = "vald/internal/backoff/Backoff.Do/retry"
type contextKey string

const (
traceTag = "vald/internal/backoff/Backoff.Do/retry"

serviceContextKey contextKey = "service"
)

// WithServiceContext returns a copy of parent in which the value associated with key (serviceContextKey).
func WithServiceContext(ctx context.Context, svc string) context.Context {
return context.WithValue(ctx, serviceContextKey, svc)
}

// New creates the new backoff with option.
func New(opts ...Option) Backoff {
Expand Down Expand Up @@ -110,6 +124,11 @@ func (b *backoff) Do(ctx context.Context, f func(ctx context.Context) (val inter
}()
return f(ssctx)
}()

if svc := ctx.Value(serviceContextKey); svc != nil && b.metricsEnabled {
b.metrics.Store(svc.(string), cnt+1)
}

if !ret {
return res, err
}
Expand Down Expand Up @@ -149,6 +168,15 @@ func (b *backoff) addJitter(dur float64) float64 {
return dur + float64(rand.LimitedUint32(uint64(hd))) - hd
}

func (b *backoff) Metrics(_ context.Context) (m map[string]int) {
hlts2 marked this conversation as resolved.
Show resolved Hide resolved
b.metrics.Range(func(key, value any) bool {
b.metrics.Delete(key)
m[key.(string)] = value.(int)
return true
})
return m
}

// Close wait for the backoff process to complete.
func (b *backoff) Close() {
b.wg.Wait()
Expand Down
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
}
}
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() == "Map" {
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]int
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]int {
return bm.MetricsFunc(ctx)
}

func (bm *backoffMock) Close() {
bm.CloseFunc()
}
59 changes: 59 additions & 0 deletions internal/observability/metrics/backoff/backoff.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package backoff
hlts2 marked this conversation as resolved.
Show resolved Hide resolved

import (
"context"

"github.com/vdaas/vald/internal/backoff"
"github.com/vdaas/vald/internal/observability/metrics"
)

var (
serviceNameKey = metrics.MustNewKey("grpc_service")
)

type backoffMetrics struct {
hlts2 marked this conversation as resolved.
Show resolved Hide resolved
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 🐶
fieldalignment: struct with 48 pointer bytes could be 32 (govet)

bo backoff.Backoff
retryCount metrics.Int64Measure
}

func New(bo backoff.Backoff) metrics.Metric {
return &backoffMetrics{
bo: bo,
retryCount: *metrics.Int64(
metrics.ValdOrg+"/grpc/backoff/retry_count",
"Backoff retry count",
metrics.UnitDimensionless),
}
}

func (*backoffMetrics) Measurement(_ context.Context) ([]metrics.Measurement, error) {
return []metrics.Measurement{}, nil
}

func (bm *backoffMetrics) MeasurementWithTags(ctx context.Context) (mts []metrics.MeasurementWithTags, err error) {
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 🐶
named return "mts" with type "[]metrics.MeasurementWithTags" found (nonamedreturns)

ms := bm.bo.Metrics(ctx)
mts = make([]metrics.MeasurementWithTags, 0, len(ms))
for svc, cnt := range ms {
mts = append(mts, metrics.MeasurementWithTags{
Measurement: bm.retryCount.M(int64(cnt)),
Tags: map[metrics.Key]string{
serviceNameKey: svc,
},
})
}
return mts, nil
}

func (bm *backoffMetrics) View() []*metrics.View {
return []*metrics.View{
{
Name: "backoff_retry_count",
Description: bm.retryCount.Description(),
Measure: &bm.retryCount,
TagKeys: []metrics.Key{
serviceNameKey,
},
Aggregation: metrics.LastValue(),
},
}
}
2 changes: 1 addition & 1 deletion internal/runner/runner_race_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
"github.com/vdaas/vald/internal/test/goleak"
)

func TestDo(t *testing.T) {
func TestDo_for_race(t *testing.T) {
Copy link
Collaborator Author

@hlts2 hlts2 May 24, 2022

Choose a reason for hiding this comment

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

I have fixed function name because of CI fails.
The reason why CI fails was because there was a same function in to the same package.

https://github.com/vdaas/vald/blob/master/internal/runner/runner_test.go#L284

type args struct {
ctx context.Context
opts []Option
Expand Down