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

Separate open and closed issue in metrics #16637

Merged
merged 3 commits into from
Aug 7, 2021

Conversation

romdum
Copy link
Contributor

@romdum romdum commented Aug 6, 2021

Hi,

My team and I need to separate closed and open issue in exposed metrics.

The purpose is to have the number of open and closed issues by week in a Grafana dashboard.

@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2021

Codecov Report

Merging #16637 (ed1f319) into main (620c569) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16637      +/-   ##
==========================================
- Coverage   45.41%   45.38%   -0.03%     
==========================================
  Files         756      756              
  Lines       85247    85279      +32     
==========================================
- Hits        38713    38707       -6     
- Misses      40274    40309      +35     
- Partials     6260     6263       +3     
Impacted Files Coverage Δ
models/models.go 53.42% <0.00%> (-2.56%) ⬇️
modules/metrics/collector.go 0.00% <0.00%> (ø)
modules/util/timer.go 42.85% <0.00%> (-42.86%) ⬇️
modules/queue/workerpool.go 53.05% <0.00%> (-1.91%) ⬇️
modules/queue/queue_channel.go 95.00% <0.00%> (-1.67%) ⬇️
modules/log/file.go 73.60% <0.00%> (-1.61%) ⬇️
routers/web/repo/view.go 41.52% <0.00%> (+0.57%) ⬆️
models/repo_list.go 77.64% <0.00%> (+0.78%) ⬆️

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 620c569...ed1f319. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 6, 2021
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 6, 2021
@lafriks
Copy link
Member

lafriks commented Aug 6, 2021

I think old metric should be kept to not break compatibility

@zeripath
Copy link
Contributor

zeripath commented Aug 6, 2021

Oh I missed that they'd removed that! Sorry for the bad review

models/models.go Outdated Show resolved Hide resolved
models/models.go Outdated Show resolved Hide resolved
modules/metrics/collector.go Show resolved Hide resolved
modules/metrics/collector.go Show resolved Hide resolved
modules/metrics/collector.go Show resolved Hide resolved
modules/metrics/collector.go Show resolved Hide resolved
@romdum
Copy link
Contributor Author

romdum commented Aug 6, 2021

If we keep Issue metrics, may be it's not necessary to keep IssueOpen metrics because: Issue - IssueClosed = IssueOpen ?

@zeripath
Copy link
Contributor

zeripath commented Aug 6, 2021

So as these metrics are not produced within a transaction you can't actually assume that.

(We have to keep Issues anyway because of the dashboard.)

@zeripath
Copy link
Contributor

zeripath commented Aug 7, 2021

OK I've made a little change which means that the counts are collected in one query.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 7, 2021
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Aug 7, 2021
@lafriks lafriks added this to the 1.16.0 milestone Aug 7, 2021
@lafriks lafriks changed the title Proposal: Separate open and closed issue in metrics Separate open and closed issue in metrics Aug 7, 2021
@lafriks lafriks merged commit 14762ab into go-gitea:main Aug 7, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants