amogh-jahagirdar commented on code in PR #11416:
URL: https://github.com/apache/iceberg/pull/11416#discussion_r1820097463
##########
core/src/main/java/org/apache/iceberg/SnapshotScan.java:
##########
@@ -154,7 +155,17 @@ public CloseableIterable<T> planFiles() {
.scanMetrics(ScanMetricsResult.fromScanMetrics(scanMetrics()))
.metadata(metadata)
.build();
- context().metricsReporter().report(scanReport);
+ MetricsReporter metricsReporter = context().metricsReporter();
+ if (metricsReporter != null) {
+ try {
+ metricsReporter.report(scanReport);
+ } catch (Exception e) {
+ LOG.warn(
+ "MetricsReporter threw an exception. metricsReporter={}",
+ metricsReporter.getClass().getName(),
Review Comment:
I feel like this should all be handled by the metrics reporter
implementation, e.g. check out
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/RESTMetricsReporter.java#L62
for example or
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/metrics/MetricsReporters.java#L72
The benefit of that approach is that we don't have to do do this every place
metrics reporter attempts to report metrics, it's all internal to the
implementation
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
