fix: restore pre-#12543 metric names - #19504
Conversation
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for working on this! This PR reverts the metric-name reordering introduced by #12543, restoring the pre-#12543 (type-first) timer and counter names in HoodieMetrics, HoodieMetaSyncMetrics, and HoodieStreamerMetrics while preserving the naming of metrics added afterward. I traced each restored name against the constant values and the original #12543 diff, and the revert is faithful and well-covered by literal-name assertions. One cross-release compatibility note is worth a look in the inline comment. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. One small naming nit in HoodieMetaSyncMetrics — otherwise the changes look clean and well-commented.
| 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. |
There was a problem hiding this 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?
|
|
||
| private static final String TIMER_METRIC_EXTENSION = ".timer"; | ||
| private static final String COUNTER_METRIC_EXTENSION = ".counter"; | ||
| private static final String TIMER_ACTION = "timer"; |
There was a problem hiding this 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?
4 participants
Describe the issue this Pull Request addresses
PR #12543 reordered the components used to construct several metric names. Although it was marked as a minor change, it changed existing timer and counter names in Hudi 1.x, breaking dashboards and alerts carried forward from 0.x. Compaction counters also gained a duplicated
compaction.compactionsegment.This PR restores the pre-#12543 names for metrics that already had a compatibility contract while preserving the naming of metrics added after #12543.
Summary and Changelog
HoodieMetrics, including compaction and conflict-resolution counters.HoodieMetaSyncMetricsand legacy timer names inHoodieStreamerMetrics.Impact
Restores 0.x-compatible names such as
timer.commit,counter.compaction.requested, andcounter.conflict_resolution.success. Dashboards and alerts using the pre-#12543 names work across the 1.x upgrade. Consumers that adopted the post-#12543 names for these existing metrics will need to switch back to the legacy names.There are no public API, configuration, storage-format, or performance changes. Metrics introduced after #12543 retain their current names.
Risk Level
medium
The change intentionally alters emitted names for the affected pre-existing metrics. The compatibility boundary is covered by literal name assertions with and without prefixes, and by the following focused test suites:
TestHoodieMetrics(25 tests)TestHoodieMetaSyncMetrics(3 tests)TestHoodieStreamerMetrics(7 tests)TestHoodieCompactor(11 tests)Documentation Update
none. This restores the established metric naming contract and does not introduce a feature, configuration, or new metric.
Contributor's checklist