This is an automated email from the ASF dual-hosted git repository.
blue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new dc8efe10db Core: Add initialize method for MetricsReporter. (#7014)
dc8efe10db is described below
commit dc8efe10dbc71cc4a511219f2569a0f8c88fda94
Author: wangyinsheng <[email protected]>
AuthorDate: Wed Mar 8 08:59:15 2023 +0800
Core: Add initialize method for MetricsReporter. (#7014)
---
.../org/apache/iceberg/metrics/MetricsReporter.java | 11 +++++++++++
.../java/org/apache/iceberg/BaseMetastoreCatalog.java | 7 +------
core/src/main/java/org/apache/iceberg/CatalogUtil.java | 13 +++++++++++--
.../org/apache/iceberg/rest/RESTSessionCatalog.java | 7 +------
.../test/java/org/apache/iceberg/TestCatalogUtil.java | 18 ++++++++++++++----
5 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/metrics/MetricsReporter.java
b/api/src/main/java/org/apache/iceberg/metrics/MetricsReporter.java
index 5fae755bbe..365f7f99d6 100644
--- a/api/src/main/java/org/apache/iceberg/metrics/MetricsReporter.java
+++ b/api/src/main/java/org/apache/iceberg/metrics/MetricsReporter.java
@@ -18,10 +18,21 @@
*/
package org.apache.iceberg.metrics;
+import java.util.Map;
+
/** This interface defines the basic API for reporting metrics for operations
to a Table. */
@FunctionalInterface
public interface MetricsReporter {
+ /**
+ * A custom MetricsReporter implementation must have a no-arg constructor,
which will be called
+ * first. {@link MetricsReporter#initialize(Map properties)} is called to
complete the
+ * initialization.
+ *
+ * @param properties properties
+ */
+ default void initialize(Map<String, String> properties) {}
+
/**
* Indicates that an operation is done by reporting a {@link MetricsReport}.
A {@link
* MetricsReport} is usually directly derived from a {@link MetricsReport}
instance.
diff --git a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
index f35aa29df0..9a09e62cb6 100644
--- a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
@@ -25,7 +25,6 @@ import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.io.InputFile;
-import org.apache.iceberg.metrics.LoggingMetricsReporter;
import org.apache.iceberg.metrics.MetricsReporter;
import org.apache.iceberg.relocated.com.google.common.base.MoreObjects;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -308,11 +307,7 @@ public abstract class BaseMetastoreCatalog implements
Catalog {
private MetricsReporter metricsReporter() {
if (metricsReporter == null) {
- metricsReporter =
- properties().containsKey(CatalogProperties.METRICS_REPORTER_IMPL)
- ? CatalogUtil.loadMetricsReporter(
- properties().get(CatalogProperties.METRICS_REPORTER_IMPL))
- : LoggingMetricsReporter.instance();
+ metricsReporter = CatalogUtil.loadMetricsReporter(properties());
}
return metricsReporter;
diff --git a/core/src/main/java/org/apache/iceberg/CatalogUtil.java
b/core/src/main/java/org/apache/iceberg/CatalogUtil.java
index ef4d17c249..31cb63556c 100644
--- a/core/src/main/java/org/apache/iceberg/CatalogUtil.java
+++ b/core/src/main/java/org/apache/iceberg/CatalogUtil.java
@@ -35,6 +35,7 @@ import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.hadoop.Configurable;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.SupportsBulkOperations;
+import org.apache.iceberg.metrics.LoggingMetricsReporter;
import org.apache.iceberg.metrics.MetricsReporter;
import org.apache.iceberg.relocated.com.google.common.base.Joiner;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -407,12 +408,18 @@ public class CatalogUtil {
*
* <p>The implementation must have a no-arg constructor.
*
- * @param impl full class name of a custom {@link MetricsReporter}
implementation
+ * @param properties catalog properties which contains class name of a
custom {@link
+ * MetricsReporter} implementation
* @return An initialized {@link MetricsReporter}.
* @throws IllegalArgumentException if class path not found or right
constructor not found or the
* loaded class cannot be cast to the given interface type
*/
- public static MetricsReporter loadMetricsReporter(String impl) {
+ public static MetricsReporter loadMetricsReporter(Map<String, String>
properties) {
+ String impl = properties.get(CatalogProperties.METRICS_REPORTER_IMPL);
+ if (impl == null) {
+ return LoggingMetricsReporter.instance();
+ }
+
LOG.info("Loading custom MetricsReporter implementation: {}", impl);
DynConstructors.Ctor<MetricsReporter> ctor;
try {
@@ -437,6 +444,8 @@ public class CatalogUtil {
e);
}
+ reporter.initialize(properties);
+
return reporter;
}
}
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
index fb8e56c292..da170a30a3 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
@@ -57,7 +57,6 @@ import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.hadoop.Configurable;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.ResolvingFileIO;
-import org.apache.iceberg.metrics.LoggingMetricsReporter;
import org.apache.iceberg.metrics.MetricsReport;
import org.apache.iceberg.metrics.MetricsReporter;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -199,11 +198,7 @@ public class RESTSessionCatalog extends BaseSessionCatalog
mergedProps, REST_SNAPSHOT_LOADING_MODE,
SnapshotMode.ALL.name())
.toUpperCase(Locale.US));
- String metricsReporterImpl =
mergedProps.get(CatalogProperties.METRICS_REPORTER_IMPL);
- this.reporter =
- null != metricsReporterImpl
- ? CatalogUtil.loadMetricsReporter(metricsReporterImpl)
- : LoggingMetricsReporter.instance();
+ this.reporter = CatalogUtil.loadMetricsReporter(mergedProps);
this.reportingViaRestEnabled =
PropertyUtil.propertyAsBoolean(mergedProps,
REST_METRICS_REPORTING_ENABLED, true);
diff --git a/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java
b/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java
index 2c139adcf4..75c27c7fc8 100644
--- a/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java
+++ b/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java
@@ -31,6 +31,7 @@ import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.io.OutputFile;
import org.apache.iceberg.metrics.MetricsReport;
import org.apache.iceberg.metrics.MetricsReporter;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
@@ -193,16 +194,21 @@ public class TestCatalogUtil {
public void loadCustomMetricsReporter_noArg() {
Map<String, String> properties = Maps.newHashMap();
properties.put("key", "val");
+ properties.put(
+ CatalogProperties.METRICS_REPORTER_IMPL,
TestMetricsReporterDefault.class.getName());
- MetricsReporter metricsReporter =
-
CatalogUtil.loadMetricsReporter(TestMetricsReporterDefault.class.getName());
+ MetricsReporter metricsReporter =
CatalogUtil.loadMetricsReporter(properties);
Assertions.assertThat(metricsReporter).isInstanceOf(TestMetricsReporterDefault.class);
}
@Test
public void loadCustomMetricsReporter_badArg() {
Assertions.assertThatThrownBy(
- () ->
CatalogUtil.loadMetricsReporter(TestMetricsReporterBadArg.class.getName()))
+ () ->
+ CatalogUtil.loadMetricsReporter(
+ ImmutableMap.of(
+ CatalogProperties.METRICS_REPORTER_IMPL,
+ TestMetricsReporterBadArg.class.getName())))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("missing no-arg constructor");
}
@@ -210,7 +216,11 @@ public class TestCatalogUtil {
@Test
public void loadCustomMetricsReporter_badClass() {
Assertions.assertThatThrownBy(
- () ->
CatalogUtil.loadMetricsReporter(TestFileIONotImpl.class.getName()))
+ () ->
+ CatalogUtil.loadMetricsReporter(
+ ImmutableMap.of(
+ CatalogProperties.METRICS_REPORTER_IMPL,
+ TestFileIONotImpl.class.getName())))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("does not implement MetricsReporter");
}
