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 c4b38935db04 fix(metrics): route the reflection failures a CloudWatch
skew actually produces (#19477)
c4b38935db04 is described below
commit c4b38935db0462eca6b352db60bfc0155a4cf0c8
Author: Ranga Reddy <[email protected]>
AuthorDate: Thu Aug 6 10:02:29 2026 +0530
fix(metrics): route the reflection failures a CloudWatch skew actually
produces (#19477)
* fix(metrics): tell a CloudWatch reporter version mismatch apart from a
missing bundle
ReflectionUtils.loadClass collapses NoSuchMethodException,
InvocationTargetException, InstantiationException and
IllegalAccessException into
one HoodieException with a fixed message, so a hudi-aws jar built against a
different Hudi version surfaced only as "Unable to instantiate class
org.apache.hudi.metrics.cloudwatch.CloudWatchMetricsReporter". Working out
that the
class had resolved and only its constructor had not matched meant digging
the
buried NoSuchMethodException out of the stack.
Translate that cause too: report that the class was found but has no
(HoodieMetricsConfig, MetricRegistry) constructor, and that the remedy is a
hudi-aws-bundle matching the Hudi bundle in use. That is deliberately
different from
the missing-module message, which says to add the bundle - absent and
mismatched
need different fixes, and conflating them is what made the message
unhelpful.
metricsReporterFactoryLeavesNonClassNotFoundFailuresUntouched used a
NoSuchMethodException to stand for "some other failure", which is now
handled, so it
switches to an InvocationTargetException and is renamed accordingly.
* test(metrics): fix a dropped word in an assertion message
Review nit: "A failure that is neither cause must not be rewritten" was
missing
the two cases it refers to.
* fix(metrics): route the reflection failures a CloudWatch skew actually
produces
Review found that the constructor-mismatch branch cannot fire for the case
its message
described, and I reproduced it: with a released hudi-aws-bundle-1.1.0-rc1
on the
classpath against this branch's hudi-common,
STEP1 Class.forName OK -> class
org.apache.hudi.aws.metrics.cloudwatch.CloudWatchMetricsReporter
STEP2 getConstructor THREW java.lang.NoClassDefFoundError:
org/apache/hudi/config/metrics/HoodieMetricsConfig
is NoSuchMethodException? false
is Error (escapes catch(Exception))? true
Class#getConstructor resolves the parameter types of every public
constructor, not just
the requested one, and #19193 moved HoodieMetricsConfig out of
org.apache.hudi.config.metrics with no stub there. So a clean older bundle
dies on the
vanished type, as an Error that ReflectionUtils never wraps, and the
previous
catch (HoodieException) never saw it. NoSuchMethodException only arrives
when the
classpath carries a stale or duplicate copy - which is also what #12902
actually was:
that report had matching 0.15.0 artifacts, and 0.15.0 did declare the
requested
constructor.
Three causes, three remedies:
- ClassNotFoundException -> hudi-aws absent, add the bundle.
- NoClassDefFoundError -> built against a different Hudi; the message
quotes the type
that vanished, which is the strongest evidence of skew available.
- NoSuchMethodException -> stale or duplicate copy; look for more than one
Hudi version,
a leftover hudi-common or hudi-client-common being the usual culprit.
Parameter types are printed fully qualified. The released bundle declares
(org.apache.hudi.config.metrics.HoodieMetricsConfig,
org.apache.hudi.com.codahale.metrics.MetricRegistry) while this Hudi
requests
(org.apache.hudi.common.config.metrics.HoodieMetricsConfig,
com.codahale.metrics.MetricRegistry) - under simple names both read as
(HoodieMetricsConfig, MetricRegistry) and the error looks like it is lying.
Tests: both ClassNotFound tests now assert their own distinguishing phrase,
since every
string they checked also appears in the mismatch message; the mismatch test
pins the
NoSuchMethodException into the cause chain, which nothing did before; and
one test drives
the real ReflectionUtils with a fixture whose only public constructor does
not match, so
the suite no longer asserts only the shapes the production code assumes.
That gap is how
the NoClassDefFoundError case stayed invisible.
---
.../hudi/metrics/MetricsReporterFactory.java | 45 ++++++++-
.../hudi/metrics/TestMetricsReporterFactory.java | 107 ++++++++++++++++++++-
2 files changed, 143 insertions(+), 9 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 403d25b9ead6..9bc7f30fcd27 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
@@ -106,13 +106,33 @@ public class MetricsReporterFactory {
/**
* 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.
+ * Reflection collapses several unrelated failures into the same opaque
{@link HoodieException}. Three of
+ * them have distinct remedies - the module is absent, it was built against
a Hudi that has since moved a
+ * class, or the classpath carries a stale duplicate - so translate those
three, and leave every other
+ * failure untouched.
*/
private static MetricsReporter createCloudWatchReporter(HoodieMetricsConfig
metricsConfig, MetricRegistry registry) {
+ return createCloudWatchReporter(CLOUDWATCH_REPORTER_CLASS, metricsConfig,
registry);
+ }
+
+ @VisibleForTesting
+ static MetricsReporter createCloudWatchReporter(String reporterClass,
HoodieMetricsConfig metricsConfig,
+ MetricRegistry registry) {
try {
- return (MetricsReporter)
ReflectionUtils.loadClass(CLOUDWATCH_REPORTER_CLASS,
+ return (MetricsReporter) ReflectionUtils.loadClass(reporterClass,
new Class[] {HoodieMetricsConfig.class, MetricRegistry.class},
metricsConfig, registry);
+ } catch (NoClassDefFoundError e) {
+ // Class#getConstructor resolves the parameter types of every public
constructor, not just the one
+ // asked for, so a jar built against an older Hudi fails here on a type
that has since moved - not
+ // with a missing-constructor error. NoClassDefFoundError is an Error,
so ReflectionUtils never wraps
+ // it and it arrives here uncaught. Its message names the type that
vanished, which is the strongest
+ // evidence of skew available.
+ throw new HoodieException(String.format(
+ "Cannot report metrics to CloudWatch: %s was found on the classpath
but was built against a "
+ + "different Hudi version - resolving its constructors needs %s,
which this Hudi no longer "
+ + "provides. Use a hudi-aws-bundle of the same version as the
engine bundle, or set %s to a "
+ + "different reporter type.",
+ reporterClass, e.getMessage(),
HoodieMetricsConfig.METRICS_REPORTER_TYPE_VALUE.key()), e);
} catch (HoodieException e) {
if (e.getCause() instanceof ClassNotFoundException) {
throw new HoodieException(String.format(
@@ -120,7 +140,24 @@ public class MetricsReporterFactory {
+ "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);
+ reporterClass,
HoodieMetricsConfig.METRICS_REPORTER_TYPE_VALUE.key()), e);
+ }
+ if (e.getCause() instanceof NoSuchMethodException) {
+ // The class resolved and so did every constructor's parameter types,
yet none matched. A jar built
+ // against an older Hudi fails earlier, in the NoClassDefFoundError
branch above, so what reaches
+ // here is a classpath carrying a stale or duplicate copy of this
class or of its parameter types.
+ // Fully qualified names, because the package move and the
shaded-codahale relocation are exactly
+ // what distinguishes the declared constructor from the requested one
- under simple names both read
+ // as (HoodieMetricsConfig, MetricRegistry) and the error looks wrong.
+ throw new HoodieException(String.format(
+ "Cannot report metrics to CloudWatch: %s was found on the
classpath but does not declare a "
+ + "(%s, %s) constructor. Some jar on the classpath is
supplying a stale or duplicate copy "
+ + "of this class or of its parameter types. Check for more
than one Hudi version on the "
+ + "classpath - a leftover hudi-common or hudi-client-common is
the usual culprit - and "
+ + "align every Hudi artifact, including the engine bundle, to
one version. Or set %s to a "
+ + "different reporter type.",
+ reporterClass, HoodieMetricsConfig.class.getName(),
+ MetricRegistry.class.getName(),
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 d1bcee365873..29af024ed4eb 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
@@ -37,6 +37,7 @@ import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
+import java.lang.reflect.InvocationTargetException;
import java.util.Properties;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -114,6 +115,10 @@ class TestMetricsReporterFactory {
() -> "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);
+ // Every string above also appears in the constructor-mismatch message, so
without this the two branches
+ // could be merged or reordered and both tests would still pass.
+ assertTrue(message.contains("was not found on the classpath"),
+ () -> "A missing class must not be reported as a constructor mismatch,
but was: " + message);
}
/**
@@ -133,26 +138,107 @@ class TestMetricsReporterFactory {
() -> MetricsReporterFactory.createReporter(metricsConfig,
registry));
assertTrue(exception.getMessage().contains("hudi-aws-bundle"),
() -> "Expected the remedy to be named, but was: " +
exception.getMessage());
+ assertTrue(exception.getMessage().contains("was not found on the
classpath"),
+ () -> "Expected this branch's own phrase, not one shared with the
mismatch branch, 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".
+ * A jar built against an older Hudi does not reach this branch - resolving
its constructors fails first
+ * with {@link NoClassDefFoundError}, covered below. What reaches here is a
classpath carrying a stale or
+ * duplicate copy, so that is the remedy the message has to give.
*/
@Test
- void metricsReporterFactoryLeavesNonClassNotFoundFailuresUntouched() {
+ void metricsReporterFactoryExplainsAConstructorMismatch() {
+ HoodieException exception = captureCloudWatchFailure(
+ new HoodieException("Unable to instantiate class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
+ new NoSuchMethodException("<init>")));
+
+ String message = exception.getMessage();
+ assertTrue(message.contains("constructor"),
+ () -> "The failure should say the constructor did not match, but was:
" + message);
+ assertTrue(message.contains("stale or duplicate copy"),
+ () -> "The failure should name a stale duplicate as the cause, but
was: " + message);
+
assertTrue(message.contains(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
+ () -> "The failure should name the reporter class, but was: " +
message);
+
assertTrue(message.contains(HoodieMetricsConfig.METRICS_REPORTER_TYPE_VALUE.key()),
+ () -> "The failure should name the config to change, but was: " +
message);
+ assertTrue(message.contains(HoodieMetricsConfig.class.getName())
+ && message.contains(MetricRegistry.class.getName()),
+ () -> "Fully qualified parameter types are what distinguish the
requested constructor from the "
+ + "declared one, but was: " + message);
+ // Positive rather than an assertFalse on the other branch's prose: a
vacuous assertFalse never fails.
+ assertTrue(message.contains("was found on the classpath but"),
+ () -> "A class that resolved must not be reported as missing, but was:
" + message);
+ assertEquals(NoSuchMethodException.class,
exception.getCause().getCause().getClass(),
+ "The original NoSuchMethodException must stay in the chain - it is the
evidence #12902 needed");
+ }
+
+ /**
+ * The clean version-skew case, and the one the mismatch message used to
claim. {@code getConstructor}
+ * resolves the parameter types of every public constructor, so a jar built
against an older Hudi dies on a
+ * type that has since moved. That is an {@link Error}, so {@code
ReflectionUtils} never wraps it and it
+ * reaches the factory uncaught.
+ */
+ @Test
+ void metricsReporterFactoryExplainsAVanishedParameterType() {
+ HoodieException exception = captureCloudWatchFailure(
+ new
NoClassDefFoundError("org/apache/hudi/config/metrics/HoodieMetricsConfig"));
+
+ String message = exception.getMessage();
+ assertTrue(message.contains("built against a different Hudi version"),
+ () -> "The failure should name version skew, but was: " + message);
+
assertTrue(message.contains("org/apache/hudi/config/metrics/HoodieMetricsConfig"),
+ () -> "The failure should name the type that vanished, but was: " +
message);
+ assertEquals(NoClassDefFoundError.class, exception.getCause().getClass(),
+ "The Error must stay in the chain - its message is the evidence of
skew");
+ }
+
+ /**
+ * The gap every mocked test above shares: none of them proves that real
reflection produces the shapes the
+ * production code branches on. This drives the translation through the real
{@code ReflectionUtils} with a
+ * fixture whose only public constructor does not match, and asserts on the
actual throwable.
+ */
+ @Test
+ void metricsReporterFactoryTranslatesARealReflectionFailure() {
+ HoodieException exception = assertThrows(HoodieException.class,
+ () -> MetricsReporterFactory.createCloudWatchReporter(
+ MismatchedReporter.class.getName(), metricsConfig, registry));
+
+ String message = exception.getMessage();
+ assertTrue(message.contains("stale or duplicate copy"),
+ () -> "A real non-matching constructor should reach the mismatch
branch, but was: " + message);
+ assertEquals(NoSuchMethodException.class,
exception.getCause().getCause().getClass(),
+ "and the real NoSuchMethodException should be chained");
+ }
+
+ /** Public, with a single constructor that deliberately does not match
(HoodieMetricsConfig, MetricRegistry). */
+ public static class MismatchedReporter {
+ public MismatchedReporter(String somethingElse) {
+ // never called; exists so getConstructor has a public constructor to
reject
+ }
+ }
+
+ /**
+ * The other direction, and the branch most likely to regress: a failure
that is neither a missing class
+ * nor a missing constructor must pass through untouched, so an error raised
by the reporter's own
+ * constructor is never rewritten into a classpath diagnosis.
+ */
+ @Test
+ void metricsReporterFactoryLeavesOtherFailuresUntouched() {
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>")));
+ new InvocationTargetException(new IllegalStateException("no AWS
region configured"))));
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");
+ exception.getMessage(),
+ "A failure that is neither a missing class nor a missing constructor
must not be rewritten");
}
}
@@ -177,6 +263,17 @@ class TestMetricsReporterFactory {
assertThrows(HoodieException.class, () ->
MetricsReporterFactory.createReporter(metricsConfig, registry));
}
+ private HoodieException captureCloudWatchFailure(Throwable
reflectionFailure) {
+
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(reflectionFailure);
+ return assertThrows(HoodieException.class,
+ () -> MetricsReporterFactory.createReporter(metricsConfig,
registry));
+ }
+ }
+
public static class DummyMetricsReporter extends CustomizableMetricsReporter
{
public DummyMetricsReporter(Properties props, MetricRegistry registry) {
