hudi-agent commented on code in PR #19504:
URL: https://github.com/apache/hudi/pull/19504#discussion_r3709204500
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java:
##########
@@ -158,26 +158,27 @@ public HoodieMetrics(HoodieWriteConfig config,
HoodieStorage storage) {
this.tableName = config.getTableName();
if (config.isMetricsOn()) {
metrics = Metrics.getInstance(config.getMetricsConfig(), storage);
- this.rollbackTimerName = getMetricsName(HoodieTimeline.ROLLBACK_ACTION,
TIMER_METRIC);
- this.cleanTimerName = getMetricsName(HoodieTimeline.CLEAN_ACTION,
TIMER_METRIC);
- this.archiveTimerName = getMetricsName(ARCHIVE_ACTION, TIMER_METRIC);
- this.commitTimerName = getMetricsName(HoodieTimeline.COMMIT_ACTION,
TIMER_METRIC);
- this.deltaCommitTimerName =
getMetricsName(HoodieTimeline.DELTA_COMMIT_ACTION, TIMER_METRIC);
- this.clusterCommitTimerName =
getMetricsName(HoodieTimeline.CLUSTERING_ACTION, TIMER_METRIC);
- this.finalizeTimerName = getMetricsName(FINALIZE_ACTION, TIMER_METRIC);
- this.compactionTimerName =
getMetricsName(HoodieTimeline.COMPACTION_ACTION, TIMER_METRIC);
- this.logCompactionTimerName =
getMetricsName(HoodieTimeline.LOG_COMPACTION_ACTION, TIMER_METRIC);
- this.indexTimerName = getMetricsName(INDEX_ACTION, TIMER_METRIC);
- this.sourceReadAndIndexTimerName =
getMetricsName(SOURCE_READ_AND_INDEX_ACTION, TIMER_METRIC);
- this.conflictResolutionTimerName =
getMetricsName(CONFLICT_RESOLUTION_STR, TIMER_METRIC);
- this.conflictResolutionSuccessCounterName =
getMetricsName(CONFLICT_RESOLUTION_STR, SUCCESS_COUNTER);
- this.conflictResolutionFailureCounterName =
getMetricsName(CONFLICT_RESOLUTION_STR, FAILURE_COUNTER);
+ // Keep the names of metrics that predate #12543 stable for dashboard
and alert compatibility.
Review Comment:
🤖 Since released 1.x versions already shipped the #12543 (action-first)
names, this restoration effectively re-changes these metric names a second
time, so dashboards built against the interim 1.x names will break. The
description lists "Documentation Update: none" — would it be worth a short
release/upgrade note so users on those 1.x versions know to switch back to the
legacy names?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/metrics/HoodieMetaSyncMetrics.java:
##########
@@ -35,8 +35,8 @@
@Slf4j
public class HoodieMetaSyncMetrics {
- private static final String TIMER_METRIC_EXTENSION = ".timer";
- private static final String COUNTER_METRIC_EXTENSION = ".counter";
+ private static final String TIMER_ACTION = "timer";
Review Comment:
🤖 nit: `TIMER_ACTION` and `COUNTER_ACTION` might be slightly confusing here
since "action" in Hudi conventionally refers to timeline actions (commit,
rollback, …). Could you rename them to something like `TIMER_METRIC_TYPE` /
`COUNTER_METRIC_TYPE` or `TIMER_PREFIX` / `COUNTER_PREFIX` to avoid the
vocabulary collision?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]