Skip to content
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

feat: add some metrics about RT #2340

Merged
merged 7 commits into from
Jul 4, 2023
Merged

feat: add some metrics about RT #2340

merged 7 commits into from
Jul 4, 2023

Conversation

ev1lQuark
Copy link
Contributor

The minimum response time (min rt) metrics for different labels are maintained using sync.Map. The purpose of labels is to differentiate between different provider-consumer RPC invocation paths. For example, if provider A offers a service and is called by two consumers, Consumer A and Consumer B, the minimum response time for each path needs to be tracked separately.

Please provide suggestions for the code organization, naming, and implementation approach for this PR.


使用sync.Map对于不同 label 的最小响应时间(min rt)指标进行维护。label 的作用是区分不同的 provider-consumer RPC 调用链路,比如 provider A 提供的服务被 consumer A 和 Consumer B 两个消费者去调用,则两条链路的最小响应时间需要分别统计。

请对于本 PR 代码组织方式、命名、实现思路提出建议。

@jasondeng1997
Copy link
Member

pls fix the ci

@ev1lQuark ev1lQuark marked this pull request as ready for review June 28, 2023 05:35
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2023

Codecov Report

Merging #2340 (524b32b) into main (95b463b) will increase coverage by 0.13%.
The diff coverage is 76.19%.

❗ Current head 524b32b differs from pull request most recent head 38373b5. Consider uploading reports for the commit 38373b5 to get more accurate results

@@            Coverage Diff             @@
##             main    #2340      +/-   ##
==========================================
+ Coverage   44.10%   44.23%   +0.13%     
==========================================
  Files         294      294              
  Lines       17890    18018     +128     
==========================================
+ Hits         7890     7971      +81     
- Misses       9161     9201      +40     
- Partials      839      846       +7     
Impacted Files Coverage Δ
filter/metrics/filter.go 73.91% <0.00%> (-11.09%) ⬇️
metrics/prometheus/model.go 33.54% <62.12%> (ø)
metrics/prometheus/reporter.go 75.55% <80.32%> (+22.22%) ⬆️
metrics/prometheus/metric_set.go 100.00% <100.00%> (ø)
metrics/prometheus/util.go 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ev1lQuark ev1lQuark changed the title feat: add min RT metrics feat: add some metrics about RT Jun 28, 2023
@ev1lQuark
Copy link
Contributor Author

  1. Add more metrics:
    • dubbo_xxx_rt_milliseconds_max
    • dubbo_xxx_rt_milliseconds_avg
    • dubbo_xxx_rt_milliseconds_sum
    • dubbo_xxx_rt_milliseconds_last
  2. refactor some file names and method names

@sonarcloud
Copy link

sonarcloud bot commented Jun 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ev1lQuark ev1lQuark requested a review from FoghostCn June 29, 2023 05:20
Copy link
Contributor

@chickenlj chickenlj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, thanks!

@justxuewei justxuewei merged commit f6fdbfd into apache:main Jul 4, 2023
6 checks passed
@ev1lQuark ev1lQuark deleted the min branch July 4, 2023 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants