lukecwik commented on a change in pull request #17094:
URL: https://github.com/apache/beam/pull/17094#discussion_r831570907
##########
File path:
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/MonitoringInfoMetricName.java
##########
@@ -19,59 +19,57 @@
import static
org.apache.beam.vendor.guava.v26_0_jre.com.google.common.base.Preconditions.checkArgument;
-import java.util.HashMap;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Objects;
import org.apache.beam.model.pipeline.v1.MetricsApi;
import org.apache.beam.sdk.metrics.MetricName;
import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.base.Strings;
+import
org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableMap;
import org.checkerframework.checker.nullness.qual.Nullable;
/**
* An implementation of {@code MetricKey} based on a MonitoringInfo's URN and
label to represent the
* key instead of only a name+namespace. This is useful when defining system
defined metrics with a
* specific urn via a {@code CounterContainer}.
*/
-@SuppressWarnings({
- "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
-})
public class MonitoringInfoMetricName extends MetricName {
- private String urn;
- private Map<String, String> labels = new HashMap<String, String>();
+ private final String urn;
+ private final Map<String, String> labels;
private MonitoringInfoMetricName(String urn, Map<String, String> labels) {
checkArgument(!Strings.isNullOrEmpty(urn), "MonitoringInfoMetricName urn
must be non-empty");
checkArgument(labels != null, "MonitoringInfoMetricName labels must be
non-null");
// TODO(ajamato): Move SimpleMonitoringInfoBuilder to
:runners:core-construction-java
// and ensure all necessary labels are set for the specific URN.
this.urn = urn;
- for (Entry<String, String> entry : labels.entrySet()) {
- this.labels.put(entry.getKey(), entry.getValue());
- }
+ this.labels = ImmutableMap.copyOf(labels);
}
@Override
public String getNamespace() {
- if (labels.containsKey(MonitoringInfoConstants.Labels.NAMESPACE)) {
- // User-generated metric
- return labels.getOrDefault(MonitoringInfoConstants.Labels.NAMESPACE,
null);
- } else if (labels.containsKey(MonitoringInfoConstants.Labels.PCOLLECTION))
{
- // System-generated metric
- return labels.getOrDefault(MonitoringInfoConstants.Labels.PCOLLECTION,
null);
- } else if (labels.containsKey(MonitoringInfoConstants.Labels.PTRANSFORM)) {
- // System-generated metric
- return labels.getOrDefault(MonitoringInfoConstants.Labels.PTRANSFORM,
null);
- } else {
- return urn.split(":", 2)[0];
+ // User-generated metric
+ String ret = labels.get(MonitoringInfoConstants.Labels.NAMESPACE);
+ if (ret != null) {
+ return ret;
+ }
+ // System-generated metric
+ ret = labels.get(MonitoringInfoConstants.Labels.PCOLLECTION);
+ if (ret != null) {
+ return ret;
+ }
+ // System-generated metric
+ ret = labels.get(MonitoringInfoConstants.Labels.PTRANSFORM);
+ if (ret != null) {
+ return ret;
}
+ return urn.split(":", 2)[0];
}
@Override
public String getName() {
if (labels.containsKey(MonitoringInfoConstants.Labels.NAME)) {
Review comment:
nit: ditto on not looking in the map multiple times
--
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]
