obelix74 commented on code in PR #3468:
URL: https://github.com/apache/polaris/pull/3468#discussion_r2706347561
##########
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:
@dimas-b I was primarily thinking about external systems. For instance, if
someone does not want to use the built in metrics persistence and forwards it
to some other system. In the past (in different data oriented systems, I have
run into problems where if someone wants to correlate an "event" based on time,
there was an issue) - I understand if a `traceId` is present, this is a
non-issue, perhaps I am being overly defensive in considering cases where the
clients do not send a `traceId`.
Without the parameter:
• Custom implementations that want to forward metrics to external systems
(CloudWatch, Prometheus, InfluxDB, Kafka) must capture their own timestamp
• That timestamp would be captured inside the reporter implementation, after
the metrics have already passed through several layers of processing
• This creates timing drift and inconsistency across different
implementations
With the parameter:
• The timestamp is captured at the point of receipt in
IcebergCatalogAdapter.reportMetrics():
``` metricsReporter.reportMetric(
catalogName, tableIdentifier, reportMetricsRequest.report(),
System.currentTimeMillis());```
• All implementations receive the same, consistent timestamp representing
when Polaris received
the metrics
Important distinction: The MetricsReport from Iceberg contains duration data
(how long a scan took), but not wall-clock time (when it happened). The
timestampMs provides that missing piece.
--
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]
