[Improvement][Metrics] Apply micrometer naming convention to metrics by EricGao888 · Pull Request #10477 · apache/dolphinscheduler

EricGao888 · 2022-06-16T09:15:42Z

Purpose of the pull request

Apply micrometer naming convention to DS metrics for future portability and compatibility.
For detailed explanation, refer to Naming Convention section
This PR closes: [Improvement][Metrics] Apply micrometer naming convention to metrics #10432
related: [DSIP-8][Metrics] Improve DolphinScheduler Monitoring #9324

Brief change log

Apply naming conventions to metrics. Metrics names in Grafana demo dashboards are updated correspondingly.
Add explanations about metrics naming convention and mapping in docs.
Add Chinese version docs.

Verify this pull request

Verified by manual test.

EricGao888 · 2022-06-16T09:53:56Z

Could u plz take a look when you have time? @ruanwenjun Thx~

ruanwenjun · 2022-06-16T10:04:17Z

...ker/src/main/java/org/apache/dolphinscheduler/server/worker/metrics/WorkerServerMetrics.java

    private static final Counter WORKER_SUBMIT_QUEUE_IS_FULL_COUNTER =
-            Counter.builder("dolphinscheduler_worker_submit_queue_is_full_count")
-                    .description("worker task submit queue is full count")
+            Counter.builder("ds.system.worker.full.submit.queue.count")


How about remove the system?

Suggested change

Counter.builder("ds.system.worker.full.submit.queue.count")

Counter.builder("ds.worker.full.submit.queue.count")

Sure, will remove system, since it might cause some ambiguities with jvm / network level stuff.

