-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
server, metrics: remove the connection count on server, only use the metrics (#51996) #53056
server, metrics: remove the connection count on server, only use the metrics (#51996) #53056
Conversation
Signed-off-by: ti-chi-bot <[email protected]>
Signed-off-by: Yang Keao <[email protected]>
ed95a71
to
6590ea3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-7.5 #53056 +/- ##
================================================
Coverage ? 72.0054%
================================================
Files ? 1411
Lines ? 410234
Branches ? 0
================================================
Hits ? 295391
Misses ? 94984
Partials ? 19859
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tiancaiamao, YangKeao The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
…51996-to-release-7.5 Signed-off-by: Yang Keao <[email protected]>
This is an automated cherry-pick of #51996
What problem does this PR solve?
Issue Number: close #51889
Problem Summary:
server
to count the connection for each resource group seems to be not necessary. We can use theGaugeVec
itself. The connection count is not used anywhere else.Maybe someone prefers to sum up them together. Actually I think both are fine, feel free to comment your opinion.
Personally, I don't like the idea to use the uncontrollable string as a label 🤦, because the prometheus will have much greater pressure when these labels multiply together.
As for now, it seems not a big issue, but it'll become much worse if anyone adds more similar labels.
What changed and how does it work?
GaugeVec
as the only counter of the connection.set resource group
manually.Check List
Tests
Release note