Merge pull request #6 from SylvainJuge/assertions-refactoring

Assertions refactoring

Author: robsunday

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

@@ -5,7 +5,6 @@
 
 package io.opentelemetry.contrib.jmxscraper.assertions;
 
-import java.util.Objects;
 import javax.annotation.Nullable;
 
 /** Implements functionality of matching data point attributes. */
@@ -42,25 +41,6 @@ public String getAttributeName() {
     return attributeName;
   }
 
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) {
-      return true;
-    }
-    if (!(o instanceof AttributeMatcher)) {
-      return false;
-    }
-    AttributeMatcher other = (AttributeMatcher) o;
-    // Do not attributeValue into account so AttributeMatcher instances can be stored in collections
-    // with guarantee of uniqueness per attribute
-    return Objects.equals(attributeName, other.attributeName);
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(attributeName);
-  }
-
   @Override
   public String toString() {
     return attributeValue == null
@@ -76,6 +56,6 @@ public String toString() {
    * @return true if this matcher is matching provided value, false otherwise.
    */
   boolean matchesValue(String value) {
-    return (attributeValue == null) || Objects.equals(attributeValue, 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

@@ -6,8 +6,6 @@
 package io.opentelemetry.contrib.jmxscraper.assertions;
 
 import java.util.Arrays;
-import java.util.Set;
-import java.util.stream.Collectors;
 
 /**
  * Utility class implementing convenience static methods to construct data point attribute matchers
@@ -40,21 +38,16 @@ public static AttributeMatcher attributeWithAnyValue(String name) {
   }
 
   /**
-   * Create a set of attribute matchers that should be used to verify set of data point attributes.
+   * Creates a group of attribute matchers that should be used to verify data point attributes.
    *
-   * @param attributes list of matchers to create set. It must contain matchers with unique names.
-   * @return set of unique attribute matchers
+   * @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
-   *     name.
-   * @see MetricAssert#hasDataPointsWithAttributes(Set[]) for detailed description of the algorithm
-   *     used for matching
+   *     attribute name
+   * @see MetricAssert#hasDataPointsWithAttributes(AttributeMatcherGroup...) for detailed
+   *     description off the algorithm used for matching
    */
-  public static Set<AttributeMatcher> attributeSet(AttributeMatcher... attributes) {
-    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;
+  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,7 +5,7 @@
 
 package io.opentelemetry.contrib.jmxscraper.assertions;
 
-import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeSet;
+import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeGroup;
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import io.opentelemetry.proto.common.v1.KeyValue;
@@ -241,73 +241,54 @@ private void dataPointsCommonCheck(List<NumberDataPoint> dataPoints) {
    */
   @CanIgnoreReturnValue
   public final MetricAssert hasDataPointsWithOneAttribute(AttributeMatcher expectedAttribute) {
-    return hasDataPointsWithAttributes(attributeSet(expectedAttribute));
+    return hasDataPointsWithAttributes(attributeGroup(expectedAttribute));
   }
 
   /**
-   * Verifies that every data point attributes set is matched exactly by one of the matcher sets
-   * provided. Also, each matcher set must match at least one data point attributes set. Data point
-   * attributes set is matched by matcher set 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.
+   * 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 attributeMatchers array of attribute matcher sets
+   * @param matcherGroups array of attribute matcher groups
    * @return this
    */
-  @SafeVarargs
   @CanIgnoreReturnValue
-  @SuppressWarnings("varargs") // required to avoid warning
-  public final MetricAssert hasDataPointsWithAttributes(
-      Set<AttributeMatcher>... attributeMatchers) {
+  public final MetricAssert hasDataPointsWithAttributes(AttributeMatcherGroup... matcherGroups) {
     return checkDataPoints(
         dataPoints -> {
           dataPointsCommonCheck(dataPoints);
 
-          boolean[] matchedSets = new boolean[attributeMatchers.length];
+          boolean[] matchedSets = new boolean[matcherGroups.length];
 
           // validate each datapoint attributes match exactly one of the provided attributes sets
           for (NumberDataPoint dataPoint : dataPoints) {
-            Map<String, String> dataPointAttributes = 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 < attributeMatchers.length; i++) {
-              if (matchAttributes(attributeMatchers[i], dataPointAttributes)) {
+            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'",
-                dataPointAttributes, actual.getName(), Arrays.asList(attributeMatchers));
+                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'",
-                attributeMatchers[i], actual.getName());
+                matcherGroups[i], actual.getName());
             objects.assertEqual(info, matchedSets[i], true);
           }
         });
   }
-
-  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()
-        .collect(Collectors.toMap(KeyValue::getKey, kv -> kv.getValue().getStringValue()));
-  }
 }

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/ActiveMqIntegrationTest.java

@@ -6,7 +6,7 @@
 package io.opentelemetry.contrib.jmxscraper.target_systems;
 
 import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attribute;
-import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeSet;
+import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeGroup;
 
 import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer;
 import java.nio.file.Path;
@@ -48,7 +48,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("{consumer}")
                     .isUpDownCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -59,7 +59,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("{producer}")
                     .isUpDownCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -78,7 +78,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("%")
                     .isGauge()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -107,7 +107,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("{message}")
                     .isUpDownCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -119,7 +119,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("{message}")
                     .isCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -130,7 +130,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("{message}")
                     .isCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -141,7 +141,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("{message}")
                     .isCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))))
         .add(
@@ -152,7 +152,7 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("ms")
                     .isGauge()
                     .hasDataPointsWithAttributes(
-                        attributeSet(
+                        attributeGroup(
                             attribute("destination", "ActiveMQ.Advisory.MasterBroker"),
                             attribute("broker", "localhost"))));
   }

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/CassandraIntegrationTest.java

@@ -6,13 +6,12 @@
 package io.opentelemetry.contrib.jmxscraper.target_systems;
 
 import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attribute;
-import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeSet;
+import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeGroup;
 
 import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer;
-import io.opentelemetry.contrib.jmxscraper.assertions.AttributeMatcher;
+import io.opentelemetry.contrib.jmxscraper.assertions.AttributeMatcherGroup;
 import java.nio.file.Path;
 import java.time.Duration;
-import java.util.Set;
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.containers.wait.strategy.Wait;
 
@@ -162,9 +161,9 @@ protected MetricsVerifier createMetricsVerifier() {
                     .hasUnit("1")
                     .isCounter()
                     .hasDataPointsWithAttributes(
-                        attributeSet(attribute("operation", "RangeSlice")),
-                        attributeSet(attribute("operation", "Read")),
-                        attributeSet(attribute("operation", "Write"))))
+                        attributeGroup(attribute("operation", "RangeSlice")),
+                        attributeGroup(attribute("operation", "Read")),
+                        attributeGroup(attribute("operation", "Write"))))
         .add(
             "cassandra.client.request.error.count",
             metric ->
@@ -184,7 +183,7 @@ protected MetricsVerifier createMetricsVerifier() {
                         errorCountAttributes("Write", "Unavailable")));
   }
 
-  private static Set<AttributeMatcher> errorCountAttributes(String operation, String status) {
-    return attributeSet(attribute("operation", operation), attribute("status", status));
+  private static AttributeMatcherGroup errorCountAttributes(String operation, String status) {
+    return attributeGroup(attribute("operation", operation), attribute("status", status));
   }
 }