…etrics (apache#10432)

…t metrics (apache#10432)

codecov-commenter · 2022-06-19T22:01:00Z

Codecov Report

❌ Patch coverage is 61.76471% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.85%. Comparing base (2c227ab) to head (05b95ef).
⚠️ Report is 2179 commits behind head on dev.

Files with missing lines	Patch %	Lines
...ler/server/worker/metrics/WorkerServerMetrics.java	0.00%	6 Missing ⚠️
...inscheduler/server/worker/metrics/TaskMetrics.java	0.00%	4 Missing ⚠️
...ler/server/master/metrics/MasterServerMetrics.java	0.00%	2 Missing ⚠️
.../server/master/metrics/ProcessInstanceMetrics.java	87.50%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                dev   #10477      +/-   ##
============================================
- Coverage     40.86%   40.85%   -0.02%     
+ Complexity     4849     4846       -3     
============================================
  Files           886      886              
  Lines         36009    36029      +20     
  Branches       3994     3999       +5     
============================================
+ Hits          14716    14718       +2     
- Misses        19835    19852      +17     
- Partials       1458     1459       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kezhenxu94

Do we have a way to unify the formats of customize metrics as well as default metrics (like jvm_gc_pause_seconds_max, etc.)? Otherwise the defaults still don't follow the micrometer naming convention.

EricGao888 · 2022-06-20T01:58:50Z

Do we have a way to unify the formats of customize metrics as well as default metrics (like jvm_gc_pause_seconds_max, etc.)? Otherwise the defaults still don't follow the micrometer naming convention.

@kezhenxu94 Thanks for pointing this out. Actually those default metrics DO follow the naming convention. If we check http://ip:port/actuator/metrics, we could get the list of metrics' names before mapping. We will see those default metrics as
. The reason why in this PR it looks different is that this PR is still a draft and WIP. I haven't updated the docs yet. Sorry for the confusion.

kezhenxu94 · 2022-06-20T02:29:26Z

Do we have a way to unify the formats of customize metrics as well as default metrics (like jvm_gc_pause_seconds_max, etc.)? Otherwise the defaults still don't follow the micrometer naming convention.

@kezhenxu94 Thanks for pointing this out. Actually those default metrics DO follow the naming convention. If we check http://ip:port/actuator/metrics, we could get the list of metrics' names before mapping. We will see those default metrics as . The reason why in this PR it looks different is that this PR is still a draft and WIP. I haven't updated the docs yet. Sorry for the confusion.

Nice if that's the case! LGTM

EricGao888 · 2022-06-20T02:33:12Z

Do we have a way to unify the formats of customize metrics as well as default metrics (like jvm_gc_pause_seconds_max, etc.)? Otherwise the defaults still don't follow the micrometer naming convention.

@kezhenxu94 Thanks for pointing this out. Actually those default metrics DO follow the naming convention. If we check http://ip:port/actuator/metrics, we could get the list of metrics' names before mapping. We will see those default metrics as . The reason why in this PR it looks different is that this PR is still a draft and WIP. I haven't updated the docs yet. Sorry for the confusion.

Nice if that's the case! LGTM

@kezhenxu94 Somehow I just couldn't fall asleep last night. So I got up very early this morning, wrote some code and docs, committed and pushed them to find some peace and went to bed again. Haven't finished it yet, lol. I will complete this PR later today. 🤣

…cs and add Chinese docs (apache#10432)

…pache#10432)

EricGao888 · 2022-06-20T13:37:26Z

docs/docs/en/guide/metrics/metrics.md

-at `dolphinscheduler-meter/resources/grafana`, you can directly import these dashboards to grafana.
-
-If you want to try at docker, you can use the following command to start the prometheus with grafana:
+- We enable Apache DolphinScheduler export metrics in `standalone` mode to help users get hands dirty easily. 


Suggested change

- We enable Apache DolphinScheduler export metrics in `standalone` mode to help users get hands dirty easily.

- We enable Apache DolphinScheduler to export metrics in `standalone` mode to help users get hands dirty easily.

sonarqubecloud · 2022-06-20T13:47:00Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

61.8% Coverage
0.0% Duplication

kezhenxu94

LGTM

ruanwenjun

+1

…pache#10477) * Apply micrometer naming convention to worker metrics * Apply micrometer naming convention all current metrics * Fix remaining metrics names, update English docs and add Chinese docs * Fix metrics names in grafana-demo dashboards

EricGao888 requested review from SbloodyS, caishunfeng and kezhenxu94 as code owners June 16, 2022 09:15

EricGao888 force-pushed the Fix-10432 branch from be70bbe to 63944d5 Compare June 16, 2022 09:30

EricGao888 marked this pull request as draft June 16, 2022 09:48

SbloodyS requested a review from ruanwenjun June 16, 2022 09:50

ruanwenjun reviewed Jun 16, 2022

View reviewed changes

SbloodyS assigned EricGao888 Jun 19, 2022

SbloodyS added improvement make more easy to user or prompt friendly backend metrics labels Jun 19, 2022

EricGao888 added 2 commits June 19, 2022 21:55

[Improvement][Metrics] Apply micrometer naming convention to worker m…

25a3192

…etrics (apache#10432)

[Improvement][Metrics] Apply micrometer naming convention all curren…

a45b9e6

…t metrics (apache#10432)

EricGao888 force-pushed the Fix-10432 branch from 63944d5 to a45b9e6 Compare June 19, 2022 21:44

kezhenxu94 reviewed Jun 20, 2022

View reviewed changes

EricGao888 added 2 commits June 20, 2022 19:58

[Improvement][Metrics] Fix remaining metrics names, update English do…

dcdfeca

…cs and add Chinese docs (apache#10432)

[Improvement][Metrics] Fix metrics names in grafana-demo dashboards (a…

05b95ef

…pache#10432)

EricGao888 marked this pull request as ready for review June 20, 2022 13:23

EricGao888 requested review from Tianqi-Dotes and zhongjiajie as code owners June 20, 2022 13:23

EricGao888 commented Jun 20, 2022

View reviewed changes

kezhenxu94 approved these changes Jun 21, 2022

View reviewed changes

EricGao888 requested a review from ruanwenjun June 21, 2022 06:07

ruanwenjun approved these changes Jun 21, 2022

View reviewed changes

ruanwenjun merged commit cc06eaa into apache:dev Jun 21, 2022

EricGao888 mentioned this pull request Jun 22, 2022

[Improvement][Metrics] Update some metrics names in grafana-demo dashboards #10552

Merged

zhongjiajie added this to the 3.1.0-alpha milestone Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improvement][Metrics] Apply micrometer naming convention to metrics#10477

[Improvement][Metrics] Apply micrometer naming convention to metrics#10477
ruanwenjun merged 4 commits intoapache:devfrom
EricGao888:Fix-10432

EricGao888 commented Jun 16, 2022 •

edited

Loading

Uh oh!

EricGao888 commented Jun 16, 2022

Uh oh!

ruanwenjun Jun 16, 2022

Uh oh!

EricGao888 Jun 16, 2022

Uh oh!

codecov-commenter commented Jun 19, 2022 •

edited

Loading

Uh oh!

kezhenxu94 left a comment

Uh oh!

EricGao888 commented Jun 20, 2022 •

edited

Loading

Uh oh!

kezhenxu94 commented Jun 20, 2022

Uh oh!

EricGao888 commented Jun 20, 2022

Uh oh!

EricGao888 Jun 20, 2022

Uh oh!

sonarqubecloud bot commented Jun 20, 2022

Uh oh!

kezhenxu94 left a comment

Uh oh!

ruanwenjun left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	Counter.builder("ds.system.worker.full.submit.queue.count")
	Counter.builder("ds.worker.full.submit.queue.count")

	- We enable Apache DolphinScheduler export metrics in `standalone` mode to help users get hands dirty easily.
	- We enable Apache DolphinScheduler to export metrics in `standalone` mode to help users get hands dirty easily.

Conversation

EricGao888 commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

EricGao888 commented Jun 16, 2022

Uh oh!

ruanwenjun Jun 16, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Jun 16, 2022

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jun 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kezhenxu94 left a comment

Choose a reason for hiding this comment

Uh oh!

EricGao888 commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kezhenxu94 commented Jun 20, 2022

Uh oh!

EricGao888 commented Jun 20, 2022

Uh oh!

EricGao888 Jun 20, 2022

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 20, 2022

Uh oh!

kezhenxu94 left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

EricGao888 commented Jun 16, 2022 •

edited

Loading

codecov-commenter commented Jun 19, 2022 •

edited

Loading

EricGao888 commented Jun 20, 2022 •

edited

Loading