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: Cassandra #587

Merged
merged 15 commits into from
Aug 3, 2020
Merged

Conversation

rinx
Copy link
Contributor

@rinx rinx commented Jul 16, 2020

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

Description:

  • Add Cassandra metrics to these components

    • meta
    • backup
  • added metrics

    • query execution total count
    • query attempts total count
    • query latency

Related Issue:

#343

How Has This Been Tested?:

nothing

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.

@pull-assistant
Copy link

pull-assistant bot commented Jul 16, 2020

Score: 0.74

Best reviewed: commit by commit


Optimal code review plan (3 warnings)

✨ add cassandra metrics

...db/nosql/cassandra/observer.go 63% changes removed in ♻️ use int64 ...

✅ update

...ndra/service/cassandra_test.go 85% changes removed in ♻️ remove red...

♻️ modified metrics type and name

...b/nosql/cassandra/cassandra.go 75% changes removed in ♻️ use int64 ...

     ♻️ modified aggregation type

     ♻️ use int64 for ms total

     ♻️ fix

     ✅ update

     ♻️ remove redundant opts

     ✨ add query observer to backup manager cassandra

     ✨ add tags

     ♻️ revise

     ✨ add query attempts total

     ♻️ use same variable for metrics description

     ✅ update

     Merge branch 'master' into feature/meta-cassandra/add-query-metrics

Powered by Pull Assistant. Last update e13c495 ... 8cc5893. Read the comment docs.

@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 Jul 17, 2020

Codecov Report

Merging #587 into master will increase coverage by 3.92%.
The diff coverage is 16.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #587      +/-   ##
==========================================
+ Coverage    7.84%   11.77%   +3.92%     
==========================================
  Files         388      409      +21     
  Lines       19945    21302    +1357     
==========================================
+ Hits         1565     2508     +943     
- Misses      18156    18514     +358     
- Partials      224      280      +56     
Impacted Files Coverage Δ
internal/cache/cacher/cacher.go 100.00% <ø> (+100.00%) ⬆️
internal/client/discoverer/discover.go 0.00% <0.00%> (ø)
internal/compress/gob.go 26.19% <0.00%> (-1.31%) ⬇️
internal/compress/gzip.go 26.98% <0.00%> (-7.02%) ⬇️
internal/compress/lz4.go 36.36% <0.00%> (-5.31%) ⬇️
internal/compress/zstd.go 29.50% <0.00%> (-5.79%) ⬇️
internal/config/blob.go 0.00% <0.00%> (ø)
internal/config/config.go 15.78% <ø> (+15.78%) ⬆️
internal/config/grpc.go 0.00% <0.00%> (ø)
internal/config/log.go 100.00% <ø> (+100.00%) ⬆️
... and 172 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 a5ac60c...8cc5893. Read the comment docs.


func (cm *cassandraMetrics) 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)

