Assertions refactoring (#1580)

Co-authored-by: Sylvain Juge <[email protected]>
Co-authored-by: jason plumb <[email protected]>

Author: 3 people

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

@@ -0,0 +1,61 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.contrib.jmxscraper.assertions;
+
+import javax.annotation.Nullable;
+
+/** Implements functionality of matching data point attributes. */
+public class AttributeMatcher {
+  private final String attributeName;
+  @Nullable private final String attributeValue;
+
+  /**
+   * Create instance used to match data point attribute with any value.
+   *
+   * @param attributeName matched attribute name
+   */
+  AttributeMatcher(String attributeName) {
+    this(attributeName, null);
+  }
+
+  /**
+   * Create instance used to match data point attribute with te same name and with the same value.
+   *
+   * @param attributeName attribute name
+   * @param attributeValue attribute value
+   */
+  AttributeMatcher(String attributeName, @Nullable String attributeValue) {
+    this.attributeName = attributeName;
+    this.attributeValue = attributeValue;
+  }
+
+  /**
+   * 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
+  public String toString() {
+    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) {
+    return attributeValue == null || attributeValue.equals(value);
+  }
+}

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

@@ -0,0 +1,60 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.contrib.jmxscraper.assertions;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/** Group of attribute matchers */
+public class AttributeMatcherGroup {
+
+  // stored as a Map for easy lookup by name
+  private final Map<String, AttributeMatcher> matchers;
+
+  /**
+   * Constructor for a set of attribute matchers
+   *
+   * @param matchers collection of matchers to build a group from
+   * @throws IllegalStateException if there is any duplicate key
+   */
+  AttributeMatcherGroup(Collection<AttributeMatcher> matchers) {
+    this.matchers =
+        matchers.stream().collect(Collectors.toMap(AttributeMatcher::getAttributeName, m -> m));
+  }
+
+  /**
+   * Checks if attributes match this attribute matcher group
+   *
+   * @param attributes attributes to check as map
+   * @return {@literal true} when the attributes match all attributes from this group
+   */
+  public boolean matches(Map<String, String> attributes) {
+    if (attributes.size() != matchers.size()) {
+      return false;
+    }
+
+    for (Map.Entry<String, String> entry : attributes.entrySet()) {
+      AttributeMatcher matcher = matchers.get(entry.getKey());
+      if (matcher == null) {
+        // no matcher for this key: unexpected key
+        return false;
+      }
+
+      if (!matcher.matchesValue(entry.getValue())) {
+        // value does not match: unexpected value
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  @Override
+  public String toString() {
+    return matchers.values().toString();
+  }
+}

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

@@ -0,0 +1,53 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.contrib.jmxscraper.assertions;
+
+import java.util.Arrays;
+
+/**
+ * Utility class implementing convenience static methods to construct data point attribute matchers
+ * and sets of matchers.
+ */
+public class DataPointAttributes {
+  private DataPointAttributes() {}
+
+  /**
+   * Create instance of matcher that should be used to check if data point attribute with given name
+   * has value identical to the one provided as a parameter (exact match).
+   *
+   * @param name name of the data point attribute to check
+   * @param value expected value of checked 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 should be used to check if data point attribute with given name
+   * exists. Any value of the attribute is considered as matching (any value match).
+   *
+   * @param name name of the data point attribute to check
+   * @return instance of matcher
+   */
+  public static AttributeMatcher attributeWithAnyValue(String name) {
+    return new AttributeMatcher(name);
+  }
+
+  /**
+   * Creates a group of attribute matchers that should be used to verify data point attributes.
+   *
+   * @param attributes list of matchers to create group. It must contain matchers with unique names.
+   * @return group of attribute matchers
+   * @throws IllegalArgumentException if provided list contains two or more matchers with the same
+   *     attribute name
+   * @see MetricAssert#hasDataPointsWithAttributes(AttributeMatcherGroup...) for detailed
+   *     description off the algorithm used for matching
+   */
+  public static AttributeMatcherGroup attributeGroup(AttributeMatcher... attributes) {
+    return new AttributeMatcherGroup(Arrays.asList(attributes));
+  }
+}

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

@@ -5,13 +5,14 @@
 
 package io.opentelemetry.contrib.jmxscraper.assertions;
 
+import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeGroup;
+
 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.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -21,15 +22,13 @@
 import org.assertj.core.api.AbstractAssert;
 import org.assertj.core.internal.Integers;
 import org.assertj.core.internal.Iterables;
-import org.assertj.core.internal.Maps;
 import org.assertj.core.internal.Objects;
 
 public class MetricAssert extends AbstractAssert<MetricAssert, Metric> {
 
   private static final Objects objects = Objects.instance();
   private static final Iterables iterables = Iterables.instance();
   private static final Integers integers = Integers.instance();
-  private static final Maps maps = Maps.instance();
 
   private boolean strict;
 
@@ -59,7 +58,7 @@ private void strictCheck(
     if (!strict) {
       return;
     }
-    String failMsgPrefix = expectedCheckStatus ? "duplicate" : "missing";
+    String failMsgPrefix = expectedCheckStatus ? "Missing" : "Duplicate";
     info.description(
         "%s assertion on %s for metric '%s'", failMsgPrefix, metricProperty, actual.getName());
     objects.assertEqual(info, actualCheckStatus, expectedCheckStatus);
@@ -122,8 +121,8 @@ private MetricAssert hasSum(boolean monotonic) {
     info.description("sum expected for metric '%s'", actual.getName());
     objects.assertEqual(info, actual.hasSum(), true);
 
-    String prefix = monotonic ? "monotonic" : "non-monotonic";
-    info.description(prefix + " sum expected for metric '%s'", actual.getName());
+    String sumType = monotonic ? "monotonic" : "non-monotonic";
+    info.description("sum for metric '%s' is expected to be %s", actual.getName(), sumType);
     objects.assertEqual(info, actual.getSum().getIsMonotonic(), monotonic);
     return this;
   }
@@ -156,6 +155,11 @@ public MetricAssert isUpDownCounter() {
     return this;
   }
 
+  /**
+   * Verifies that there is no attribute in any of data points.
+   *
+   * @return this
+   */
   @CanIgnoreReturnValue
   public MetricAssert hasDataPointsWithoutAttributes() {
     isNotNull();
@@ -195,6 +199,7 @@ private MetricAssert checkDataPoints(Consumer<List<NumberDataPoint>> listConsume
     return this;
   }
 
+  // TODO: To be removed and calls will be replaced with hasDataPointsWithAttributes()
   @CanIgnoreReturnValue
   public MetricAssert hasTypedDataPoints(Collection<String> types) {
     return checkDataPoints(
@@ -229,102 +234,61 @@ private void dataPointsCommonCheck(List<NumberDataPoint> dataPoints) {
   }
 
   /**
-   * Verifies that all data points have all the expected attributes
+   * Verifies that all metric data points have the same expected one attribute
    *
-   * @param attributes expected attributes
+   * @param expectedAttribute attribute matcher to validate data points attributes
    * @return this
    */
-  @SafeVarargs
   @CanIgnoreReturnValue
-  public final MetricAssert hasDataPointsAttributes(Map.Entry<String, String>... attributes) {
-    return checkDataPoints(
-        dataPoints -> {
-          dataPointsCommonCheck(dataPoints);
-
-          Map<String, String> attributesMap = new HashMap<>();
-          for (Map.Entry<String, String> attributeEntry : attributes) {
-            attributesMap.put(attributeEntry.getKey(), attributeEntry.getValue());
-          }
-          for (NumberDataPoint dataPoint : dataPoints) {
-            Map<String, String> dataPointAttributes = toMap(dataPoint.getAttributesList());
-
-            // all attributes must match
-            info.description(
-                "missing/unexpected data points attributes for metric '%s'", actual.getName());
-            containsExactly(dataPointAttributes, attributes);
-            maps.assertContainsAllEntriesOf(info, dataPointAttributes, attributesMap);
-          }
-        });
+  public final MetricAssert hasDataPointsWithOneAttribute(AttributeMatcher expectedAttribute) {
+    return hasDataPointsWithAttributes(attributeGroup(expectedAttribute));
   }
 
   /**
-   * Verifies that all data points have their attributes match one of the attributes set and that
-   * all provided attributes sets matched at least once.
+   * Verifies that every data point attributes is matched exactly by one of the matcher groups
+   * provided. Also, each matcher group must match at least one data point attributes set. Data
+   * point attributes are matched by matcher group if each attribute is matched by one matcher and
+   * each matcher matches one attribute. In other words: number of attributes is the same as number
+   * of matchers and there is 1:1 matching between them.
    *
-   * @param attributeSets sets of attributes as maps
+   * @param matcherGroups array of attribute matcher groups
    * @return this
    */
-  @SafeVarargs
   @CanIgnoreReturnValue
-  @SuppressWarnings("varargs") // required to avoid warning
-  public final MetricAssert hasDataPointsAttributes(Map<String, String>... attributeSets) {
+  public final MetricAssert hasDataPointsWithAttributes(AttributeMatcherGroup... matcherGroups) {
     return checkDataPoints(
         dataPoints -> {
           dataPointsCommonCheck(dataPoints);
 
-          boolean[] matchedSets = new boolean[attributeSets.length];
+          boolean[] matchedSets = new boolean[matcherGroups.length];
 
-          // validate each datapoint attributes match exactly one of the provided attributes set
+          // validate each datapoint attributes match exactly one of the provided attributes sets
           for (NumberDataPoint dataPoint : dataPoints) {
-            Map<String, String> map = toMap(dataPoint.getAttributesList());
-
+            Map<String, String> dataPointAttributes =
+                dataPoint.getAttributesList().stream()
+                    .collect(
+                        Collectors.toMap(KeyValue::getKey, kv -> kv.getValue().getStringValue()));
             int matchCount = 0;
-            for (int i = 0; i < attributeSets.length; i++) {
-              if (mapEquals(map, attributeSets[i])) {
+            for (int i = 0; i < matcherGroups.length; i++) {
+              if (matcherGroups[i].matches(dataPointAttributes)) {
                 matchedSets[i] = true;
                 matchCount++;
               }
             }
 
             info.description(
                 "data point attributes '%s' for metric '%s' must match exactly one of the attribute sets '%s'",
-                map, actual.getName(), Arrays.asList(attributeSets));
+                dataPointAttributes, actual.getName(), Arrays.asList(matcherGroups));
             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());
+                matcherGroups[i], actual.getName());
             objects.assertEqual(info, matchedSets[i], true);
           }
         });
   }
-
-  /**
-   * Map equality utility
-   *
-   * @param m1 first map
-   * @param m2 second map
-   * @return true if the maps have exactly the same keys and values
-   */
-  private static boolean mapEquals(Map<String, String> m1, Map<String, String> m2) {
-    if (m1.size() != m2.size()) {
-      return false;
-    }
-    return m1.entrySet().stream().allMatch(e -> e.getValue().equals(m2.get(e.getKey())));
-  }
-
-  @SafeVarargs
-  @SuppressWarnings("varargs") // required to avoid warning
-  private final void containsExactly(
-      Map<String, String> map, Map.Entry<String, String>... entries) {
-    maps.assertContainsExactly(info, map, entries);
-  }
-
-  private static Map<String, String> toMap(List<KeyValue> list) {
-    return list.stream()
-        .collect(Collectors.toMap(KeyValue::getKey, kv -> kv.getValue().getStringValue()));
-  }
 }