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

hlts2 · 2022-05-20T08:40:11Z

Description:

WHAT

I have added the logic to be able to get backoff metrics.
This allows collection of backoff metrics (retry count) within each RPC request when observability and backoff are enabled

WHY

We are in the process of implementing CircuitBreaker, but first we need to get proper metrics on the backoffs that occur inside RPC requests.

Related Issue:

How Has This Been Tested?:

Environment:

Go Version: 1.18.2
Docker Version: 20.10.8
Kubernetes Version: 1.22.0
NGT Version: 1.14.3

Types of changes:

Bug fix [type/bug]
New feature [type/feature]
Add tests [type/test]
Security related changes [type/security]
Add documents [type/documentation]
Refactoring [type/refactoring]
Update dependencies [type/dependency]
Update benchmarks and performances [type/bench]
Update CI [type/ci]

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you successfully ran tests with your changes locally?

Checklist:

I have read the CONTRIBUTING document.
I have checked open Pull Requests for the similar feature or fixes?
I have added tests and benchmarks to cover my changes.
I have ensured all new and existing tests passed.
I have commented my code, particularly in hard-to-understand areas
I have updated the documentation accordingly.

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

vdaas-ci · 2022-05-20T08:42:10Z

[CHATOPS:HELP] ChatOps commands.

🙆‍♀️ /approve - approve
💌 /changelog - replace the PR body by changelog details
🍱 /format - format codes and add licenses
✅ /gen-test - generate test codes
🏷️ /label - add labels
⏪ /rebase - rebase master
🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

codecov · 2022-05-20T08:58:22Z

Codecov Report

Merging #1666 (e92afd7) into master (46334b2) will decrease coverage by 0.11%.
The diff coverage is 6.45%.

@@            Coverage Diff             @@
##           master    #1666      +/-   ##
==========================================
- Coverage   31.20%   31.09%   -0.12%     
==========================================
  Files         372      374       +2     
  Lines       32502    32602     +100     
==========================================
- Hits        10142    10136       -6     
- Misses      21982    22081      +99     
- Partials      378      385       +7

Impacted Files	Coverage Δ
internal/client/v1/client/discoverer/discover.go	`0.00% <0.00%> (ø)`
internal/config/backoff.go	`63.88% <0.00%> (-36.12%)`	⬇️
internal/net/grpc/client.go	`0.00% <0.00%> (ø)`
internal/net/grpc/context.go	`0.00% <0.00%> (ø)`
internal/net/http/transport/roundtrip_mock.go	`50.00% <0.00%> (-16.67%)`	⬇️
pkg/gateway/lb/handler/grpc/handler.go	`0.00% <0.00%> (ø)`
pkg/gateway/lb/usecase/vald.go	`0.00% <0.00%> (ø)`
pkg/manager/index/service/indexer.go	`0.00% <0.00%> (ø)`
pkg/manager/index/usecase/indexer.go	`0.00% <0.00%> (ø)`
internal/backoff/backoff.go	`74.25% <11.11%> (-13.70%)`	⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3f0823...e92afd7. Read the comment docs.

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions · 2022-05-23T09:53:43Z

internal/observability/metrics/backoff/backoff.go

+	return []metrics.Measurement{}, nil
+}
+
+func (bm *backoffMetrics) MeasurementWithTags(ctx context.Context) (mts []metrics.MeasurementWithTags, err error) {


🚫 [golangci] _{reported by reviewdog 🐶}
named return "mts" with type "[]metrics.MeasurementWithTags" found (nonamedreturns)

internal/backoff/backoff.go

internal/observability/metrics/backoff/backoff.go

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 · 2022-05-24T02:54:16Z

internal/runner/runner_race_test.go

@@ -31,7 +31,7 @@ import (
 	"github.com/vdaas/vald/internal/test/goleak"
 )

-func TestDo(t *testing.T) {
+func TestDo_for_race(t *testing.T) {


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

internal/observability/metrics/backoff/backoff.go

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

vdaas-ci · 2022-05-24T09:35:13Z

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

internal/observability/metrics/backoff/backoff.go

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

internal/observability/metrics/backoff/backoff.go

github-actions · 2022-05-30T05:25:12Z

internal/observability/metrics/backoff/backoff.go

+	"github.com/vdaas/vald/internal/observability/metrics"
+)
+
+type backoffMetrics struct {


🚫 [golangci] _{reported by reviewdog 🐶}
fieldalignment: struct with 48 pointer bytes could be 32 (govet)

pkg/gateway/lb/usecase/vald.go

internal/backoff/backoff.go

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 · 2022-05-31T02:03:33Z

pkg/manager/index/service/indexer.go

-				_, err = idx.client.GetClient().Do(ctx, addr, func(ctx context.Context, conn *grpc.ClientConn, copts ...grpc.CallOption) (_ interface{}, err error) {
-					return agent.NewAgentClient(conn).SaveIndex(ctx, &payload.Empty{}, copts...)
-				})
+				_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "v1.vald.SaveIndex"), addr,


Suggested change

_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "v1.vald.SaveIndex"), addr,

_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "vald.v1.SaveIndex"), addr,

Suggested change

_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "v1.vald.SaveIndex"), addr,

_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "v1.vald.SaveIndex"), addr,

pkg/manager/index/service/indexer.go

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions · 2022-06-01T03:39:27Z

internal/observability/metrics/backoff/backoff.go

