singhpk234 commented on code in PR #3468:
URL: https://github.com/apache/polaris/pull/3468#discussion_r2714870291


##########
runtime/service/src/main/java/org/apache/polaris/service/reporting/PolarisMetricsReporter.java:
##########
@@ -18,9 +18,39 @@
  */
 package org.apache.polaris.service.reporting;
 
+import java.time.Instant;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.metrics.MetricsReport;
 
+/**
+ * SPI interface for reporting Iceberg metrics received by Polaris.
+ *
+ * <p>Implementations can be used to send metrics to external systems for 
analysis and monitoring.
+ * Custom implementations can be annotated with appropriate {@code Quarkus} 
scope and {@link
+ * io.smallrye.common.annotation.Identifier @Identifier("my-reporter-type")} 
for CDI discovery.
+ *
+ * <p>The implementation to use is selected via the {@code 
polaris.iceberg-metrics.reporting.type}
+ * configuration property, which defaults to {@code "default"}.
+ *
+ * <p>Implementations can inject other CDI beans for context.
+ *
+ * @see DefaultMetricsReporter
+ * @see MetricsReportingConfiguration
+ */
 public interface PolarisMetricsReporter {
-  public void reportMetric(String catalogName, TableIdentifier table, 
MetricsReport metricsReport);

Review Comment:
   @dimas-b can we not add the api in the same interface and add the default 
impl of old api call this null clock, i would like to understand this more if 
this is possible. If its not possible i am supportive of the change !
   
   > @singhpk234 : please clarify whether you consider this comment thread a 
blocker for merging (or not).
   
    thanks for asking the clarification i believe anything is thats not marked 
as 'nit' / 'not blocker' / 'not' is expected to be resolved before merging (per 
[here](https://polaris.apache.org/community/contributing-guidelines/#code-contribution-guidelines))
 , i really appreciate you checking in ! 



-- 
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]

Reply via email to