obelix74 commented on code in PR #3468:
URL: https://github.com/apache/polaris/pull/3468#discussion_r2706385002
##########
runtime/service/src/main/java/org/apache/polaris/service/reporting/PolarisMetricsReporter.java:
##########
@@ -22,5 +22,6 @@
import org.apache.iceberg.metrics.MetricsReport;
public interface PolarisMetricsReporter {
- public void reportMetric(String catalogName, TableIdentifier table,
MetricsReport metricsReport);
+ void reportMetric(
+ String catalogName, TableIdentifier table, MetricsReport metricsReport,
long timestampMs);
Review Comment:
Valid point about clock skew @dimas-b. You're right that cross-node
synchronization isn't guaranteed.
The main benefit is consistency within the metrics pipeline - capturing the
timestamp at receipt in `IcebergCatalogAdapter` rather than letting each
implementation capture it independently at different points in processing. It's
also useful for correlating with other events on the same node during debugging.
That said, I'm not strongly attached to this parameter. If you think it adds
more complexity than value, happy to remove it - the backward-compatible
default method makes either approach work. Let me know your preference.
--
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]