@rinx rinx force-pushed the feature/meta-cassandra/add-query-metrics branch from 1255083 to c003660 Compare July 21, 2020 03:11
@@ -55,7 +56,7 @@ func New() metrics.Metric {
heapReleased: *metrics.Int64(metrics.ValdOrg+"/memory/heap_released", "bytes of physical memory returned to the OS", metrics.UnitBytes),
stackInuse: *metrics.Int64(metrics.ValdOrg+"/memory/stack_inuse", "bytes in stack spans", metrics.UnitBytes),
stackSys: *metrics.Int64(metrics.ValdOrg+"/memory/stack_sys", "bytes of stack memory obtained from the OS", metrics.UnitBytes),
pauseTotalMs: *metrics.Float64(metrics.ValdOrg+"/memory/pause_ms_total", "the cumulative milliseconds in GC", metrics.UnitMilliseconds),
pauseTotalMs: *metrics.Int64(metrics.ValdOrg+"/memory/pause_ms_total", "the cumulative milliseconds in GC", metrics.UnitMilliseconds),
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 136 characters (lll)

@@ -40,6 +40,8 @@ var (
LastValue = view.LastValue
Sum = view.Sum

DefaultMillisecondsDistribution = Distribution(0.01, 0.05, 0.1, 0.3, 0.6, 0.8, 1, 2, 3, 4, 5, 6, 8, 10, 13, 16, 20, 25, 30, 40, 50, 65, 80, 100, 130, 160, 200, 250, 300, 400, 500, 650, 800, 1000, 2000, 5000, 10000, 20000, 50000, 100000)
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 237 characters (lll)


func New() Observer {
return &cassandraMetrics{
queryTotal: *metrics.Int64(metrics.ValdOrg+"/db/nosql/cassandra/completed_query_total", "cumulative count of completed queries", metrics.UnitDimensionless),
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 160 characters (lll)

func New() Observer {
return &cassandraMetrics{
queryTotal: *metrics.Int64(metrics.ValdOrg+"/db/nosql/cassandra/completed_query_total", "cumulative count of completed queries", metrics.UnitDimensionless),
queryLatency: *metrics.Float64(metrics.ValdOrg+"/db/nosql/cassandra/query_latency", "query latency", metrics.UnitMilliseconds),
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 129 characters (lll)

ms []metrics.MeasurementWithTags
}

type Observer 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 Observer should have comment or be unexported (golint)

cassandra.QueryObserver
}

func New() (o Observer, 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 🐶
exported function New should have comment or be unexported (golint)

cm.ms = append(
cm.ms,
metrics.MeasurementWithTags{
Measurement: cm.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)

"github.com/gocql/gocql"
)

type QueryObserver = gocql.QueryObserver
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 QueryObserver should have comment or be unexported (golint)

)

type QueryObserver = gocql.QueryObserver
type ObservedQuery = gocql.ObservedQuery
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 ObservedQuery should have comment or be unexported (golint)

type QueryObserver = gocql.QueryObserver
type ObservedQuery = gocql.ObservedQuery

type BatchObserver = gocql.BatchObserver
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 BatchObserver should have comment or be unexported (golint)


type BatchObserver = gocql.BatchObserver

type ConnectObserver = gocql.ConnectObserver
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 ConnectObserver should have comment or be unexported (golint)


type ConnectObserver = gocql.ConnectObserver

type FrameHeaderObserver = gocql.FrameHeaderObserver
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 FrameHeaderObserver should have comment or be unexported (golint)

@rinx rinx force-pushed the feature/meta-cassandra/add-query-metrics branch from 438ed70 to 39d4911 Compare July 21, 2020 10:17
@rinx rinx marked this pull request as ready for review July 22, 2020 03:34
@rinx
Copy link
Contributor Author

rinx commented Jul 27, 2020

/rebase
/format

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by rinx for branch: feature/meta-cassandra/add-query-metrics

@vdaas-ci vdaas-ci force-pushed the feature/meta-cassandra/add-query-metrics branch from 4776087 to 07434c0 Compare July 27, 2020 11:38
@vdaas-ci
Copy link
Collaborator

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

@@ -40,6 +40,8 @@ var (
LastValue = view.LastValue
Sum = view.Sum

DefaultMillisecondsDistribution = Distribution(0.01, 0.05, 0.1, 0.3, 0.6, 0.8, 1, 2, 3, 4, 5, 6, 8, 10, 13, 16, 20, 25, 30, 40, 50, 65, 80, 100, 130, 160, 200, 250, 300, 400, 500, 650, 800, 1000, 2000, 5000, 10000, 20000, 50000, 100000)
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 🐶
DefaultMillisecondsDistribution is a global variable (gochecknoglobals)

@rinx rinx requested review from kpango and datelier July 27, 2020 12:46
kpango
kpango previously approved these changes Jul 27, 2020
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.

LGTM

datelier
datelier previously approved these changes Jul 28, 2020
@rinx
Copy link
Contributor Author

rinx commented Jul 28, 2020

/rebase

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by rinx for branch: feature/meta-cassandra/add-query-metrics

@vdaas-ci vdaas-ci dismissed stale reviews from datelier and kpango via 2513c9c July 28, 2020 14:47
@vdaas-ci vdaas-ci force-pushed the feature/meta-cassandra/add-query-metrics branch from 07434c0 to 2513c9c Compare July 28, 2020 14:47
@kpango
Copy link
Collaborator

kpango commented Aug 3, 2020

/rebase
/format
/approve

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Aug 3, 2020

[REBASE] Rebase triggered by kpango for branch: feature/meta-cassandra/add-query-metrics

rinx added 14 commits August 3, 2020 03:12
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>
@vdaas-ci vdaas-ci force-pushed the feature/meta-cassandra/add-query-metrics branch from 2513c9c to b95d636 Compare August 3, 2020 03:12
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Aug 3, 2020

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

Copy link
Collaborator

@vdaas-ci vdaas-ci left a comment

Choose a reason for hiding this comment

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

[APPROVED] This PR is approved by kpango.

@kpango kpango merged commit 2348bba into master Aug 3, 2020
@kpango kpango deleted the feature/meta-cassandra/add-query-metrics branch August 3, 2020 03:26
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.

4 participants