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

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

                    .register(Metrics.globalRegistry);

    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

Merging #10477 (a45b9e6) into dev (2c227ab) will decrease coverage by 0.00%.
The diff coverage is 43.33%.

@@             Coverage Diff              @@
##                dev   #10477      +/-   ##
============================================
- Coverage     40.86%   40.85%   -0.01%     
+ Complexity     4849     4848       -1     
============================================
  Files           886      886              
  Lines         36009    36032      +23     
  Branches       3994     3998       +4     
============================================
+ Hits          14716    14722       +6     
- Misses        19835    19850      +15     
- Partials       1458     1460       +2

Impacted Files	Coverage Δ
...ler/server/master/metrics/MasterServerMetrics.java	`0.00% <0.00%> (ø)`
...nscheduler/service/process/ProcessServiceImpl.java	`30.76% <ø> (ø)`
...inscheduler/server/worker/metrics/TaskMetrics.java	`0.00% <0.00%> (ø)`
...ler/server/worker/metrics/WorkerServerMetrics.java	`0.00% <0.00%> (ø)`
.../server/worker/processor/TaskExecuteProcessor.java	`0.00% <ø> (ø)`
...inscheduler/plugin/task/zeppelin/ZeppelinTask.java	`48.68% <34.37%> (-7.92%)`	⬇️
.../server/master/metrics/ProcessInstanceMetrics.java	`62.50% <85.71%> (ø)`
...inscheduler/server/master/metrics/TaskMetrics.java	`69.44% <100.00%> (ø)`
...duler/plugin/task/zeppelin/ZeppelinParameters.java	`84.61% <100.00%> (+7.69%)`	⬆️
...er/master/dispatch/host/assign/RandomSelector.java	`77.77% <0.00%> (-5.56%)`	⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 704e633...a45b9e6. Read the comment docs.

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 caishunfeng, SbloodyS 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 zhongjiajie and Tianqi-Dotes 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

EricGao888 commented Jun 16, 2022 •

edited

Loading

EricGao888 commented Jun 16, 2022

ruanwenjun Jun 16, 2022

EricGao888 Jun 16, 2022

codecov-commenter commented Jun 19, 2022

kezhenxu94 left a comment

EricGao888 commented Jun 20, 2022 •

edited

Loading

kezhenxu94 commented Jun 20, 2022

EricGao888 commented Jun 20, 2022

EricGao888 Jun 20, 2022

sonarqubecloud bot commented Jun 20, 2022

kezhenxu94 left a comment

ruanwenjun left a comment

	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.

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

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

Conversation

EricGao888 commented Jun 16, 2022 • edited Loading

Purpose of the pull request

Brief change log

Verify this pull request

EricGao888 commented Jun 16, 2022

ruanwenjun Jun 16, 2022

Choose a reason for hiding this comment

EricGao888 Jun 16, 2022

Choose a reason for hiding this comment

codecov-commenter commented Jun 19, 2022

Codecov Report

kezhenxu94 left a comment

Choose a reason for hiding this comment

EricGao888 commented Jun 20, 2022 • edited Loading

kezhenxu94 commented Jun 20, 2022

EricGao888 commented Jun 20, 2022

EricGao888 Jun 20, 2022

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 20, 2022

kezhenxu94 left a comment

Choose a reason for hiding this comment

ruanwenjun left a comment

Choose a reason for hiding this comment

EricGao888 commented Jun 16, 2022 •

edited

Loading

EricGao888 commented Jun 20, 2022 •

edited

Loading