[#1348] improvement(metrics): Unify tags generation for shuffle-server metrics reporter #1349
Conversation
…-server metrics reporter
|
|
||
| // the value is used for client/server compatible, eg, online upgrade | ||
| public static final String SHUFFLE_SERVER_VERSION = "ss_v4"; | ||
| public static final String METRICS_TAG_LABEL_NAME = "label"; |
There was a problem hiding this comment.
the name of label is not proper in proemetheus.
There was a problem hiding this comment.
cc @smallzhongfeng @xianjingfeng
Is it ok for you? Do you have influence for your production environment?
There was a problem hiding this comment.
If you just change the name, the impact is not bad, but I remember that GRPC or NETTY_GRPC is available. I have tested this before. 🤔️
There was a problem hiding this comment.
I tested in this morning, but I haven't seen this. Like this,
There was a problem hiding this comment.
Well, it may be that a certain PR has rolled back this function. If you can ensure that the tag parameter configuration is correct and the error will still be reported, I agree with this change.
There was a problem hiding this comment.
FYI @leixm The tag function of the online cluster should be normal now, right?
| private Constants() {} | ||
|
|
||
| // the value is used for client/server compatible, eg, online upgrade | ||
| public static final String SHUFFLE_SERVER_VERSION = "ss_v4"; |
There was a problem hiding this comment.
Does this default label also need to be deleted?
There was a problem hiding this comment.
No. This is necessary for upgrade when having in-compatible changes.
|
|
||
| // the value is used for client/server compatible, eg, online upgrade | ||
| public static final String SHUFFLE_SERVER_VERSION = "ss_v4"; | ||
| public static final String METRICS_TAG_LABEL_NAME = "label"; |
There was a problem hiding this comment.
Well, it may be that a certain PR has rolled back this function. If you can ensure that the tag parameter configuration is correct and the error will still be reported, I agree with this change.
|
|
||
| // the value is used for client/server compatible, eg, online upgrade | ||
| public static final String SHUFFLE_SERVER_VERSION = "ss_v4"; | ||
| public static final String METRICS_TAG_LABEL_NAME = "label"; |
There was a problem hiding this comment.
FYI @leixm The tag function of the online cluster should be normal now, right?
|
Could you help review again @smallzhongfeng @leixm |
smallzhongfeng
left a comment
There was a problem hiding this comment.
LTGM, thanks @zuston
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1349 +/- ##
=============================================
+ Coverage 0 54.12% +54.12%
- Complexity 0 2713 +2713
=============================================
Files 0 398 +398
Lines 0 21560 +21560
Branches 0 2041 +2041
=============================================
+ Hits 0 11670 +11670
- Misses 0 9176 +9176
- Partials 0 714 +714 ☔ View full report in Codecov by Sentry. |
* master: (23 commits) format use synchronized format modify to singleton format format format format format change Coordinator to singleton like ShuffleServerClientFactory.java [apache#1189] feat(server): Add netty used direct memory size metric (apache#1363) format format format rename file rename [apache#960] fix(dashboard): simplify dependency and correct the startup script (apache#1347) [apache#1348] improvement(metrics): Unify tags generation for shuffle-server metrics reporter (apache#1349) [MINOR] chore: fix kubernetes ci pipeline (apache#1368) [MINOR] fix(spark): Fix NPE for ShuffleWriteClientImpl.unregisterShuffle (apache#1367) ...
…re_1119_new_2023_12_23 * feature_1119_BUFFER_LIMIT_OF_HUGE_PARTITION: add file for commit [apache#1189] feat(server): Add netty used direct memory size metric (apache#1363) format format format rename file rename [apache#960] fix(dashboard): simplify dependency and correct the startup script (apache#1347) [apache#1348] improvement(metrics): Unify tags generation for shuffle-server metrics reporter (apache#1349) [MINOR] chore: fix kubernetes ci pipeline (apache#1368) [MINOR] fix(spark): Fix NPE for ShuffleWriteClientImpl.unregisterShuffle (apache#1367) format code format code 添加堆外内存监控的代码
…-server metrics reporter (apache#1349) ### What changes were proposed in this pull request? Currently, the tags label is generated by self when registering metrics for shuffle-server. This is wrong, which will lack the GRPC or NETTY_GRPC tags. ### Why are the changes needed? Fix: apache#1348 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Neen't
What changes were proposed in this pull request?
Currently, the tags label is generated by self when registering metrics for shuffle-server. This is wrong, which will lack the GRPC or NETTY_GRPC tags.
Why are the changes needed?
Fix: #1348
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Neen't