johnjcasey commented on code in PR #33408:
URL: https://github.com/apache/beam/pull/33408#discussion_r1892970875
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/MetricsToPerStepNamespaceMetricsConverter.java:
##########
@@ -86,6 +87,38 @@ private static Optional<MetricValue>
convertCounterToMetricValue(
.setValueInt64(value));
}
+ /**
+ * @param metricName The {@link MetricName} that represents this counter.
+ * @param value The counter value.
+ * @return If the conversion succeeds, {@code MetricValue} that represents
this counter. Otherwise
+ * returns an empty optional
+ */
+ private static Optional<MetricValue> convertGaugeToMetricValue(
+ MetricName metricName,
+ Long value,
+ Map<MetricName, LabeledMetricNameUtils.ParsedMetricName>
parsedPerWorkerMetricsCache) {
+
+ if
((!metricName.getNamespace().equals(BigQuerySinkMetrics.METRICS_NAMESPACE)
Review Comment:
If so, I'd convert it to be an or condition, not a pair of nots and an and
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/MetricsToPerStepNamespaceMetricsConverter.java:
##########
@@ -86,6 +87,38 @@ private static Optional<MetricValue>
convertCounterToMetricValue(
.setValueInt64(value));
}
+ /**
+ * @param metricName The {@link MetricName} that represents this counter.
+ * @param value The counter value.
+ * @return If the conversion succeeds, {@code MetricValue} that represents
this counter. Otherwise
+ * returns an empty optional
+ */
+ private static Optional<MetricValue> convertGaugeToMetricValue(
+ MetricName metricName,
+ Long value,
+ Map<MetricName, LabeledMetricNameUtils.ParsedMetricName>
parsedPerWorkerMetricsCache) {
+
+ if
((!metricName.getNamespace().equals(BigQuerySinkMetrics.METRICS_NAMESPACE)
Review Comment:
We probably shouldn't do this check here, and separate out the check from
the conversion itself. To be clear, this is checking that the namespace is
either BQ or Kafka, right?
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/MetricsToPerStepNamespaceMetricsConverter.java:
##########
@@ -86,6 +87,38 @@ private static Optional<MetricValue>
convertCounterToMetricValue(
.setValueInt64(value));
}
+ /**
+ * @param metricName The {@link MetricName} that represents this counter.
+ * @param value The counter value.
+ * @return If the conversion succeeds, {@code MetricValue} that represents
this counter. Otherwise
+ * returns an empty optional
+ */
+ private static Optional<MetricValue> convertGaugeToMetricValue(
+ MetricName metricName,
+ Long value,
+ Map<MetricName, LabeledMetricNameUtils.ParsedMetricName>
parsedPerWorkerMetricsCache) {
+
+ if
((!metricName.getNamespace().equals(BigQuerySinkMetrics.METRICS_NAMESPACE)
Review Comment:
or check that the namespace is in the set of supported namespaces
--
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]
