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

Add DB metrics & traces: Redis, MySQL #623

Merged
merged 22 commits into from
Aug 20, 2020

Conversation

rinx
Copy link
Contributor

@rinx rinx commented Aug 12, 2020

Signed-off-by: Rintaro Okamura rintaro.okamura@gmail.com

Description:

  • Added DB metrics & traces

    • Redis
      • completed query total
      • query latency
      • completed pipeline total
      • pipeline latency
      • traces for query & pipeline
    • MySQL
      • completed query total
      • query latency
      • traces for query
  • refactored service layer of meta-redis & manager-backup-mysql

because #626 contains tests for mysql/option.go, this PR should be updated after #626 merged into master branch.

Related Issue:

#343

How Has This Been Tested?:

in our k8s cluster.

Environment:

  • Go Version: 1.14.4
  • Docker Version: 19.03.8
  • Kubernetes Version: 1.18.2
  • NGT Version: 1.12.0

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.

@vdaas-ci
Copy link
Collaborator

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

@pull-assistant
Copy link

pull-assistant bot commented Aug 12, 2020

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - add changelog comment
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase master

@codecov
Copy link

codecov bot commented Aug 12, 2020

Codecov Report

Merging #623 into master will increase coverage by 0.19%.
The diff coverage is 34.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #623      +/-   ##
==========================================
+ Coverage   14.41%   14.61%   +0.19%     
==========================================
  Files         409      411       +2     
  Lines       21519    21573      +54     
==========================================
+ Hits         3103     3153      +50     
- Misses      18141    18143       +2     
- Partials      275      277       +2     
Impacted Files Coverage Δ
internal/config/mysql.go 43.47% <0.00%> (-56.53%) ⬇️
internal/config/redis.go 0.00% <0.00%> (ø)
internal/db/rdb/mysql/mysql.go 0.00% <0.00%> (ø)
internal/observability/trace/trace.go 0.00% <ø> (ø)
pkg/manager/backup/mysql/service/mysql.go 0.00% <0.00%> (ø)
pkg/manager/backup/mysql/service/option.go 0.00% <0.00%> (ø)
pkg/manager/backup/mysql/usecase/backupd.go 0.00% <0.00%> (ø)
pkg/meta/redis/handler/grpc/handler.go 0.00% <0.00%> (ø)
pkg/meta/redis/service/redis.go 0.00% <0.00%> (ø)
pkg/meta/redis/usecase/meta.go 0.00% <0.00%> (ø)
... and 8 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 cd8be5b...6cdb3ec. Read the comment docs.

