Introducing cachemetrics package #2156

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.

Sign up for GitHub

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

Jump to bottom

Merged

pipecd-bot merged 1 commit into master from cache-git-metrics

Jun 30, 2021

Member

khanhtc1202 commented Jun 30, 2021

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

pipecd-bot added release-note-none size/M area/manifests area/server labels

pipecd-bot requested review from nakabonne and nghialv

June 30, 2021 09:34

Collaborator

pipecd-bot commented Jun 30, 2021

Code coverage for golang is 32.63%. This pull request increases coverage by 0.02%.

File	Function	Base	Head	Diff
pkg/cache/cachemetrics/metrics.go	Register	`--`	`0.00%`	`+0.00%`
pkg/cache/cachemetrics/metrics.go	IncrementRequestCacheCounter	`--`	`100.00%`	`+100.00%`
pkg/cache/cache.go	multiGetter.Get	`91.67%`	`87.50%`	`-4.17%`

pipecd-bot reviewed

View reviewed changes

Collaborator

pipecd-bot left a comment

Some issues were detected while linting go source files in your changes.

pkg/cache/cachemetrics/metrics.go Show resolved Hide resolved

pipecd-bot added size/L and removed size/M labels

pipecd-bot reviewed

View reviewed changes

Collaborator

pipecd-bot left a comment

Some issues were detected while linting go source files in your changes.

pkg/cache/cachemetrics/metrics.go Show resolved Hide resolved

nghialv reviewed

View reviewed changes

pkg/cache/cachemetrics/metrics.go Outdated

+              	r.MustRegister(getCounter)
+              }
+              func IncrementRequestCacheCounter(status StatusLabel, source SourceLabel) {

Member

nghialv Jun 30, 2021

nit: Increment or Inc? which is better?

Member Author

khanhtc1202 Jun 30, 2021

Lets go with Inc for short 👍


          Add cachemetrics package

102f792

khanhtc1202 force-pushed the cache-git-metrics branch from 31974eb to 102f792 Compare

June 30, 2021 10:10

pipecd-bot added the lgtm label

pipecd-bot reviewed

View reviewed changes

Collaborator

pipecd-bot left a comment

Some issues were detected while linting go source files in your changes.

pkg/cache/cachemetrics/metrics.go Show resolved Hide resolved

pkg/cache/cachemetrics/metrics.go Show resolved Hide resolved

Member

nghialv commented Jun 30, 2021

Thank you.
/lgtm

nakabonne reviewed

View reviewed changes

pkg/cache/cachemetrics/metrics.go

+              var (
+              	getCounter = prometheus.NewCounterVec(
+              		prometheus.CounterOpts{
+              			Name: "controlplane_cache_get_operation_total",

Member

nakabonne Jun 30, 2021

Is prefix controlplane okay? pipecd is better?

But it's okay to determine later.

Member Author

khanhtc1202 Jun 30, 2021

Ops, should be as your suggestion 🙏 lets me address it in this PR 🙏

Member

nakabonne Jun 30, 2021

Oops, already merged lol

Member Author

khanhtc1202 Jun 30, 2021

@nakabonne plz #2157 🙏

Member

nakabonne Jun 30, 2021

Gotcha!

Collaborator

pipecd-bot commented Jun 30, 2021

Code coverage for golang is 32.59%. This pull request decreases coverage by -0.02%.

File	Function	Base	Head	Diff
pkg/cache/cachemetrics/metrics.go	Register	`--`	`0.00%`	`+0.00%`
pkg/cache/cachemetrics/metrics.go	IncGetOperationCounter	`--`	`0.00%`	`+0.00%`
pkg/cache/rediscache/cache.go	RedisCache.Get	`0.00%`	`0.00%`	`+0.00%`

Member

nakabonne commented Jun 30, 2021

I can't wait to see those metrics on our dashboard.
/approve

pipecd-bot added the approved label

Collaborator

pipecd-bot commented Jun 30, 2021

This pull request is APPROVED by nakabonne.

Approvers can cancel the approval by writing /approve cancel in a comment. Any additional commits also will change this pull request to be not-approved.

pipecd-bot merged commit 8bb26e0 into master

pipecd-bot deleted the cache-git-metrics branch

June 30, 2021 10:19

khanhtc1202 mentioned this pull request

Refactor cachemetrics interface and label name #2157

Merged

pipecd-bot pushed a commit that referenced this pull request


          Refactor cachemetrics interface and label name (#2157)

b89a5fd

**What this PR does / why we need it**:

Adopt #2156 (comment) 👀 

**Which issue(s) this PR fixes**:

Fixes #

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
-->
```release-note
NONE
```

This PR was merged by Kapetanios.

pipecd-bot mentioned this pull request

Refactor piped metrics exporter #2177

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved area/manifests area/server lgtm release-note-none size/L