rdblue commented on code in PR #5780:
URL: https://github.com/apache/iceberg/pull/5780#discussion_r974838907
##########
api/src/main/java/org/apache/iceberg/metrics/ScanReport.java:
##########
@@ -419,86 +154,60 @@ public static class ScanMetrics {
public static final String TOTAL_FILE_SIZE_IN_BYTES =
"total-file-size-in-bytes";
public static final String TOTAL_DELETE_FILE_SIZE_IN_BYTES =
"total-delete-file-size-in-bytes";
public static final String SKIPPED_DATA_MANIFESTS =
"skipped-data-manifests";
- private final Timer totalPlanningDuration;
- private final Counter resultDataFiles;
- private final Counter resultDeleteFiles;
- private final Counter totalDataManifests;
- private final Counter totalDeleteManifests;
- private final Counter scannedDataManifests;
- private final Counter skippedDataManifests;
- private final Counter totalFileSizeInBytes;
- private final Counter totalDeleteFileSizeInBytes;
-
- public ScanMetrics(MetricsContext metricsContext) {
- Preconditions.checkArgument(null != metricsContext, "Invalid metrics
context: null");
- this.totalPlanningDuration =
- metricsContext.timer(TOTAL_PLANNING_DURATION, TimeUnit.NANOSECONDS);
- this.resultDataFiles = metricsContext.counter(RESULT_DATA_FILES,
MetricsContext.Unit.COUNT);
- this.resultDeleteFiles =
- metricsContext.counter(RESULT_DELETE_FILES,
MetricsContext.Unit.COUNT);
- this.scannedDataManifests =
- metricsContext.counter(SCANNED_DATA_MANIFESTS,
MetricsContext.Unit.COUNT);
- this.totalDataManifests =
- metricsContext.counter(TOTAL_DATA_MANIFESTS,
MetricsContext.Unit.COUNT);
- this.totalDeleteManifests =
- metricsContext.counter(TOTAL_DELETE_MANIFESTS,
MetricsContext.Unit.COUNT);
- this.totalFileSizeInBytes =
- metricsContext.counter(TOTAL_FILE_SIZE_IN_BYTES,
MetricsContext.Unit.BYTES);
- this.totalDeleteFileSizeInBytes =
- metricsContext.counter(TOTAL_DELETE_FILE_SIZE_IN_BYTES,
MetricsContext.Unit.BYTES);
- this.skippedDataManifests =
- metricsContext.counter(SKIPPED_DATA_MANIFESTS,
MetricsContext.Unit.COUNT);
+
+ public static ScanMetrics noop() {
+ return ScanMetrics.of(MetricsContext.nullMetrics());
}
+ public abstract MetricsContext metricsContext();
+
+ @Value.Derived
public Timer totalPlanningDuration() {
- return totalPlanningDuration;
+ return metricsContext().timer(TOTAL_PLANNING_DURATION,
TimeUnit.NANOSECONDS);
Review Comment:
Wouldn't it be simpler just to use a `ScanMetrics.from(MetricsContext)`
method so that `ScanMetrics` doesn't need to keep track of the context? The
`from` method could create all of the metrics and pass them into the builder
and we wouldn't need all the generated code for initialization and making sure
`MetricsContext` is only called once per counter/timer. If we also make these
required, then there should be no problem with them being null.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
