metrics: add connection and fail metrics by resource group name

[bufferflies]

What problem does this PR solve?

Issue Number: ref #49318

Problem Summary:

What changed and how does it work?

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
  http://perf.pingcap.net/d/iarAjGo7z/benchbot-overview?orgId=1&var-email=All

• I checked and no code files have been changed.

Side effects

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

[ti-chi-bot]

Hi @bufferflies. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tiprow]

Hi @bufferflies. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bufferflies]

/review @nolouch @glorv

[ti-chi-bot]

@bufferflies:

General Overview

The PR titled "metrics: add connection and fail metrics by resource group name" aims to add connection and fail metrics by resource group name. The changes made in this PR mainly affect pkg/metrics/grafana/tidb_resource_control.json, pkg/metrics/server.go, pkg/metrics/session.go, pkg/server/conn.go, pkg/server/conn_test.go, and pkg/server/server.go.

What problem does this PR solve?

The PR resolves issue #49318 by adding connection and fail metrics by the resource group name.

What changed and how does it work?

The changes in this PR are primarily updates to the metrics collection and handling. The metrics for connections and execution errors are now collected by the resource group name, which provides more granular insights. The connections metric is now a GaugeVec, allowing for the resource group label. The ExecuteErrorCounter also includes the resource group label.

The changes to the conn.go file update the handling of connections, where the number of connections is now tracked by the resource group name. The server.go file sees similar updates, where connections are registered by the resource group name.

The tidb_resource_control.json file has been updated to reflect these changes, with the expressions for metrics now including the resource group name.

Check List

There are no unit or integration tests included with this PR, and it does not specify whether manual tests were run. There are also no apparent side effects or documentation updates.

Review Comments

1. It's not clear whether any testing was done based on the PR description. It's recommended to include details about the kind of testing performed or why testing was not necessary.

2. The PR description does not provide a detailed explanation of how these changes work. It would be helpful if the author could include more information about the changes and their impact.

3. The changes in conn_test.go are minimal and do not seem to fully test the new functionality. It would be beneficial to add more extensive tests to verify that the connection and fail metrics are correctly tracked by the resource group name.

4. The PR could benefit from a more detailed release note, especially if these changes have any user-facing impact.

Overall, the changes look solid, but the PR could benefit from more extensive testing and documentation.

In response to this:

/review @nolouch @glorv

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[glorv]

/ok-to-test

[codecov]

Codecov Report

Merging #49424 (2d2ecaa) into master (5f6ef18) will increase coverage by 0.3891%.
Report is 64 commits behind head on master.
The diff coverage is 84.6153%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #49424        +/-   ##
================================================
+ Coverage   70.9221%   71.3113%   +0.3891%     
================================================
  Files          1368       1434        +66     
  Lines        396367     439041     +42674     
================================================
+ Hits         281112     313086     +31974     
- Misses        95564     107005     +11441     
+ Partials      19691      18950       -741     

Flag	Coverage Δ
integration	44.9489% <84.2696%> (?)
unit	71.0425% <100.0000%> (+0.1204%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	53.9663% <ø> (ø)
parser	∅ <ø> (∅)
br	45.2348% <ø> (-7.6685%)	⬇️

[glorv]

ping @tiancaiamao PTAL

[ti-chi-bot]

[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:

• OWNERS [tiancaiamao]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2023-12-27 04:02:31.459794675 +0000 UTC m=+1624842.497021586: ☑️ agreed by nolouch.
• 2024-01-03 04:17:21.006840791 +0000 UTC m=+2230532.044067736: ☑️ agreed by tiancaiamao.

[bufferflies]

/label needs-cherry-pick-7.5

[ti-chi-bot]

@bufferflies: The label(s) needs-cherry-pick-7.5 cannot be applied. These labels are supported: fuzz/sqlancer, challenge-program, compatibility-breaker, first-time-contributor, contribution, good first issue, correctness, duplicate, proposal, security, ok-to-test, needs-ok-to-test, needs-more-info, needs-cherry-pick-release-5.4, needs-cherry-pick-release-6.1, needs-cherry-pick-release-6.5, needs-cherry-pick-release-7.1, needs-cherry-pick-release-7.5, affects-5.4, affects-6.1, affects-6.5, affects-7.1, affects-7.5, may-affects-5.4, may-affects-6.1, may-affects-6.5, may-affects-7.1, may-affects-7.5.

In response to this:

/label needs-cherry-pick-7.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[bufferflies]

/label needs-cherry-pick-release-7.5

[ti-chi-bot]

In response to a cherrypick label: new pull request created to branch release-7.5: #50124.

[ti-chi-bot]

In response to a cherrypick label: new pull request created to branch release-7.1: #52698.