smattheis commented on a change in pull request #18991:
URL: https://github.com/apache/flink/pull/18991#discussion_r820776221
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/TaskIOMetricGroup.java
##########
@@ -100,6 +104,10 @@ public TaskIOMetricGroup(TaskMetricGroup parent) {
hardBackPressuredTimePerSecond::getMaxSingleMeasurement);
this.busyTimePerSecond = gauge(MetricNames.TASK_BUSY_TIME,
this::getBusyTimePerSecond);
+
+ this.mailboxThroughput = meter(MetricNames.MAILBOX_THROUGHPUT, new
MeterView(60));
+ this.mailboxLatency =
+ histogram(MetricNames.MAILBOX_LATENCY, new
DescriptiveStatisticsHistogram(60));
Review comment:
In fact, it is the default parameter. The odd thing is that the
constructor of MeterView requires either new SimpleCounter() or
timeSpanInSeconds as parameter. So it would be necessary to add std
constructors to both MeterView and DescrHistogram. However, I thought this is
the best place to set the value and have it consistent for both mailbox metrics
even though it's hard coded.
##########
File path: docs/content.zh/docs/ops/metrics.md
##########
@@ -1328,7 +1328,7 @@ Note that the metrics are only available via reporters.
<td>Histogram</td>
</tr>
<tr>
- <th rowspan="20"><strong>Task</strong></th>
+ <th rowspan="22"><strong>Task</strong></th>
Review comment:
There is the TaskMetricGroup which contains the TaskMetricIOGroup.
However, the TaskMetricGroup seemed to me as the place used by the application
developer to add custom metrics. Though I'm not sure about it.
If TaskMetricIOGroup is not the right place, it seems that we need to add
new MetricGroup to TaskMetricGroup. What do you think?
--
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...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
