small refactor circuitbreaker #1816

kpango · 2022-10-21T07:17:03Z

Signed-off-by: kpango kpango@vdaas.org

Description:

refactor circuitbreaker
update go dependencies
update otel version and fix vald code
- Refactor Prometheus exporter open-telemetry/opentelemetry-go#3239

Related Issue:

How Has This Been Tested?:

Environment:

Go Version: 1.19.2
Docker Version: 20.10.8
Kubernetes Version: 1.22.0
NGT Version: 1.14.8

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.

cloudflare-workers-and-pages · 2022-10-21T07:17:06Z

Deploying with Cloudflare Pages

Latest commit:	`e14bb31`
Status:	✅ Deploy successful!
Preview URL:	https://b6370216.vald.pages.dev
Branch Preview URL:	https://refactor-internal-circuitbre.vald.pages.dev

View logs

vdaas-ci · 2022-10-21T07:17:21Z

[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 main
🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

codecov · 2022-10-21T07:23:29Z

Codecov Report

Base: 50.23% // Head: 30.33% // Decreases project coverage by -19.90% ⚠️

Coverage data is based on head (e14bb31) compared to base (97eed29).
Patch coverage: 33.53% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1816       +/-   ##
===========================================
- Coverage   50.23%   30.33%   -19.91%     
===========================================
  Files         231      371      +140     
  Lines       13464    34033    +20569     
===========================================
+ Hits         6764    10323     +3559     
- Misses       6499    23288    +16789     
- Partials      201      422      +221

Impacted Files	Coverage Δ
cmd/tools/cli/loadtest/main.go	`0.00% <ø> (ø)`
hack/benchmark/assets/x1b/loader.go	`0.00% <ø> (ø)`
hack/benchmark/internal/assets/dataset.go	`0.00% <ø> (ø)`
hack/benchmark/internal/operation/insert.go	`0.00% <ø> (ø)`
hack/benchmark/internal/operation/operation.go	`0.00% <ø> (ø)`
hack/benchmark/internal/operation/option.go	`0.00% <ø> (ø)`
hack/benchmark/internal/operation/remove.go	`0.00% <ø> (ø)`
hack/benchmark/internal/operation/search.go	`0.00% <ø> (ø)`
hack/benchmark/internal/operation/util.go	`0.00% <ø> (ø)`
hack/license/gen/main.go	`0.00% <0.00%> (ø)`
... and 248 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: kpango <kpango@vdaas.org>

github-actions · 2022-10-21T08:07:26Z

[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/refactor%2Finternal%2Fcircuitbreaker/0d2ee27f768d10057357e3adf9bf22546f145092

vdaas-ci · 2022-10-21T08:14:50Z

Profile Report

type	vald-agent-ngt	vald-lb-gateway	vald-discoverer	vald-manager-index
cpu
heap

other images

github-actions · 2022-10-21T08:22:52Z

internal/circuitbreaker/breaker.go

 		if errors.As(err, &serr) {
 			b.success()
-			return nil, b.currentState(), serr.Unwrap()
+			return nil, b.currentState(), err.(*errors.ErrCircuitBreakerMarkWithSuccess).Unwrap()


🚫 [golangci] _{reported by reviewdog 🐶}
type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)

github-actions · 2022-10-21T08:22:53Z

internal/circuitbreaker/breaker.go

 		if errors.As(err, &igerr) {
-			return nil, b.currentState(), igerr.Unwrap()
+			return nil, b.currentState(), err.(*errors.ErrCircuitBreakerIgnorable).Unwrap()


🚫 [golangci] _{reported by reviewdog 🐶}
type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)

github-actions · 2022-10-21T08:22:53Z

internal/circuitbreaker/manager.go

 	if err != nil {
-		return nil, err
+		switch st {


🚫 [golangci] _{reported by reviewdog 🐶}
missing cases in switch of type State: StateUnknown (exhaustive)

github-actions · 2022-10-21T08:22:53Z

internal/circuitbreaker/breaker.go

@@ -39,10 +39,15 @@
 	closedRefreshExp      int64 // unix time
 }

+var (
+	serr  = new(errors.ErrCircuitBreakerMarkWithSuccess)


🚫 [golangci] _{reported by reviewdog 🐶}
serr is a global variable (gochecknoglobals)

github-actions · 2022-10-21T08:22:53Z

internal/circuitbreaker/breaker.go

@@ -39,10 +39,15 @@
 	closedRefreshExp      int64 // unix time
 }

+var (
+	serr  = new(errors.ErrCircuitBreakerMarkWithSuccess)
+	igerr = new(errors.ErrCircuitBreakerIgnorable)


🚫 [golangci] _{reported by reviewdog 🐶}
igerr is a global variable (gochecknoglobals)

github-actions · 2022-10-21T08:22:53Z

internal/circuitbreaker/manager.go

@@ -72,20 +72,41 @@
 		mu.Unlock()
 	}()

+	var br *breaker


🚫 [golangci] _{reported by reviewdog 🐶}
variable name 'br' is too short for the scope of its usage (varnamelen)

Signed-off-by: kpango <kpango@vdaas.org>

hlts2

LGTM

github-actions bot added area/internal size/L type/refactoring priority/low labels Oct 21, 2022

hlts2 previously approved these changes Oct 21, 2022

View reviewed changes

kpango dismissed hlts2’s stale review via 33078b6 October 21, 2022 07:23

kpango force-pushed the refactor/internal/circuitbreaker branch from 7215b3c to 33078b6 Compare October 21, 2022 07:23

kpango force-pushed the refactor/internal/circuitbreaker branch 2 times, most recently from 94d2ea6 to d6607ee Compare October 21, 2022 07:35

small refactor circuitbreaker and update otelprom to new version

b0232ed

Signed-off-by: kpango <kpango@vdaas.org>

kpango force-pushed the refactor/internal/circuitbreaker branch from d6607ee to b0232ed Compare October 21, 2022 07:56

github-actions bot added the team/set SET team label Oct 21, 2022

kpango added actions/e2e-deploy actions/fossa actions/e2e-chaos actions/e2e-profiling actions/e2e-max-dim labels Oct 21, 2022

kpango requested review from hlts2, vankichi and kevindiu October 21, 2022 08:10

kpango removed the actions/e2e-max-dim label Oct 21, 2022

github-actions bot added team/sre SRE team type/ci labels Oct 21, 2022

kpango added actions/e2e-max-dim and removed team/sre SRE team type/ci labels Oct 21, 2022

github-actions bot reviewed Oct 21, 2022

View reviewed changes

change max dimensiont test start bit

e14bb31

Signed-off-by: kpango <kpango@vdaas.org>

kpango force-pushed the refactor/internal/circuitbreaker branch from ad7cd99 to e14bb31 Compare October 21, 2022 08:31

kpango removed the actions/e2e-max-dim label Oct 21, 2022

github-actions bot added team/sre SRE team type/ci labels Oct 21, 2022

kpango added actions/e2e-max-dim and removed team/sre SRE team priority/low type/ci labels Oct 21, 2022

github-actions bot added the priority/low label Oct 21, 2022

vankichi approved these changes Oct 24, 2022

View reviewed changes

hlts2 approved these changes Oct 24, 2022

View reviewed changes

kpango merged commit 80c42fc into main Oct 24, 2022

kpango deleted the refactor/internal/circuitbreaker branch October 24, 2022 02:27

kpango mentioned this pull request Jan 19, 2023

[minor] Release v1.7.0 #1926

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

small refactor circuitbreaker #1816

small refactor circuitbreaker #1816

kpango commented Oct 21, 2022 •

edited

Loading

cloudflare-workers-and-pages bot commented Oct 21, 2022 •

edited

Loading

vdaas-ci commented Oct 21, 2022

codecov bot commented Oct 21, 2022 •

edited

Loading

github-actions bot commented Oct 21, 2022

vdaas-ci commented Oct 21, 2022

github-actions bot Oct 21, 2022

github-actions bot Oct 21, 2022

github-actions bot Oct 21, 2022

github-actions bot Oct 21, 2022

github-actions bot Oct 21, 2022

github-actions bot Oct 21, 2022

hlts2 left a comment

small refactor circuitbreaker #1816

small refactor circuitbreaker #1816

Conversation

kpango commented Oct 21, 2022 • edited Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

cloudflare-workers-and-pages bot commented Oct 21, 2022 • edited Loading

Deploying with Cloudflare Pages

vdaas-ci commented Oct 21, 2022

codecov bot commented Oct 21, 2022 • edited Loading

Codecov Report

github-actions bot commented Oct 21, 2022

vdaas-ci commented Oct 21, 2022

Profile Report

github-actions bot Oct 21, 2022

Choose a reason for hiding this comment

github-actions bot Oct 21, 2022

Choose a reason for hiding this comment

github-actions bot Oct 21, 2022

Choose a reason for hiding this comment

github-actions bot Oct 21, 2022

Choose a reason for hiding this comment

github-actions bot Oct 21, 2022

Choose a reason for hiding this comment

github-actions bot Oct 21, 2022

Choose a reason for hiding this comment

hlts2 left a comment

Choose a reason for hiding this comment

kpango commented Oct 21, 2022 •

edited

Loading

cloudflare-workers-and-pages bot commented Oct 21, 2022 •

edited

Loading

codecov bot commented Oct 21, 2022 •

edited

Loading