This is an automated email from the ASF dual-hosted git repository.
voonhous pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new 637996c5ab20 fix(metrics): explain how to enable the CloudWatch
reporter when hudi-aws is absent (#19418)
637996c5ab20 is described below
commit 637996c5ab2078dfd858a2a425da8f28af1ce789
Author: Ranga Reddy <[email protected]>
AuthorDate: Mon Aug 3 14:06:50 2026 +0530
fix(metrics): explain how to enable the CloudWatch reporter when hudi-aws
is absent (#19418)
* fix(metrics): explain how to enable the CloudWatch reporter when hudi-aws
is absent
The CloudWatch reporter lives in the optional hudi-aws module and is loaded
reflectively, but hudi-spark-bundle does not shade that module. Selecting
hoodie.metrics.reporter.type=CLOUDWATCH with only hudi-spark-bundle on the
classpath therefore failed with "Unable to load class", which names neither
the missing class nor a remedy.
Report the missing class, point at hudi-aws-bundle, and name the config to
change instead. Other reflection failures are rethrown unchanged.
Closes #15293
* fix(common): name the class in the generic reflective-load failure
Review feedback on the CloudWatch-specific message.
ReflectionUtils.getClass threw "Unable to load class" with no class name,
which
is the message every reflective load in Hudi produces on a missing class:
payload classes, key generators, index types, sources, sinks, write clients.
Naming the class there fixes half of the reported defect for all of them,
and
the CloudWatch remedy now layers on an already-useful base message.
Also from review:
- Dropped the hudi-spark-bundle references from the user-facing message and
the
javadoc. Naming one bundle's contents in hudi-common couples the string to
packaging with nothing keeping them in sync, and it is noise for a Flink
or
Kafka Connect user who reaches the line another way.
- Made CLOUDWATCH_REPORTER_CLASS package-private with @VisibleForTesting so
the
tests reference it instead of repeating the FQCN in three places.
- Added two tests that pin the branch deterministically rather than by
classpath: a ClassNotFoundException cause is rewritten, and any other
failure
passes through unchanged so an unrelated instantiation error never
becomes a
misleading "add hudi-aws-bundle".
* test(metrics): stop naming a specific bundle in the CloudWatch test
javadoc
The javadoc pinned the test's rationale to hudi-spark-bundle not shading
hudi-aws. That is a packaging fact with nothing keeping it in sync: if that
bundle ever shades hudi-aws, which is what #15293 asks for, the comment goes
stale silently. The reason the test works is that the module's test
classpath
lacks hudi-aws, which is what any non-shading bundle looks like, so say that
instead. Same reasoning already applied to the user-facing message and the
production javadoc in bf4ef23.
---------
Co-authored-by: voonhous <[email protected]>
---
.../hudi/metrics/MetricsReporterFactory.java | 30 +++++++++-
.../hudi/metrics/TestMetricsReporterFactory.java | 64 +++++++++++++++++++++-
.../apache/hudi/common/util/ReflectionUtils.java | 2 +-
3 files changed, 92 insertions(+), 4 deletions(-)
diff --git
a/hudi-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java
b/hudi-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java
index 93ad8ddc0583..403d25b9ead6 100644
---
a/hudi-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java
+++
b/hudi-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java
@@ -22,6 +22,7 @@ import
org.apache.hudi.common.config.metrics.HoodieMetricsConfig;
import org.apache.hudi.common.util.Option;
import org.apache.hudi.common.util.ReflectionUtils;
import org.apache.hudi.common.util.StringUtils;
+import org.apache.hudi.common.util.VisibleForTesting;
import org.apache.hudi.exception.HoodieException;
import org.apache.hudi.metrics.custom.CustomizableMetricsReporter;
import org.apache.hudi.metrics.datadog.DatadogMetricsReporter;
@@ -40,6 +41,10 @@ import java.util.Properties;
@Slf4j
public class MetricsReporterFactory {
+ @VisibleForTesting
+ static final String CLOUDWATCH_REPORTER_CLASS =
+ "org.apache.hudi.aws.metrics.cloudwatch.CloudWatchMetricsReporter";
+
public static Option<MetricsReporter> createReporter(HoodieMetricsConfig
metricsConfig, MetricRegistry registry) {
String reporterClassName = metricsConfig.getMetricReporterClassName();
@@ -84,8 +89,7 @@ public class MetricsReporterFactory {
reporter = new ConsoleMetricsReporter(registry);
break;
case CLOUDWATCH:
- reporter = (MetricsReporter)
ReflectionUtils.loadClass("org.apache.hudi.aws.metrics.cloudwatch.CloudWatchMetricsReporter",
- new Class[]{HoodieMetricsConfig.class, MetricRegistry.class},
metricsConfig, registry);
+ reporter = createCloudWatchReporter(metricsConfig, registry);
break;
case M3:
reporter = new M3MetricsReporter(metricsConfig, registry);
@@ -99,4 +103,26 @@ public class MetricsReporterFactory {
}
return Option.ofNullable(reporter);
}
+
+ /**
+ * The CloudWatch reporter ships in the optional {@code hudi-aws} module and
so is loaded reflectively.
+ * Not every engine bundle shades that module, in which case class loading
fails without pointing at a
+ * remedy. Translate that into an actionable error.
+ */
+ private static MetricsReporter createCloudWatchReporter(HoodieMetricsConfig
metricsConfig, MetricRegistry registry) {
+ try {
+ return (MetricsReporter)
ReflectionUtils.loadClass(CLOUDWATCH_REPORTER_CLASS,
+ new Class[] {HoodieMetricsConfig.class, MetricRegistry.class},
metricsConfig, registry);
+ } catch (HoodieException e) {
+ if (e.getCause() instanceof ClassNotFoundException) {
+ throw new HoodieException(String.format(
+ "Cannot report metrics to CloudWatch: %s was not found on the
classpath. It ships in the "
+ + "optional hudi-aws module, which not every engine bundle
includes. Add the "
+ + "hudi-aws-bundle jar matching your Hudi version to the
classpath, or set %s to a "
+ + "different reporter type.",
+ CLOUDWATCH_REPORTER_CLASS,
HoodieMetricsConfig.METRICS_REPORTER_TYPE_VALUE.key()), e);
+ }
+ throw e;
+ }
+ }
}
diff --git
a/hudi-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
b/hudi-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
index 68a85572086d..d1bcee365873 100644
---
a/hudi-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
+++
b/hudi-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java
@@ -82,7 +82,7 @@ class TestMetricsReporterFactory {
try (MockedStatic<ReflectionUtils> mockedStatic =
Mockito.mockStatic(ReflectionUtils.class)) {
mockedStatic.when(() ->
ReflectionUtils.loadClass(
-
eq("org.apache.hudi.aws.metrics.cloudwatch.CloudWatchMetricsReporter"),
+ eq(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
any(Class[].class),
eq(metricsConfig),
eq(registry)
@@ -94,6 +94,68 @@ class TestMetricsReporterFactory {
}
}
+ /**
+ * {@code hudi-aws} is deliberately absent from this module's test
classpath, which is exactly the
+ * situation a user hits on an engine bundle that does not shade that
module: the reflectively loaded
+ * CloudWatch reporter cannot be found. The failure must name the missing
class and how to fix it,
+ * not just report that some class could not be loaded.
+ */
+ @Test
+ void
metricsReporterFactoryShouldExplainHowToEnableCloudWatchWhenHudiAwsIsMissing() {
+
when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.CLOUDWATCH);
+
+ HoodieException exception = assertThrows(HoodieException.class,
+ () -> MetricsReporterFactory.createReporter(metricsConfig, registry));
+
+ String message = exception.getMessage();
+
assertTrue(message.contains(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
+ () -> "The failure should name the missing reporter class, but was: "
+ message);
+ assertTrue(message.contains("hudi-aws-bundle"),
+ () -> "The failure should name the bundle that provides the reporter,
but was: " + message);
+
assertTrue(message.contains(HoodieMetricsConfig.METRICS_REPORTER_TYPE_VALUE.key()),
+ () -> "The failure should name the config to change, but was: " +
message);
+ }
+
+ /**
+ * The classpath-based test above exercises the real failure but passes for
any resolution failure.
+ * This pins the mapping itself: a ClassNotFoundException cause is what
triggers the rewrite.
+ */
+ @Test
+ void metricsReporterFactoryRewritesClassNotFoundIntoAnActionableMessage() {
+
when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.CLOUDWATCH);
+ try (MockedStatic<ReflectionUtils> mockedStatic =
Mockito.mockStatic(ReflectionUtils.class)) {
+ mockedStatic.when(() -> ReflectionUtils.loadClass(
+ eq(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
any(Class[].class), eq(metricsConfig), eq(registry)))
+ .thenThrow(new HoodieException("Unable to load class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
+ new
ClassNotFoundException(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS)));
+
+ HoodieException exception = assertThrows(HoodieException.class,
+ () -> MetricsReporterFactory.createReporter(metricsConfig,
registry));
+ assertTrue(exception.getMessage().contains("hudi-aws-bundle"),
+ () -> "Expected the remedy to be named, but was: " +
exception.getMessage());
+ }
+ }
+
+ /**
+ * The other direction, and the branch most likely to regress: a failure
that is not a missing class must
+ * pass through untouched, so an unrelated instantiation error is never
rewritten into "add hudi-aws-bundle".
+ */
+ @Test
+ void metricsReporterFactoryLeavesNonClassNotFoundFailuresUntouched() {
+
when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.CLOUDWATCH);
+ try (MockedStatic<ReflectionUtils> mockedStatic =
Mockito.mockStatic(ReflectionUtils.class)) {
+ mockedStatic.when(() -> ReflectionUtils.loadClass(
+ eq(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
any(Class[].class), eq(metricsConfig), eq(registry)))
+ .thenThrow(new HoodieException("Unable to instantiate class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
+ new NoSuchMethodException("<init>")));
+
+ HoodieException exception = assertThrows(HoodieException.class,
+ () -> MetricsReporterFactory.createReporter(metricsConfig,
registry));
+ assertEquals("Unable to instantiate class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
+ exception.getMessage(), "A non-ClassNotFound failure must not be
rewritten");
+ }
+ }
+
@Test
void metricsReporterFactoryShouldReturnUserDefinedReporter() {
when(metricsConfig.getMetricReporterClassName()).thenReturn(DummyMetricsReporter.class.getName());
diff --git
a/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
b/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
index cb4cbc4290f8..6a6bd436ab41 100644
--- a/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
+++ b/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
@@ -50,7 +50,7 @@ public class ReflectionUtils {
try {
return Class.forName(c);
} catch (ClassNotFoundException e) {
- throw new HoodieException("Unable to load class", e);
+ throw new HoodieException("Unable to load class " + c, e);
}
});
}
