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

Maxime/jmxfetch exclude tags #116

Merged
merged 1 commit into from
Dec 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/main/java/org/datadog/jmxfetch/Filter.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Filter {
HashMap<String, Object> filter;
Pattern domainRegex;
ArrayList<Pattern> beanRegexes = null;
ArrayList<String> excludeTags = null;

/**
* A simple class to manipulate include/exclude filter elements more easily
Expand Down Expand Up @@ -105,6 +106,21 @@ public ArrayList<Pattern> getBeanRegexes() {
return this.beanRegexes;
}

public ArrayList<String> getExcludeTags() {
// Return excluded tags as an ArrayList whether it's defined as a list or not

if (this.excludeTags == null) {
if (filter.get("exclude_tags") == null){
this.excludeTags = new ArrayList<String>();
} else {
final Object exclude_tags = filter.get("exclude_tags");
this.excludeTags = toStringArrayList(exclude_tags);
}
}

return this.excludeTags;
}

public String getDomain() {
return (String) filter.get("domain");
}
Expand Down
23 changes: 22 additions & 1 deletion src/main/java/org/datadog/jmxfetch/JMXAttribute.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
Expand All @@ -28,7 +29,7 @@ public abstract class JMXAttribute {
protected static final String ALIAS = "alias";
protected static final String METRIC_TYPE = "metric_type";
private final static Logger LOGGER = Logger.getLogger(JMXAttribute.class.getName());
private static final List<String> EXCLUDED_BEAN_PARAMS = Arrays.asList("domain", "domain_regex", "bean_name", "bean", "bean_regex", "attribute");
private static final List<String> EXCLUDED_BEAN_PARAMS = Arrays.asList("domain", "domain_regex", "bean_name", "bean", "bean_regex", "attribute", "exclude_tags");
private static final String FIRST_CAP_PATTERN = "(.)([A-Z][a-z]+)";
private static final String ALL_CAP_PATTERN = "([a-z0-9])([A-Z])";
private static final String METRIC_REPLACEMENT = "([^a-zA-Z0-9_.]+)|(^[^a-zA-Z]+)";
Expand Down Expand Up @@ -73,6 +74,23 @@ public abstract class JMXAttribute {
this.defaultTagsList = sanitizeParameters(beanParametersList);
}

/**
* Remove tags listed in the 'exclude_tags' list from configuration.
*/
private void applyTagsBlackList() {
Filter include = this.matchingConf.getInclude();
if (include != null) {

for (String excludedTagName : include.getExcludeTags()) {
for (Iterator<String> it = this.defaultTagsList.iterator(); it.hasNext();) {
if (it.next().startsWith(excludedTagName + ":")) {
it.remove();
}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this double for loop, can we have the getExcludeTags method to return a Set, and only iterates over defaultTagsList performing lookups?

}
}
}
}

public static HashMap<String, String> getBeanParametersHash(String beanParametersString) {
String[] beanParameters = beanParametersString.split(",");
HashMap<String, String> beanParamsMap = new HashMap<String, String>(beanParameters.length);
Expand Down Expand Up @@ -349,6 +367,9 @@ public Configuration getMatchingConf() {

public void setMatchingConf(Configuration matchingConf) {
this.matchingConf = matchingConf;

// Now that we have the matchingConf, we can filter out excluded tags
applyTagsBlackList();
}

MBeanAttributeInfo getAttribute() {
Expand Down
25 changes: 25 additions & 0 deletions src/test/java/org/datadog/jmxfetch/TestApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,31 @@ public void testMetricTypes() throws Exception {
assertMetric("test.counter", 0.0, commonTags, 5, "counter");
}

@Test
public void testExcludeTags() throws Exception {
// We expose a few metrics through JMX
SimpleTestJavaApp testApp = new SimpleTestJavaApp();
registerMBean(testApp, "org.datadog.jmxfetch.test:type=SimpleTestJavaApp");

// We do a first collection
initApplication("jmx_exclude_tags.yml");
run();
LinkedList<HashMap<String, Object>> metrics = getMetrics();

// We test for the presence and the value of the metrics we want to collect.
// Tags "type", "newTag" and "env" should be excluded
List<String> commonTags = Arrays.asList(
"instance:jmx_test_instance",
"jmx_domain:org.datadog.jmxfetch.test");

// 15 = 13 metrics from java.lang + the 2 collected (gauge and histogram)
assertEquals(15, metrics.size());

// There should only left 2 tags per metric
assertMetric("test1.gauge", 1000.0, commonTags, 2, "gauge");
assertMetric("test1.histogram", 424242, commonTags, 2, "histogram");
}

/**
* FIXME: Split this test in multiple sub-tests.
*/
Expand Down
22 changes: 22 additions & 0 deletions src/test/resources/jmx_exclude_tags.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
init_config:

instances:
- process_name_regex: .*surefire.*
name: jmx_test_instance
tags:
env: stage
newTag: test
conf:
- include:
domain: org.datadog.jmxfetch.test
exclude_tags:
- env
- type
- newTag
attribute:
ShouldBe1000:
metric_type: gauge
alias: test1.gauge
Int424242:
metric_type: histogram
alias: test1.histogram