-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #51996 +/- ##
=================================================
- Coverage 70.7175% 54.9760% -15.7416%
=================================================
Files 1477 1592 +115
Lines 438748 615070 +176322
=================================================
+ Hits 310272 338141 +27869
- Misses 109008 253582 +144574
- Partials 19468 23347 +3879
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/retest |
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
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: nolouch, tiancaiamao 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:
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
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