[Improvement][Metrics] Switch to use tags to indicate task / workflow execution status for metrics by EricGao888 · Pull Request #11128 · apache/dolphinscheduler

EricGao888 · 2022-07-24T08:45:42Z

Purpose of the pull request

This PR closes: [Improvement][Metrics] Use tags to indicate task / workflow execution status for metrics #10867
related: [DSIP-8][Metrics] Improve DolphinScheduler Monitoring #9324

Brief change log

Use tags instead of names to indicate task / workflow instance execution state / status for metrics.
Will update docs / grafana-demos in following commits.

Verify this pull request

Verified by manual tests.

… execution status for metrics (apache#10867)

codecov-commenter · 2022-07-24T13:48:41Z

Codecov Report

Merging #11128 (011040f) into dev (71cf7e1) will increase coverage by 0.04%.
The diff coverage is 33.33%.

❗ Current head 011040f differs from pull request most recent head 9dd91b8. Consider uploading reports for the commit 9dd91b8 to get more accurate results

@@             Coverage Diff              @@
##                dev   #11128      +/-   ##
============================================
+ Coverage     40.37%   40.41%   +0.04%     
- Complexity     4873     4881       +8     
============================================
  Files           950      950              
  Lines         37187    37186       -1     
  Branches       4079     4078       -1     
============================================
+ Hits          15014    15030      +16     
+ Misses        20662    20632      -30     
- Partials       1511     1524      +13

Impacted Files	Coverage Δ
...r/api/service/impl/DqExecuteResultServiceImpl.java	`70.37% <ø> (-1.06%)`	⬇️
...che/dolphinscheduler/common/utils/HadoopUtils.java	`15.42% <33.33%> (+9.45%)`	⬆️
...r/plugin/task/sqoop/parameter/SqoopParameters.java	`53.33% <0.00%> (-1.34%)`	⬇️
...r/plugin/registry/zookeeper/ZookeeperRegistry.java	`55.85% <0.00%> (-0.91%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

sonarqubecloud · 2022-07-24T13:57:33Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

34.9% Coverage
2.2% Duplication

ruanwenjun

Basically LGTM, some nit.

ruanwenjun · 2022-07-25T11:51:32Z

.../src/main/java/org/apache/dolphinscheduler/server/master/metrics/ProcessInstanceMetrics.java

+    private static Map<String, Counter> PROCESS_INSTANCE_COUNTERS = new HashMap<>();
+
+    private static final Set<String> PROCESS_INSTANCE_STATES = ImmutableSet.of(
+            "submit", "timeout", "finish", "failover", "success", "fail", "stop");


It's better to directly use ExecutionStatus.SUCCESS.getCode() to replace the success, and use

ProcessInstanceMetrics.incProcessInstanceByState(ExecutionStatus.SUCCESS.getCode());

to increase the counter.

Sure, thx for the suggestion. Will fix it.

@ruanwenjun Hi I just checked this file

dolphinscheduler/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/enums/ExecutionStatus.java

Lines 27 to 68 in 011040f

public enum ExecutionStatus {

/**

* status：

* 0 submit success

* 1 running

* 2 ready pause

* 3 pause

* 4 ready stop

* 5 stop

* 6 failure

* 7 success

* 8 need fault tolerance

* 9 kill

* 10 waiting thread

* 11 waiting depend node complete

* 12 delay execution

* 13 forced success

* 14 serial wait

* 15 ready block

* 16 block

* 17 dispatch

*/

SUBMITTED_SUCCESS(0, "submit success"),

RUNNING_EXECUTION(1, "running"),

READY_PAUSE(2, "ready pause"),

PAUSE(3, "pause"),

READY_STOP(4, "ready stop"),

STOP(5, "stop"),

FAILURE(6, "failure"),

SUCCESS(7, "success"),

NEED_FAULT_TOLERANCE(8, "need fault tolerance"),

KILL(9, "kill"),

WAITING_THREAD(10, "waiting thread"),

WAITING_DEPEND(11, "waiting depend node complete"),

DELAY_EXECUTION(12, "delay execution"),

FORCED_SUCCESS(13, "forced success"),

SERIAL_WAIT(14, "serial wait"),

READY_BLOCK(15, "ready block"),

BLOCK(16, "block"),

DISPATCH(17, "dispatch"),

;

It seems those states in workflow metrics and task metrics are not fully covered by ExecutionStatus e.g. failover and finish. What about keeping it this way for consistency and easier intelligibility?

This is history problem, ExecutionStatus need to split into two class WorkflowExecutionStatus and TaskExecutionStatus, some states are only belong to workflow, and some states are only belong to task, and some status are deprecated.

This is history problem, ExecutionStatus need to split into two class WorkflowExecutionStatus and TaskExecutionStatus, some states are only belong to workflow, and some states are only belong to task, and some status are deprecated.

@ruanwenjun Considering the legacy here, what should we do in this PR? Maybe I could open a new issue to split this ExecutionStatus ?

I opened a new issue to split ExecutionStatus and remove deprecated states in it: #11145

Another thing is if we use ExecutionStatus.SUCCESS.getCode() for tag, it might be a little bit unclear on the Grafana side as the tag value is integer in that case?

[Improvement][Metrics] Switch to use tags to indicate task / workflow…

9aa11d0

… execution status for metrics (apache#10867)

github-actions bot added the backend label Jul 24, 2022

EricGao888 self-assigned this Jul 24, 2022

EricGao888 added this to the 3.1.0-alpha milestone Jul 24, 2022

EricGao888 added metrics improvement make more easy to user or prompt friendly labels Jul 24, 2022

EricGao888 added 2 commits July 24, 2022 17:40

Clean up code

da4b635

Update docs and grafana demo dashboards

a0fd81a

github-actions bot added the document label Jul 24, 2022

EricGao888 marked this pull request as ready for review July 24, 2022 13:27

EricGao888 requested review from SbloodyS, Tianqi-Dotes, caishunfeng, kezhenxu94, ruanwenjun and zhongjiajie as code owners July 24, 2022 13:27

Fix import order

9dd91b8

kezhenxu94 approved these changes Jul 25, 2022

View reviewed changes

ruanwenjun approved these changes Jul 25, 2022

View reviewed changes

EricGao888 mentioned this pull request Jul 26, 2022

[Improvement][Task] Split ExecutionStatus for workflow and task respectively #11145

Closed

3 tasks

EricGao888 merged commit 0e15ce3 into apache:dev Jul 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improvement][Metrics] Switch to use tags to indicate task / workflow execution status for metrics#11128

[Improvement][Metrics] Switch to use tags to indicate task / workflow execution status for metrics#11128
EricGao888 merged 4 commits intoapache:devfrom
EricGao888:Fix-10867

EricGao888 commented Jul 24, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented Jul 24, 2022 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Jul 24, 2022

Uh oh!

ruanwenjun left a comment

Uh oh!

ruanwenjun Jul 25, 2022

Uh oh!

EricGao888 Jul 25, 2022

Uh oh!

EricGao888 Jul 25, 2022

Uh oh!

ruanwenjun Jul 25, 2022

Uh oh!

EricGao888 Jul 25, 2022

Uh oh!

EricGao888 Jul 26, 2022

Uh oh!

EricGao888 Jul 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	public enum ExecutionStatus {

	/**
	* status：
	* 0 submit success
	* 1 running
	* 2 ready pause
	* 3 pause
	* 4 ready stop
	* 5 stop
	* 6 failure
	* 7 success
	* 8 need fault tolerance
	* 9 kill
	* 10 waiting thread
	* 11 waiting depend node complete
	* 12 delay execution
	* 13 forced success
	* 14 serial wait
	* 15 ready block
	* 16 block
	* 17 dispatch
	*/
	SUBMITTED_SUCCESS(0, "submit success"),
	RUNNING_EXECUTION(1, "running"),
	READY_PAUSE(2, "ready pause"),
	PAUSE(3, "pause"),
	READY_STOP(4, "ready stop"),
	STOP(5, "stop"),
	FAILURE(6, "failure"),
	SUCCESS(7, "success"),
	NEED_FAULT_TOLERANCE(8, "need fault tolerance"),
	KILL(9, "kill"),
	WAITING_THREAD(10, "waiting thread"),
	WAITING_DEPEND(11, "waiting depend node complete"),
	DELAY_EXECUTION(12, "delay execution"),
	FORCED_SUCCESS(13, "forced success"),
	SERIAL_WAIT(14, "serial wait"),
	READY_BLOCK(15, "ready block"),
	BLOCK(16, "block"),
	DISPATCH(17, "dispatch"),
	;

Conversation

EricGao888 commented Jul 24, 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!

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

Uh oh!

Codecov Report

Uh oh!

sonarqubecloud bot commented Jul 24, 2022

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun Jul 25, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Jul 25, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Jul 25, 2022

Choose a reason for hiding this comment

Uh oh!

ruanwenjun Jul 25, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Jul 25, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Jul 26, 2022

Choose a reason for hiding this comment

Uh oh!

EricGao888 Jul 26, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EricGao888 commented Jul 24, 2022 •

edited

Loading

codecov-commenter commented Jul 24, 2022 •

edited

Loading