-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Remove duplicated loki_boltdb_shipper
prefix from tables_upload_operation_total
metric
#7040
Conversation
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the PR @bakunowski. Sounds totally reasonable. Also double checked with metrics coming from internal Loki clusters.
Since this is the user visible change, can you please add it in CHANGELOG as well please? I will merge after that.
Thanks for the review @kavirajk, glad to hear I wasn't completely off the mark. Updated CHANGELOG in the |
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for fixing it!
…eration_total` metric (grafana#7040) This PR removes the duplicated `loki_boltdb_shipper` prefix from the `tables_upload_operation_total` metric.
What this PR does / why we need it:
This PR removes the duplicated
loki_boltdb_shipper
prefix from thetables_upload_operation_total
metric.In #6278 metrics for tracking
indexshipper
operations were added.WrapRegistererWithPrefix
was used to make sure the same metrics as before were exposed (see here).While the
downloads
metrics had theNamespace
field removed (see here), theuploads
metric did not (see here). This results in thetables_upload_operation_total
being exposed asloki_boltdb_shipper_loki_boltdb_shipper_tables_upload_operation_total
instead ofloki_boltdb_shipper_tables_upload_operation_total
.Which issue(s) this PR fixes:
This PR serves both as an issue and a fix.
Special notes for your reviewer:
This is my best interpretation why the exposed name of the
uploads
metric has changed. If this is incorrect, or the change was intentional - I'm sorry for the noise.Checklist
CHANGELOG.md
.docs/sources/upgrading/_index.md