@rinx rinx force-pushed the feature/observability/add-db-metrics-redis-mysql branch 2 times, most recently from 40c5e70 to 22227d0 Compare August 13, 2020 01:40
@github-actions github-actions bot added team/set SET team size/XXL and removed size/XL labels Aug 13, 2020
@rinx rinx force-pushed the feature/observability/add-db-metrics-redis-mysql branch from 938a605 to 3a3c7d9 Compare August 13, 2020 06:18
@@ -25,6 +25,13 @@ import (

var (
enabled bool

BoolAttribute = trace.BoolAttribute
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 🐶
BoolAttribute is a global variable (gochecknoglobals)

@@ -25,6 +25,13 @@ import (

var (
enabled bool

BoolAttribute = trace.BoolAttribute
Float64Attribute = trace.Float64Attribute
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 🐶
Float64Attribute is a global variable (gochecknoglobals)


BoolAttribute = trace.BoolAttribute
Float64Attribute = trace.Float64Attribute
Int64Attribute = trace.Int64Attribute
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 🐶
Int64Attribute is a global variable (gochecknoglobals)

BoolAttribute = trace.BoolAttribute
Float64Attribute = trace.Float64Attribute
Int64Attribute = trace.Int64Attribute
StringAttribute = trace.StringAttribute
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 🐶
StringAttribute is a global variable (gochecknoglobals)

Int64Attribute = trace.Int64Attribute
StringAttribute = trace.StringAttribute

FromContext = trace.FromContext
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 🐶
FromContext is a global variable (gochecknoglobals)

}

return []*metrics.View{
&metrics.View{
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 🐶
File is not gofmt-ed with -s (gofmt)

internal/observability/metrics/db/kvs/redis/redis.go Outdated Show resolved Hide resolved
internal/observability/metrics/db/kvs/redis/redis.go Outdated Show resolved Hide resolved
@rinx rinx force-pushed the feature/observability/add-db-metrics-redis-mysql branch from 3a3c7d9 to 5223959 Compare August 13, 2020 06:36
type args struct {
ctx context.Context
}
type fields struct {
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 🐶
struct of size 296 bytes could be of size 288 bytes (maligned)

},
want: want{
wantR: nil,
err: errors.Wrap(errors.Wrap(err, errors.ErrRedisConnectionPingFailed.Error()), context.DeadlineExceeded.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 🐶
line is 121 characters (lll)

@rinx rinx changed the title Add DB metrics: Redis, MySQL Add DB metrics & traces: Redis, MySQL Aug 13, 2020
"github.com/vdaas/vald/internal/observability/trace"
)

type mysqlMetrics struct {
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 🐶
struct of size 56 bytes could be of size 48 bytes (maligned)


func (mm *mysqlMetrics) View() []*metrics.View {
return []*metrics.View{
&metrics.View{
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 🐶
File is not gofmt-ed with -s (gofmt)


mm.ms = append(
mm.ms,
mm.queryTotal.M(1),
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 🐶
mnd: Magic number: 1, in detected (gomnd)

@rinx rinx linked an issue Aug 13, 2020 that may be closed by this pull request
45 tasks
redis "github.com/go-redis/redis/v7"
)

type Hook = redis.Hook
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 🐶
exported type Hook should have comment or be unexported (golint)

)

type Hook = redis.Hook
type Cmder = redis.Cmder
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 🐶
exported type Cmder should have comment or be unexported (golint)

)

var (
// Nil is a type alias of redis.Nil.
Nil = redis.Nil
)

// Redis is an interface to manipulate Redis server.
type Builder interface {
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 🐶
exported type Builder should have comment or be unexported (golint)

internal/db/kvs/redis/redis_test.go Outdated Show resolved Hide resolved
internal/db/kvs/redis/redis_test.go Outdated Show resolved Hide resolved
if err := test.checkFunc(test.want, got, err); err != nil {
tt.Errorf("error = %v", err)
}

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 🐶
unnecessary trailing newline (whitespace)

if der != nil {
r.dialerFunc = der
}
return nil
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 🐶
return statements should not be cuddled if block has more than two lines (wsl)

@rinx
Copy link
Contributor Author

rinx commented Aug 13, 2020

/rebase
/format

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by rinx for branch: feature/observability/add-db-metrics-redis-mysql

@vdaas-ci vdaas-ci force-pushed the feature/observability/add-db-metrics-redis-mysql branch from 4689228 to b955df4 Compare August 13, 2020 10:13
@vdaas-ci
Copy link
Collaborator

[FORMAT] Updating license headers and formatting go codes triggered by rinx.

@rinx rinx marked this pull request as ready for review August 13, 2020 10:22
@rinx rinx requested a review from kpango August 13, 2020 10:23
rinx and others added 20 commits August 19, 2020 10:19
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Co-authored-by: Yusuke Kato <i.can.feel.gravity@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx rinx force-pushed the feature/observability/add-db-metrics-redis-mysql branch from a29c35c to f9503cf Compare August 19, 2020 01:19
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

Others Looks Good to me

internal/db/kvs/redis/redis.go Outdated Show resolved Hide resolved
internal/db/kvs/redis/redis.go Outdated Show resolved Hide resolved
Co-authored-by: Yusuke Kato <i.can.feel.gravity@gmail.com>
if err != nil {
return nil, err
}
opts = append(opts, mysql.WithTLSConfig(tls))
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 🐶
append only allowed to cuddle with appended value (wsl)

if err != nil {
return nil, err
}
opts = append(opts, mysql.WithDialer(dialer))
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 🐶
append only allowed to cuddle with appended value (wsl)

if err != nil {
return nil, err
}
opts = append(opts, redis.WithTLSConfig(tls))
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 🐶
append only allowed to cuddle with appended value (wsl)

@kpango
Copy link
Collaborator

kpango commented Aug 20, 2020

LGTM

@kpango kpango merged commit 642c149 into master Aug 20, 2020
@kpango kpango deleted the feature/observability/add-db-metrics-redis-mysql branch August 20, 2020 02:37
@vdaas-ci vdaas-ci mentioned this pull request Sep 2, 2020
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[proposal] Add more custom metrics for monitoring
5 participants