-
Notifications
You must be signed in to change notification settings - Fork 130
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 #1580
Merged
+801
−568
Merged
Assertions refactoring #1580
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
c3deb51
New metrics validation framework fundamentals.
robsunday fcf2bf8
Spotless fixes
robsunday d833450
Improved check for not received metrics
robsunday 8cb96ad
Cassandra integration test converted
robsunday f3bac7e
Fine tuning assertion messages.
robsunday c9eb0a7
ActiveMqIntegrationTest converted
robsunday 2c85c38
Spotless fix
robsunday bd1947f
introduce 'register' API
SylvainJuge 1e64f80
introduce dedicated assertThat for metrics
SylvainJuge f7c6373
refactor metrics verifier
SylvainJuge b10a340
add some javadoc & few comments
SylvainJuge 65ddfb3
spotless & minor things
SylvainJuge db54835
add new assertion for attribute entries
SylvainJuge 1bdf694
check for missing assertions
SylvainJuge 9f8a394
verify attributes are checked in strict mode
SylvainJuge 6fb7960
enhance datapoint attributes check
SylvainJuge a458c1c
comments, cleanup and inline a bit
SylvainJuge b9f054c
strict check avoids duplicate assertions
SylvainJuge cf72d19
remove obsolete comments in activemq yaml
SylvainJuge eab7c69
register -> add
SylvainJuge 9c3390c
reformat
SylvainJuge 9392780
refactor cassandra
SylvainJuge 5ae735d
fix lint
SylvainJuge 2c65318
refactor activemq
SylvainJuge a670561
refactor jvm metrics
SylvainJuge df09034
remove unused code
SylvainJuge 06a968f
recycle assertions when we can
SylvainJuge 0e5fd2c
Merge pull request #4 from SylvainJuge/assertions-refactoring-more
robsunday 8062a96
Added some JavaDocs.
robsunday ba95efa
Merge branch 'main' into assertions-refactoring
robsunday 4bed658
Cleanup
robsunday fddc5fc
Spotless fix
robsunday fd82042
Refactoring of data point attribute assertions
robsunday 80994f2
Merge branch 'main' into assertions-refactoring
robsunday b26cf42
Coe review changes
robsunday 0bf10f5
JavaDoc update
robsunday 9f47ef6
JavaDoc update
robsunday 2971ef7
Update jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/…
robsunday 7d7e504
Code review changes
robsunday 19f5d4c
Cleanup
robsunday 6431feb
Additional comments added
robsunday ba0f900
add attribute matcher set class
SylvainJuge fdc64e3
remove equals/hashcode
SylvainJuge 7754e3a
use AttributeMatcherSet for matching
SylvainJuge 7b7f0d0
code cleanup
SylvainJuge cdf4c74
Merge branch 'assertions-refactoring' of github.com:robsunday/opentel…
SylvainJuge b170021
move set matching
SylvainJuge 59a7dfc
inline conversion to map
SylvainJuge 34b3ab3
reformat & cleanup
SylvainJuge 45ba126
simplify matchesValue
SylvainJuge 5e69a82
rename attribute set -> group
SylvainJuge dfe3af3
Merge pull request #6 from SylvainJuge/assertions-refactoring
robsunday File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
61 changes: 61 additions & 0 deletions
61
...integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/AttributeMatcher.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} | ||
} |
60 changes: 60 additions & 0 deletions
60
...rationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/AttributeMatcherGroup.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); | ||
} | ||
} |
53 changes: 53 additions & 0 deletions
53
...egrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/DataPointAttributes.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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)); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] I left this method for convenience, because there are lots of assertions with just one attribute, but I can remove it if it makes matching logic harder to understand. |
||
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())); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
893 changes: 489 additions & 404 deletions
893
...ionTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/HBaseIntegrationTest.java
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor] add javadoc to explain the differences between all those methods: exact match, match with any value (even if the method name is quite clear on the latter, but not on the first on the exact match here).