You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "brandboat (via GitHub)" <gi...@apache.org> on 2023/09/10 12:19:02 UTC
[GitHub] [yunikorn-core] brandboat opened a new pull request, #643: [YUNIKORN-1967] Update the help message in the application_total metric
brandboat opened a new pull request, #643:
URL: https://github.com/apache/yunikorn-core/pull/643
### What is this PR for?
The help message of the application_total metric lack of `failed` state.
https://github.com/apache/yunikorn-core/blob/master/pkg/metrics/scheduler.go#L89
application_total metric record applications under failed state
https://github.com/apache/yunikorn-core/blob/master/pkg/scheduler/objects/application_state.go#L233
### What type of PR is it?
* [ ] - Bug Fix
* [x] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1967
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] brandboat commented on a diff in pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on code in PR #643:
URL: https://github.com/apache/yunikorn-core/pull/643#discussion_r1320924298
##########
pkg/metrics/scheduler.go:
##########
@@ -86,7 +86,7 @@ func InitSchedulerMetrics() *SchedulerMetrics {
Namespace: Namespace,
Subsystem: SchedulerSubsystem,
Name: "application_total",
- Help: "Total number of applications. State of the application includes `running` and `completed`.",
+ Help: "Total number of applications. State of the application includes `running`, `completed` and `failed`.",
Review Comment:
Correct me if I'm wrong, but I didn't see those labels (i.e. accepted/rejected) in https://github.com/apache/yunikorn-core/blob/1540a5508db1816f11645ab86781a4685de08ebc/pkg/metrics/scheduler.go#L267-L317. Not sure where are these two labels implemented ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] wusamzong commented on a diff in pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
Posted by "wusamzong (via GitHub)" <gi...@apache.org>.
wusamzong commented on code in PR #643:
URL: https://github.com/apache/yunikorn-core/pull/643#discussion_r1320930962
##########
pkg/metrics/scheduler.go:
##########
@@ -86,7 +86,7 @@ func InitSchedulerMetrics() *SchedulerMetrics {
Namespace: Namespace,
Subsystem: SchedulerSubsystem,
Name: "application_total",
- Help: "Total number of applications. State of the application includes `running` and `completed`.",
+ Help: "Total number of applications. State of the application includes `running`, `completed` and `failed`.",
Review Comment:
These two labels are implemented in [here](https://github.com/apache/yunikorn-core/blob/master/pkg/metrics/scheduler.go#L251-L265)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] wusamzong commented on a diff in pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
Posted by "wusamzong (via GitHub)" <gi...@apache.org>.
wusamzong commented on code in PR #643:
URL: https://github.com/apache/yunikorn-core/pull/643#discussion_r1320956508
##########
pkg/metrics/scheduler.go:
##########
@@ -86,7 +86,7 @@ func InitSchedulerMetrics() *SchedulerMetrics {
Namespace: Namespace,
Subsystem: SchedulerSubsystem,
Name: "application_total",
- Help: "Total number of applications. State of the application includes `running` and `completed`.",
+ Help: "Total number of applications. State of the application includes `running`, `completed` and `failed`.",
Review Comment:
Sorry I missed the part about application_submission_total in metrics/scheduler.go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] wusamzong commented on a diff in pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
Posted by "wusamzong (via GitHub)" <gi...@apache.org>.
wusamzong commented on code in PR #643:
URL: https://github.com/apache/yunikorn-core/pull/643#discussion_r1320921730
##########
pkg/metrics/scheduler.go:
##########
@@ -86,7 +86,7 @@ func InitSchedulerMetrics() *SchedulerMetrics {
Namespace: Namespace,
Subsystem: SchedulerSubsystem,
Name: "application_total",
- Help: "Total number of applications. State of the application includes `running` and `completed`.",
+ Help: "Total number of applications. State of the application includes `running`, `completed` and `failed`.",
Review Comment:
I think the state can also be either 'accepted' or 'rejected'[1], and we can also include these states in the message!
[1] [metrics/init.go#78](https://github.com/apache/yunikorn-core/blob/master/pkg/metrics/init.go#L78C38-L78C38)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] codecov[bot] commented on pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #643:
URL: https://github.com/apache/yunikorn-core/pull/643#issuecomment-1712803413
## [Codecov](https://app.codecov.io/gh/apache/yunikorn-core/pull/643?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#643](https://app.codecov.io/gh/apache/yunikorn-core/pull/643?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5088ab2) into [master](https://app.codecov.io/gh/apache/yunikorn-core/commit/1540a5508db1816f11645ab86781a4685de08ebc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (1540a55) will **decrease** coverage by `0.03%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #643 +/- ##
==========================================
- Coverage 77.66% 77.63% -0.03%
==========================================
Files 79 79
Lines 13058 13058
==========================================
- Hits 10141 10138 -3
- Misses 2589 2591 +2
- Partials 328 329 +1
```
| [Files Changed](https://app.codecov.io/gh/apache/yunikorn-core/pull/643?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg/metrics/scheduler.go](https://app.codecov.io/gh/apache/yunikorn-core/pull/643?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `50.40% <100.00%> (ø)` | |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/yunikorn-core/pull/643/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-core] manirajv06 closed pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
Posted by "manirajv06 (via GitHub)" <gi...@apache.org>.
manirajv06 closed pull request #643: [YUNIKORN-1967] Update the help message in the application_total metric
URL: https://github.com/apache/yunikorn-core/pull/643
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org