@@ -0,0 +1,82 @@
+//


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

github-actions · 2022-06-01T03:39:27Z

internal/backoff/context.go

@@ -0,0 +1,35 @@
+//


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

hlts2 · 2022-06-06T06:51:34Z

I will close this PR. Because the following PR was merged to master.

#1684

add logic to get backoff metrics

421501d

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot added area/internal size/M type/feature New feature labels May 20, 2022

github-actions bot added the priority/medium label May 20, 2022

hlts2 added 5 commits May 23, 2022 14:26

add backoff metrics view

e98669a

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

rename variable name

3320531

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

add logic to set value into the context

57fb78c

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix bug of context value

6f549bb

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

add option for enable/disable metrics

9315c0e

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot added size/L and removed size/M labels May 23, 2022

fix fails test

2a98372

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot added the team/set SET team label May 23, 2022

hlts2 self-assigned this May 23, 2022

fix deepsource warning

88b2cce

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot reviewed May 23, 2022

View reviewed changes

hlts2 added 2 commits May 24, 2022 10:35

Merge branch 'master' into feature/add-backoff-metrics

307e271

fix golangcilint warning

c432624

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 commented May 24, 2022

View reviewed changes

github-actions bot reviewed May 24, 2022

View reviewed changes

internal/observability/metrics/backoff/backoff.go Outdated Show resolved Hide resolved

hlts2 added 2 commits May 24, 2022 18:20

Merge branch 'master' into feature/add-backoff-metrics

eb6f54a

add field of values for backoff metrics

2893de0

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot added the area/helm label May 24, 2022

hlts2 added 2 commits May 24, 2022 18:28

make helm/schema/vald & make helm/schema/crd/vald

66009b6

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

add metrics configuration

d4a4cbd

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

refactor and added logic to get metrics

0a7133d

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 added 6 commits May 26, 2022 20:33

add debug log

5c7af01

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix bug of map initialize logic

00d9988

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

change from Count to lastValue

f98545a

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

change to sum aggregation

012b8b3

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix backoff metrics type

cbffcda

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix aggregation type

ed8ff25

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 commented May 30, 2022

View reviewed changes

internal/observability/metrics/backoff/backoff.go Outdated Show resolved Hide resolved

hlts2 added 2 commits May 30, 2022 13:28

Update internal/observability/metrics/backoff/backoff.go

1fbe737

deleted import path

acc4711

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 requested review from kpango and datelier May 30, 2022 04:52

hlts2 changed the title ~~[WIP] Add logic to get backoff metrics~~ Add logic to get backoff metrics May 30, 2022

github-actions bot reviewed May 30, 2022

View reviewed changes

hlts2 marked this pull request as ready for review May 30, 2022 07:25

hlts2 added 6 commits May 30, 2022 17:20

fix backoff counting point

5003cbb

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix lint warning

1a90ecc

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

deleted ctxkey package

e4fc27f

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

deleted space

d1c5e7e

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

store grpc method name to context

e1dc9d5

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix countup bug

a925c68

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

hlts2 commented May 31, 2022

View reviewed changes

pkg/manager/index/service/indexer.go Outdated Show resolved Hide resolved

hlts2 added 2 commits May 31, 2022 11:03

Update pkg/manager/index/service/indexer.go

6d5261b

make format

e92afd7

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot reviewed Jun 1, 2022

View reviewed changes

hlts2 changed the title ~~Add logic to get backoff metrics~~ [DONT MERGE] Add logic to get backoff metrics Jun 2, 2022

hlts2 mentioned this pull request Jun 2, 2022

Add backoff metrics for grpc #1684

Merged

18 tasks

hlts2 closed this Jun 6, 2022

hlts2 deleted the feature/add-backoff-metrics branch June 6, 2022 06:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hlts2 commented May 20, 2022 •

edited

Loading

vdaas-ci commented May 20, 2022

codecov bot commented May 20, 2022 •

edited

Loading

github-actions bot May 23, 2022

hlts2 May 24, 2022 •

edited

Loading

vdaas-ci commented May 24, 2022

github-actions bot May 30, 2022

hlts2 May 31, 2022

github-actions bot Jun 1, 2022

github-actions bot Jun 1, 2022

hlts2 commented Jun 6, 2022

	_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "v1.vald.SaveIndex"), addr,
	_, err = idx.client.GetClient().Do(grpc.WithGRPCMethod(ctx, "vald.v1.SaveIndex"), addr,

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

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

Conversation

hlts2 commented May 20, 2022 • edited Loading

Description:

WHAT

WHY

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

vdaas-ci commented May 20, 2022

codecov bot commented May 20, 2022 • edited Loading

Codecov Report

github-actions bot May 23, 2022

Choose a reason for hiding this comment

hlts2 May 24, 2022 • edited Loading

Choose a reason for hiding this comment

vdaas-ci commented May 24, 2022

github-actions bot May 30, 2022

Choose a reason for hiding this comment

hlts2 May 31, 2022

Choose a reason for hiding this comment

github-actions bot Jun 1, 2022

Choose a reason for hiding this comment

github-actions bot Jun 1, 2022

Choose a reason for hiding this comment

hlts2 commented Jun 6, 2022

hlts2 commented May 20, 2022 •

edited

Loading

codecov bot commented May 20, 2022 •

edited

Loading

hlts2 May 24, 2022 •

edited

Loading