Register quota metrics unconditionally - #6253
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 1 file with indirect coverage changes @@ Coverage Diff @@
## development/9.4 #6253 +/- ##
===================================================
- Coverage 86.38% 86.33% -0.05%
===================================================
Files 212 212
Lines 14567 14566 -1
===================================================
- Hits 12583 12575 -8
- Misses 1984 1991 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
b448fe5 to
d580a11
Compare
|
Requested @francoisferrand in place of Maha Benzekri, who is currently on PTO. |
d580a11 to
fcefd5a
Compare
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
francoisferrand
left a comment
francoisferrand
left a comment
There was a problem hiding this comment.
- I think creating the metrics is harmless (it was already the case); and it avoids adding all the ?. → just create them always
- less sure how to fix crrCacheToPom
delthas
changed the title
fcefd5a to
33f77af
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
francoisferrand
left a comment
francoisferrand
left a comment
There was a problem hiding this comment.
LGTM, but I think this warrants another set of eyes from product, to confirm we want/accept to have the metrics always (not just the presence of the metrics, but especially publishing actual data points for the bucketsWithQuota/accountsWithQuota metrics)
Both quota gates in monitoringHandler tested config.isQuotaEnabled -- the function reference rather than a call -- so both were always true and all six quota metrics were always registered. Drop the conditionals rather than calling the function. Always registering is what has actually happened since these checks were written, nothing depends on the metrics disappearing when quota is off, and gating registration would mean guarding every metric touch point against a metric that was never created: crrCacheToProm reached bucketsWithQuota unguarded, and the scuba wrapper observes its histogram in a finally block, on a path the Veeam capacity route hits with no quota gating at all. Behaviour is unchanged -- the six s3_cloudserver_quota_* series are exposed exactly as before -- so this is a dead code removal. Issue: CLDSRV-972
33f77af to
eeb84c5
Compare
|
Asking product. |
|
Product said both options are OK, let's do simplest. So, merging as is. |
|
/approve |
|
!done 3h |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
This pull request does not target the following hotfix branch(es) so they
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
Queue build failedThe corresponding build for the queue failed:
Remove the pull request from the queue
|
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-972. Goodbye delthas. |
|
|
4 participants
Stacked on #6252 — review that one first. This PR's diff is only the last two
commits; retarget to
development/9.4once #6252 merges.Both quota gates in
lib/utilities/monitoringHandler.jstestedconfig.isQuotaEnabled— the function reference rather than a call — so bothwere always true, and all six quota metrics have always been registered.
Rather than calling the function, the conditionals are removed. Always
registering is what has actually happened since these checks were written,
nothing depends on the metrics disappearing when quota is off, and gating
registration would mean guarding every metric touch point against a metric that
was never created:
crrCacheToPromreachedbucketsWithQuotaunguarded, andthe scuba wrapper observes its histogram in a
finallyblock, on a path theVeeam capacity route reaches with no quota gating at all.
Behaviour is unchanged — the six
s3_cloudserver_quota_*series are exposedexactly as before — so this is a dead code removal rather than a metric surface
change.
Issue: CLDSRV-972