Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertions refactoring #6

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -42,25 +42,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
Expand All @@ -76,6 +57,9 @@ 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);
if (attributeValue == null) {
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
return Objects.equals(attributeValue, value);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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;

/** Holder class for a set of attribute matchers */
public class AttributeMatcherSet {

// 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 set from
* @throws IllegalStateException if there is any duplicate key
*/
AttributeMatcherSet(Collection<AttributeMatcher> matchers) {
this.matchers =
matchers.stream().collect(Collectors.toMap(AttributeMatcher::getAttributeName, m -> m));
}

Map<String, AttributeMatcher> getMatchers() {
return matchers;
}

/**
* Checks if attributes match this attribute matcher set
*
* @param attributes attributes to check as map
* @return {@literal true} when the attributes match all attributes from this set
*/
public boolean matches(Map<String, String> attributes) {
Copy link
Author

@SylvainJuge SylvainJuge Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[for reviewer] we can move part of the matching logic here as we don't need to generate the assertions from here. details on potential mismatches are provided by the calling code in MetricAssert. Another benefit of doing this is that is allows to not expose elsewhere that it's stored in map.

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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -46,15 +44,10 @@ public static AttributeMatcher attributeWithAnyValue(String name) {
* @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
* used for matching
* @see MetricAssert#hasDataPointsWithAttributes(AttributeMatcherSet...) 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 AttributeMatcherSet attributeSet(AttributeMatcher... attributes) {
return new AttributeMatcherSet(Arrays.asList(attributes));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,8 @@ public final MetricAssert hasDataPointsWithOneAttribute(AttributeMatcher expecte
* @param attributeMatchers array of attribute matcher sets
* @return this
*/
@SafeVarargs
@CanIgnoreReturnValue
@SuppressWarnings("varargs") // required to avoid warning
public final MetricAssert hasDataPointsWithAttributes(
Set<AttributeMatcher>... attributeMatchers) {
public final MetricAssert hasDataPointsWithAttributes(AttributeMatcherSet... attributeMatchers) {
return checkDataPoints(
dataPoints -> {
dataPointsCommonCheck(dataPoints);
Expand All @@ -267,10 +264,11 @@ public final MetricAssert hasDataPointsWithAttributes(

// 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)) {
if (attributeMatchers[i].matches(dataPointAttributes)) {
matchedSets[i] = true;
matchCount++;
}
Expand All @@ -292,22 +290,4 @@ public final MetricAssert hasDataPointsWithAttributes(
});
}

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()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@
import static io.opentelemetry.contrib.jmxscraper.assertions.DataPointAttributes.attributeSet;

import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer;
import io.opentelemetry.contrib.jmxscraper.assertions.AttributeMatcher;
import io.opentelemetry.contrib.jmxscraper.assertions.AttributeMatcherSet;
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;

Expand Down Expand Up @@ -184,7 +183,7 @@ protected MetricsVerifier createMetricsVerifier() {
errorCountAttributes("Write", "Unavailable")));
}

private static Set<AttributeMatcher> errorCountAttributes(String operation, String status) {
private static AttributeMatcherSet errorCountAttributes(String operation, String status) {
return attributeSet(attribute("operation", operation), attribute("status", status));
}
}