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

refactor: make rt metric more performance and easy to use #2376

Merged
merged 8 commits into from
Aug 16, 2023
Merged

refactor: make rt metric more performance and easy to use #2376

merged 8 commits into from
Aug 16, 2023

Conversation

FoghostCn
Copy link
Contributor

@FoghostCn FoghostCn commented Aug 9, 2023

  1. optimize rt calculation time
    The current implementation rt will be calculated every time the sample data is added, which is performance-intensive and unnecessary.
    new implementation will only be calculated when Prometheus pulls data

  2. add more unit tests for metrics

  3. add Rt interface to MetricRegisrty simplfy rt processing

for example:

var MetricRegisrty registry
registry.Rt(&MetricId{Name:"dubbo_request_rt", Desc: "Request RT"}).Observe(100)

// next time also call it in this way, no need to manually cache it
registry.Rt(&MetricId{Name:"dubbo_request_rt", Desc: "Request RT"}).Observe(100)

it will produce 5 metrics in prometheus:

# HELP dubbo_request_rt_avg Average request RT
# TYPE dubbo_request_rt_avg gauge
dubbo_request_rt_avg{app="dubbo",version="1.0.0"} 45
# HELP dubbo_request_rt_last Last request RT
# TYPE dubbo_request_rt_last gauge
dubbo_request_rt_last{app="dubbo",version="1.0.0"} 90
# HELP dubbo_request_rt_max Max request RT
# TYPE dubbo_request_rt_max gauge
dubbo_request_rt_max{app="dubbo",version="1.0.0"} 90
# HELP dubbo_request_rt_min Min request RT
# TYPE dubbo_request_rt_min gauge
dubbo_request_rt_min{app="dubbo",version="1.0.0"} 0
# HELP dubbo_request_rt_sum Sum request RT
# TYPE dubbo_request_rt_sum gauge
dubbo_request_rt_sum{app="dubbo",version="1.0.0"} 450

merge histogram summary metric interface to ObservableMetric
Calculate the rt result when Prometheus pulls the data
simplfy prometheus registry and impl rt interface
@FoghostCn FoghostCn changed the title refactor: make rt metric more performance and esay to use refactor: make rt metric more performance and easy to use Aug 9, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2023

Codecov Report

Merging #2376 (32fb9d6) into main (8b43135) will increase coverage by 0.73%.
Report is 2 commits behind head on main.
The diff coverage is 95.03%.

@@            Coverage Diff             @@
##             main    #2376      +/-   ##
==========================================
+ Coverage   44.27%   45.00%   +0.73%     
==========================================
  Files         304      306       +2     
  Lines       18528    18687     +159     
==========================================
+ Hits         8203     8410     +207     
+ Misses       9467     9404      -63     
- Partials      858      873      +15     
Files Changed Coverage Δ
common/host_util.go 85.18% <ø> (+14.81%) ⬆️
metrics/api.go 0.00% <0.00%> (ø)
metrics/prometheus/reporter.go 77.14% <ø> (ø)
metrics/prometheus/util.go 100.00% <ø> (ø)
metrics/prometheus/registry.go 88.88% <97.77%> (+87.71%) ⬆️
metrics/prometheus/rt_vec.go 100.00% <100.00%> (ø)
metrics/util/aggregate/aggregator.go 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

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

@ev1lQuark
Copy link
Contributor

LGTM

@sonarqubecloud
Copy link

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
No Duplication information No Duplication information

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