rangareddy opened a new pull request, #19476:
URL: https://github.com/apache/hudi/pull/19476
### Describe the issue this Pull Request addresses
Closes #12182 and #13051. Both report the same thing: CloudWatch metrics are
enabled, the job succeeds,
and **no metrics arrive at all**, with this in the logs every interval:
```
ERROR ScheduledReporter: Exception thrown from CloudWatchReporter#report.
Exception was suppressed.
java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 1
at
org.apache.hudi.aws.cloudwatch.CloudWatchReporter.stageMetricDatum(CloudWatchReporter.java:281)
at
org.apache.hudi.aws.cloudwatch.CloudWatchReporter.processGauge(CloudWatchReporter.java:250)
at
org.apache.hudi.aws.cloudwatch.CloudWatchReporter.report(CloudWatchReporter.java:189)
```
`stageMetricDatum` derives the CloudWatch `Table` dimension from the part of
the metric name before the
first dot. A name without a dot therefore cannot be mapped. HUDI-9068
(#12873) put a
`ValidationUtils.checkArgument` there, which turned the
`ArrayIndexOutOfBoundsException` into an
`IllegalArgumentException` — but it still **throws**, and that is the part
that produces the reported
symptom. `report()` stages every gauge, counter, histogram, meter and timer
into one list before calling
`putMetricData`, so throwing part-way through means the request is never
sent: one unmappable name costs
every metric in that interval. `ScheduledReporter` then suppresses the
exception, so the user sees a log
line and an empty dashboard.
#12873 also fixed one producer of dotless names
(`HoodieBackedTableMetadataWriter`'s
`table_service_execution_*`), but not the class of problem. Names without a
dot still reach the reporter on
current master, because `HoodieMetadataMetrics#setMetric` calls
`metrics.registerGauge(action, value)`
with no prefix at all — unlike `Metrics#registerGauges`, which applies
`hoodie.metrics.reporter.metricsname.prefix`. Two live examples:
- `HoodieMetadataMetrics.getStats` registers a bare `partitionCount`.
- `BaseTableMetadata` registers a bare
`lookup_meta_index_bloom_filters_file_count`.
So any table with the metadata table enabled and `CLOUDWATCH` selected still
loses all of its metrics.
### Summary and Changelog
- `stageMetricDatum` now skips a metric it cannot map instead of throwing,
so the rest of the batch is
still published. The name is logged at warn level, once per name rather
than once per interval, so a
persistent offender does not spam the log every reporting cycle.
This intentionally changes what #12873 introduced, so to be explicit about
why: the goal there — do not
silently report a metric under a wrong or missing table dimension — is
preserved, because the metric is
still not reported and is now named in a warning. What changes is that it no
longer takes the other
metrics down with it. Fail-fast is not reachable here in any case:
`ScheduledReporter` catches and
suppresses everything `report()` throws, so the throw could never surface to
a caller — it could only
delete the batch.
`testReportOnMetricsWithoutTableName`, which asserted the throw, is replaced
by
`testReportSkipsMetricsWithoutTableNameAndPublishesTheRest`, which pins the
new contract: the unmappable
metric is skipped and the well-formed one in the same batch is still
published, with the right dimensions.
### Verification
Reproduced first, using a name master actually produces (`partitionCount`)
alongside a well-formed one:
```
@@@THREW@@@ IllegalArgumentException: metricName doesn't follow the naming
convention and doesn't
contain a dot as splitter! metricName:partitionCount
@@@RESULT@@@ putMetricData was never called - every metric in the batch was
lost
```
After the change, `putMetricData` is called once with exactly one datum,
`testPrefix.gauge2`, carrying the
`Table=testTable` dimension. The new test is red with the production change
reverted:
```
[ERROR] testReportSkipsMetricsWithoutTableNameAndPublishesTheRest -- ERROR!
java.lang.IllegalArgumentException: metricName doesn't follow the naming
convention and doesn't contain
a dot as splitter! metricName:partitionCount
```
Whole `hudi-aws` module: `Tests run: 94, Failures: 0, Errors: 0, Skipped:
16` (skips pre-existing).
`checkstyle:check` and `apache-rat:check` clean.
### Not in this PR
The naming inconsistency behind it. `HoodieMetadataMetrics#setMetric`
bypasses the prefix that
`Metrics#registerGauges` applies, which has two consequences for CloudWatch
beyond the crash:
- dotless names such as `partitionCount` are now skipped, so those metrics
still do not reach CloudWatch;
- names that contain a dot but no table, such as
`lookup_meta_index_bloom_filters.count`, are reported with
the *action* as the `Table` dimension.
Fixing that means prefixing metadata metric names with the table, which
renames them for every reporter —
Graphite, Prometheus, JMX, Datadog — and would break existing dashboards.
That is a maintainer call rather
than something to slip into a crash fix, so I have left it and am happy to
follow up in a separate PR if
you would like it done.
### Impact
CloudWatch users whose tables register a dotless metric name go from
receiving no metrics at all to
receiving every well-formed metric. No effect on any other reporter, and no
effect on a CloudWatch setup
that never produces such a name. No API, config or table format change.
### Risk Level
low — one branch in one reporter, scoped to a case that currently throws.
Verified that the well-formed
metrics publish with correct dimensions, that the new test fails without the
change, and that the rest of
`hudi-aws` is unaffected.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
- [x] CI passes on my PR
--
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]
