Coe review changes

Author: robsunday

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/AttributeMatcher.java

@@ -5,38 +5,41 @@
 
 package io.opentelemetry.contrib.jmxscraper.assertions;
 
-import static io.opentelemetry.contrib.jmxscraper.assertions.AttributeValueMatcher.ANY_VALUE_MATCHER;
-
 import java.util.Objects;
 
 /** Implements functionality of matching data point attributes. */
 public class AttributeMatcher {
-  private final String name;
-  private final AttributeValueMatcher attributeValueMatcher;
+  private final String attributeName;
+  private final String attributeValue;
 
   /**
-   * Create instance used to match data point attribute with te same name and with any value.
+   * Create instance used to match data point attribute with any value.
    *
-   * @param name attribute name
+   * @param attributeName matched attribute name
    */
-  AttributeMatcher(String name) {
-    this.name = name;
-    this.attributeValueMatcher = ANY_VALUE_MATCHER;
+  AttributeMatcher(String attributeName) {
+    this.attributeName = attributeName;
+    this.attributeValue = null;
   }
 
   /**
    * Create instance used to match data point attribute with te same name and with the same value.
    *
-   * @param name attribute name
-   * @param value attribute value
+   * @param attributeName attribute name
+   * @param attributeValue attribute value
    */
-  AttributeMatcher(String name, String value) {
-    this.name = name;
-    this.attributeValueMatcher = new AttributeValueMatcher(value);
+  AttributeMatcher(String attributeName, String attributeValue) {
+    this.attributeName = attributeName;
+    this.attributeValue = attributeValue;
   }
 
-  public String getName() {
-    return name;
+  /**
+   * Return name of data point attribute that this AttributeMatcher is supposed to match value with.
+   *
+   * @return name of validated attribute
+   */
+  public String getAttributeName() {
+    return attributeName;
   }
 
   @Override
@@ -48,21 +51,33 @@ public boolean equals(Object o) {
       return false;
     }
     AttributeMatcher other = (AttributeMatcher) o;
-    return Objects.equals(name, other.name)
-        && attributeValueMatcher.matchValue(other.attributeValueMatcher);
+    return Objects.equals(attributeName, other.attributeName);
   }
 
   @Override
   public int hashCode() {
     // Do not use value matcher here to support value wildcards
-    return Objects.hash(name);
+    return Objects.hash(attributeName);
   }
 
   @Override
   public String toString() {
-    return attributeValueMatcher.getValue() == null
-        ? '{' + name + '}'
-        : '{' + name + '=' + attributeValueMatcher.getValue() + '}';
+    return attributeValue == null
+        ? '{' + attributeName + '}'
+        : '{' + attributeName + '=' + attributeValue + '}';
+  }
+
+  /**
+   * Verify if this matcher is matching provided attribute value. If this matcher holds null value
+   * then it is matching any attribute value
+   *
+   * @param value a value to be matched
+   * @return true if this matcher is matching provided value, false otherwise.
+   */
+  boolean matchesValue(String value) {
+    if ((attributeValue == null) || (value == null)) {
+      return true;
+    }
+    return Objects.equals(attributeValue, value);
   }
-  ;
 }

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/AttributeValueMatcher.java

@@ -16,15 +16,45 @@
 public class DataPointAttributes {
   private DataPointAttributes() {}
 
+  /**
+   * Create instance of matcher that will be used to check if data point attribute with given name
+   * has provided value.
+   *
+   * @param name name of the data point attribute to check
+   * @param value expected value of data point attribute
+   * @return instance of matcher
+   */
   public static AttributeMatcher attribute(String name, String value) {
     return new AttributeMatcher(name, value);
   }
 
+  /**
+   * Create instance of matcher that will be used to check if data point attribute with given name
+   * exists. Any value of the attribute is considered as matching.
+   *
+   * @param name name of the data point attribute to check
+   * @return instance of matcher
+   */
   public static AttributeMatcher attributeWithAnyValue(String name) {
     return new AttributeMatcher(name);
   }
 
+  /**
+   * Create a set of attribute matchers that will be used to verify set of data point attributes.
+   *
+   * @param attributes list of matchers to create set. It must contain matchers with unique names.
+   * @return set of unique attribute matchers
+   * @throws IllegalArgumentException if provided list contains two or more matchers with the same
+   *     name.
+   * @see MetricAssert#hasDataPointsWithAttributes(Set[]) for detailed description of the algorithm
+   *     of matching
+   */
   public static Set<AttributeMatcher> attributeSet(AttributeMatcher... attributes) {
-    return Arrays.stream(attributes).collect(Collectors.toSet());
+    Set<AttributeMatcher> matcherSet = Arrays.stream(attributes).collect(Collectors.toSet());
+    if (matcherSet.size() < attributes.length) {
+      throw new IllegalArgumentException(
+          "Duplicated matchers found in " + Arrays.toString(attributes));
+    }
+    return matcherSet;
   }
 }

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/DataPointAttributes.java

@@ -5,17 +5,17 @@
 
 package io.opentelemetry.contrib.jmxscraper.assertions;
 
-import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attribute;
+import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeSet;
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import io.opentelemetry.proto.common.v1.KeyValue;
 import io.opentelemetry.proto.metrics.v1.Metric;
 import io.opentelemetry.proto.metrics.v1.NumberDataPoint;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
@@ -234,64 +234,78 @@ private void dataPointsCommonCheck(List<NumberDataPoint> dataPoints) {
   }
 
   /**
-   * Verifies that all data points have the same expected one attribute
+   * Verifies that all metric data points have the same expected one attribute
    *
-   * @param expectedAttribute expected attribute
+   * @param expectedAttribute attribute matcher to validate data points attributes
    * @return this
    */
   @CanIgnoreReturnValue
   public final MetricAssert hasDataPointsWithOneAttribute(AttributeMatcher expectedAttribute) {
-    return hasDataPointsWithAttributes(Collections.singleton(expectedAttribute));
+    return hasDataPointsWithAttributes(attributeSet(expectedAttribute));
   }
 
   /**
-   * Verifies that every provided attribute matcher set matches attributes of at least one metric
-   * data point. Attribute matcher set is considered matching data point attributes if each matcher
-   * matches exactly one data point attribute
+   * Verifies that every data point attributes set is matched by one of matcher sets provided. Data
+   * point attributes set is matched by matcher set if each attribute is matched by one matcher and
+   * matcher count is equal to attribute count.
    *
-   * @param attributeSets array of attribute matcher sets
+   * @param attributeMatchers array of attribute matcher sets
    * @return this
    */
   @SafeVarargs
   @CanIgnoreReturnValue
   @SuppressWarnings("varargs") // required to avoid warning
-  public final MetricAssert hasDataPointsWithAttributes(Set<AttributeMatcher>... attributeSets) {
+  public final MetricAssert hasDataPointsWithAttributes(
+      Set<AttributeMatcher>... attributeMatchers) {
     return checkDataPoints(
         dataPoints -> {
           dataPointsCommonCheck(dataPoints);
 
-          boolean[] matchedSets = new boolean[attributeSets.length];
+          boolean[] matchedSets = new boolean[attributeMatchers.length];
 
           // validate each datapoint attributes match exactly one of the provided attributes sets
           for (NumberDataPoint dataPoint : dataPoints) {
-            Set<AttributeMatcher> dataPointAttributes = toSet(dataPoint.getAttributesList());
+            Map<String, String> dataPointAttributes = toMap(dataPoint.getAttributesList());
             int matchCount = 0;
-            for (int i = 0; i < attributeSets.length; i++) {
-              if (attributeSets[i].equals(dataPointAttributes)) {
+            for (int i = 0; i < attributeMatchers.length; i++) {
+              if (matchAttributes(attributeMatchers[i], dataPointAttributes)) {
                 matchedSets[i] = true;
                 matchCount++;
               }
             }
 
             info.description(
                 "data point attributes '%s' for metric '%s' must match exactly one of the attribute sets '%s'",
-                dataPointAttributes, actual.getName(), Arrays.asList(attributeSets));
+                dataPointAttributes, actual.getName(), Arrays.asList(attributeMatchers));
             integers.assertEqual(info, matchCount, 1);
           }
 
           // check that all attribute sets matched at least once
           for (int i = 0; i < matchedSets.length; i++) {
             info.description(
                 "no data point matched attribute set '%s' for metric '%s'",
-                attributeSets[i], actual.getName());
+                attributeMatchers[i], actual.getName());
             objects.assertEqual(info, matchedSets[i], true);
           }
         });
   }
 
-  private static Set<AttributeMatcher> toSet(List<KeyValue> list) {
+  private static boolean matchAttributes(
+      Set<AttributeMatcher> attributeMatchers, Map<String, String> dataPointAttributes) {
+    if (attributeMatchers.size() != dataPointAttributes.size()) {
+      return false;
+    }
+    for (AttributeMatcher matcher : attributeMatchers) {
+      String attributeValue = dataPointAttributes.get(matcher.getAttributeName());
+      if (!matcher.matchesValue(attributeValue)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  private static Map<String, String> toMap(List<KeyValue> list) {
     return list.stream()
-        .map(entry -> attribute(entry.getKey(), entry.getValue().getStringValue()))
-        .collect(Collectors.toSet());
+        .collect(Collectors.toMap(KeyValue::getKey, kv -> kv.getValue().getStringValue()));
   }
 }

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/MetricAssert.java

@@ -36,7 +36,6 @@ protected JmxScraperContainer customizeScraperContainer(
   @Override
   protected MetricsVerifier createMetricsVerifier() {
     return MetricsVerifier.create()
-        .disableStrictMode()
         .add(
             "hbase.master.region_server.count",
             metric ->