From 2730b5291424228688ac1c3c52859daf2ca07909 Mon Sep 17 00:00:00 2001 From: Chaitanya GSK Date: Tue, 29 Jan 2019 15:36:00 -0800 Subject: [PATCH 1/3] explicit tags fix for scanners --- .../filter/ExplicitTagsFilterFactory.java | 2 +- .../opentsdb/query/filter/FilterUtils.java | 80 +++++++++++++------ .../net/opentsdb/storage/MockDataStore.java | 2 +- .../TestExplicitTagsFilterAndFactory.java | 2 +- .../query/filter/TestFilterUtils.java | 26 +++--- .../net/opentsdb/storage/Tsdb1xScanner.java | 4 +- .../storage/Tsdb1xBigtableScanner.java | 4 +- 7 files changed, 75 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/net/opentsdb/query/filter/ExplicitTagsFilterFactory.java b/core/src/main/java/net/opentsdb/query/filter/ExplicitTagsFilterFactory.java index 326b21f766..163b56869c 100644 --- a/core/src/main/java/net/opentsdb/query/filter/ExplicitTagsFilterFactory.java +++ b/core/src/main/java/net/opentsdb/query/filter/ExplicitTagsFilterFactory.java @@ -65,7 +65,7 @@ public QueryFilter parse(final TSDB tsdb, + "for type: " + type); } - return (QueryFilter) NotFilter.newBuilder() + return (QueryFilter) ExplicitTagsFilter.newBuilder() .setFilter(factory.parse(tsdb, mapper, filter)) .build(); } diff --git a/core/src/main/java/net/opentsdb/query/filter/FilterUtils.java b/core/src/main/java/net/opentsdb/query/filter/FilterUtils.java index ead0fa8879..ffa25ce03a 100644 --- a/core/src/main/java/net/opentsdb/query/filter/FilterUtils.java +++ b/core/src/main/java/net/opentsdb/query/filter/FilterUtils.java @@ -14,7 +14,10 @@ // limitations under the License. package net.opentsdb.query.filter; +import com.google.common.collect.Sets; + import java.util.Map; +import java.util.Set; /** * Utilities for working with filters. @@ -27,9 +30,9 @@ public class FilterUtils { * Private ctor, statics only. */ private FilterUtils() { } - + /** - * Determines if the filter(s) are satisfied with the tag sets. This + * Determines if the filter(s) are satisfied with the tag sets. This * utility will handle chained filters based on the AND or OR operand. * For non-tag handling filters, this method will return true. * @param filter The filter to evaluate. @@ -37,49 +40,76 @@ private FilterUtils() { } * @return True if the filter(s) matched, false if not or true if the * filter(s) were not tag filters. */ - public static boolean matchesTags(final QueryFilter filter, - final Map tags) { + public static boolean matchesTags(final QueryFilter filter, + final Map tags, + Set matched) { if (filter == null) { throw new IllegalArgumentException("Filter cannot be null."); } if (tags == null) { throw new IllegalArgumentException("Tags cannot be null."); } - + + if (filter instanceof ExplicitTagsFilter) { + matched = Sets.newHashSetWithExpectedSize(tags.size()); + final boolean satisfied = matchesTags( + ((ExplicitTagsFilter) filter).getFilter(), tags, matched); + + if (!satisfied) { + return false; + } + if (matched.size() != tags.size()) { + return false; + } + return true; + } + if (filter instanceof TagValueFilter) { - return ((TagValueFilter) filter).matches(tags); + if (((TagValueFilter) filter).matches(tags)) { + if (matched != null) { + matched.add(((TagValueFilter) filter).getTagKey()); + } + return true; + } + return false; } if (filter instanceof TagKeyFilter) { - return ((TagValueFilter) filter).matches(tags); + if (((TagValueFilter) filter).matches(tags)) { + if (matched != null) { + matched.add(((TagKeyFilter) filter).filter()); + } + return true; + } + return false; } if (filter instanceof ChainFilter) { final ChainFilter chain = (ChainFilter) filter; switch (chain.getOp()) { - case AND: - for (final QueryFilter child : chain.getFilters()) { - if (!matchesTags(child, tags)) { - return false; + case AND: + for (final QueryFilter child : chain.getFilters()) { + if (!matchesTags(child, tags, matched)) { + return false; + } } - } - return true; - case OR: - for (final QueryFilter child : chain.getFilters()) { - if (matchesTags(child, tags)) { - return true; + return true; + case OR: + for (final QueryFilter child : chain.getFilters()) { + if (matchesTags(child, tags, matched)) { + return true; + } } - } - return false; - default: - throw new IllegalStateException("Unsupported chain operator: " - + chain.getOp()); + return false; + default: + throw new IllegalStateException("Unsupported chain operator: " + + chain.getOp()); } } if (filter instanceof NotFilter) { - return !matchesTags(((NotFilter) filter).getFilter(), tags); + return !matchesTags(((NotFilter) filter).getFilter(), tags, matched); } - + // it's a different type of filter so return true. return true; } - + } diff --git a/core/src/main/java/net/opentsdb/storage/MockDataStore.java b/core/src/main/java/net/opentsdb/storage/MockDataStore.java index 3e083aeb48..096c436f60 100644 --- a/core/src/main/java/net/opentsdb/storage/MockDataStore.java +++ b/core/src/main/java/net/opentsdb/storage/MockDataStore.java @@ -558,7 +558,7 @@ public void run() { } if (filter != null) { - if (!FilterUtils.matchesTags(filter, entry.getKey().tags())) { + if (!FilterUtils.matchesTags(filter, entry.getKey().tags(), null)) { continue; } } diff --git a/core/src/test/java/net/opentsdb/query/filter/TestExplicitTagsFilterAndFactory.java b/core/src/test/java/net/opentsdb/query/filter/TestExplicitTagsFilterAndFactory.java index 8ff2e3c105..6aa0b83b0f 100644 --- a/core/src/test/java/net/opentsdb/query/filter/TestExplicitTagsFilterAndFactory.java +++ b/core/src/test/java/net/opentsdb/query/filter/TestExplicitTagsFilterAndFactory.java @@ -52,7 +52,7 @@ public void parse() throws Exception { .thenReturn(exp_factory); JsonNode node = JSON.getMapper().readTree(json); - NotFilter filter = (NotFilter) exp_factory.parse(tsdb, JSON.getMapper(), node); + ExplicitTagsFilter filter = (ExplicitTagsFilter) exp_factory.parse(tsdb, JSON.getMapper(), node); assertTrue(filter.getFilter() instanceof TagValueLiteralOrFilter); assertEquals("web01|web02", ((TagValueLiteralOrFilter) filter.getFilter()).getFilter()); diff --git a/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java b/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java index ceecbb9289..cfccfdf9b3 100644 --- a/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java +++ b/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java @@ -45,7 +45,7 @@ public void matchesTags() throws Exception { .setFilter("tyrion") .build()) .build(); - assertTrue(FilterUtils.matchesTags(filter, tags)); + assertTrue(FilterUtils.matchesTags(filter, tags, null)); filter = ChainFilter.newBuilder() .setOp(FilterOp.AND) @@ -58,7 +58,7 @@ public void matchesTags() throws Exception { .setFilter("tyrion") .build()) .build(); - assertFalse(FilterUtils.matchesTags(filter, tags)); + assertFalse(FilterUtils.matchesTags(filter, tags, null)); // not chains filter = NotFilter.newBuilder() @@ -74,7 +74,7 @@ public void matchesTags() throws Exception { .build()) .build()) .build(); - assertFalse(FilterUtils.matchesTags(filter, tags)); + assertFalse(FilterUtils.matchesTags(filter, tags, null)); filter = NotFilter.newBuilder() .setFilter(ChainFilter.newBuilder() @@ -89,7 +89,7 @@ public void matchesTags() throws Exception { .build()) .build()) .build(); - assertTrue(FilterUtils.matchesTags(filter, tags)); + assertTrue(FilterUtils.matchesTags(filter, tags, null)); // nested chains filter = ChainFilter.newBuilder() @@ -110,7 +110,7 @@ public void matchesTags() throws Exception { .build()) .build()) .build(); - assertTrue(FilterUtils.matchesTags(filter, tags)); + assertTrue(FilterUtils.matchesTags(filter, tags, null)); filter = ChainFilter.newBuilder() .setOp(FilterOp.AND) @@ -130,20 +130,20 @@ public void matchesTags() throws Exception { .build()) .build()) .build(); - assertFalse(FilterUtils.matchesTags(filter, tags)); + assertFalse(FilterUtils.matchesTags(filter, tags, null)); // singles filter = TagValueLiteralOrFilter.newBuilder() .setTagKey("host") .setFilter("web01") .build(); - assertTrue(FilterUtils.matchesTags(filter, tags)); + assertTrue(FilterUtils.matchesTags(filter, tags, null)); filter = TagValueLiteralOrFilter.newBuilder() .setTagKey("owner") .setFilter("tyrion") .build(); - assertFalse(FilterUtils.matchesTags(filter, tags)); + assertFalse(FilterUtils.matchesTags(filter, tags, null)); filter = NotFilter.newBuilder() .setFilter(TagValueLiteralOrFilter.newBuilder() @@ -151,7 +151,7 @@ public void matchesTags() throws Exception { .setFilter("web01") .build()) .build(); - assertFalse(FilterUtils.matchesTags(filter, tags)); + assertFalse(FilterUtils.matchesTags(filter, tags, null)); filter = NotFilter.newBuilder() .setFilter(TagValueLiteralOrFilter.newBuilder() @@ -159,22 +159,22 @@ public void matchesTags() throws Exception { .setFilter("tyrion") .build()) .build(); - assertTrue(FilterUtils.matchesTags(filter, tags)); + assertTrue(FilterUtils.matchesTags(filter, tags, null)); // TODO - tag key filters filter = MetricLiteralFilter.newBuilder() .setMetric("sys.cpu.user") .build(); - assertTrue(FilterUtils.matchesTags(filter, tags)); + assertTrue(FilterUtils.matchesTags(filter, tags, null)); try { - FilterUtils.matchesTags(null, tags); + FilterUtils.matchesTags(null, tags, null); fail("Expected IllegalArgumentException"); } catch (IllegalArgumentException e) { } try { - FilterUtils.matchesTags(filter, null); + FilterUtils.matchesTags(filter, null, null); fail("Expected IllegalArgumentException"); } catch (IllegalArgumentException e) { } } diff --git a/storage/asynchbase/src/main/java/net/opentsdb/storage/Tsdb1xScanner.java b/storage/asynchbase/src/main/java/net/opentsdb/storage/Tsdb1xScanner.java index 5f233e3ebf..2cb43ab116 100644 --- a/storage/asynchbase/src/main/java/net/opentsdb/storage/Tsdb1xScanner.java +++ b/storage/asynchbase/src/main/java/net/opentsdb/storage/Tsdb1xScanner.java @@ -857,7 +857,7 @@ public Void call(final TimeSeriesStringId id) throws Exception { deferred.callback(ex); } - if (FilterUtils.matchesTags(filter, id.tags())) { + if (FilterUtils.matchesTags(filter, id.tags(), null)) { synchronized (keepers) { keepers.add(hash); } @@ -940,4 +940,4 @@ public Void call(final Exception ex) throws Exception { } } } -} \ No newline at end of file +} diff --git a/storage/bigtable/src/main/java/net/opentsdb/storage/Tsdb1xBigtableScanner.java b/storage/bigtable/src/main/java/net/opentsdb/storage/Tsdb1xBigtableScanner.java index 0f012546fd..e8a993a17a 100644 --- a/storage/bigtable/src/main/java/net/opentsdb/storage/Tsdb1xBigtableScanner.java +++ b/storage/bigtable/src/main/java/net/opentsdb/storage/Tsdb1xBigtableScanner.java @@ -854,7 +854,7 @@ public Void call(final TimeSeriesStringId id) throws Exception { deferred.callback(ex); } - if (FilterUtils.matchesTags(filter, id.tags())) { + if (FilterUtils.matchesTags(filter, id.tags(), null)) { synchronized (keepers) { keepers.add(hash); } @@ -938,4 +938,4 @@ public Void call(final Exception ex) throws Exception { } } -} \ No newline at end of file +} From b80929aa0aa5f67d77a1099e52996c72108c4f66 Mon Sep 17 00:00:00 2001 From: Chaitanya GSK Date: Tue, 29 Jan 2019 16:34:43 -0800 Subject: [PATCH 2/3] UT for explicit tags filter --- .../query/filter/TestFilterUtils.java | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java b/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java index cfccfdf9b3..34632a750e 100644 --- a/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java +++ b/core/src/test/java/net/opentsdb/query/filter/TestFilterUtils.java @@ -28,6 +28,60 @@ public class TestFilterUtils { + @Test + public void matchesExplicitTags() throws Exception { + Map tags = Maps.newHashMap(); + tags.put("host", "web01"); + + // nested explicit tags chains + QueryFilter filter = ChainFilter.newBuilder() + .setOp(FilterOp.OR) + .addFilter(TagValueLiteralOrFilter.newBuilder() + .setTagKey("owner") + .setFilter("tyrion") + .build()) + .addFilter(ChainFilter.newBuilder() + .setOp(FilterOp.OR) + .addFilter(TagValueLiteralOrFilter.newBuilder() + .setTagKey("host") + .setFilter("web02") + .build()) + .addFilter(TagValueLiteralOrFilter.newBuilder() + .setTagKey("host") + .setFilter("web01") + .build()) + .build()) + .build(); + + QueryFilter explicitFilter = ExplicitTagsFilter.newBuilder().setFilter(filter).build(); + assertTrue(FilterUtils.matchesTags(explicitFilter, tags, null)); + + + // nested explicit tags chains + filter = ChainFilter.newBuilder() + .setOp(FilterOp.OR) + .addFilter(TagValueLiteralOrFilter.newBuilder() + .setTagKey("owner") + .setFilter("tyrion") + .build()) + .addFilter(ChainFilter.newBuilder() + .setOp(FilterOp.OR) + .addFilter(TagValueLiteralOrFilter.newBuilder() + .setTagKey("host") + .setFilter("web02") + .build()) + .addFilter(TagValueLiteralOrFilter.newBuilder() + .setTagKey("host1") + .setFilter("web01") + .build()) + .build()) + .build(); + + explicitFilter = ExplicitTagsFilter.newBuilder().setFilter(filter).build(); + assertFalse(FilterUtils.matchesTags(explicitFilter, tags, null)); + + } + @Test public void matchesTags() throws Exception { Map tags = Maps.newHashMap(); @@ -111,7 +165,7 @@ public void matchesTags() throws Exception { .build()) .build(); assertTrue(FilterUtils.matchesTags(filter, tags, null)); - + filter = ChainFilter.newBuilder() .setOp(FilterOp.AND) .addFilter(TagValueLiteralOrFilter.newBuilder() From 6d1c48687c8fbc054765a36bec5ef82f766b8224 Mon Sep 17 00:00:00 2001 From: Chaitanya GSK Date: Wed, 30 Jan 2019 12:43:17 -0800 Subject: [PATCH 3/3] Add explicit query tag in ES query --- .../NamespacedAggregatedDocumentSchema.java | 28 ++++++++++++----- ...TestNamespacedAggregatedDocumentQuery.java | 30 +++++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/implementation/elasticsearch/src/main/java/net/opentsdb/meta/NamespacedAggregatedDocumentSchema.java b/implementation/elasticsearch/src/main/java/net/opentsdb/meta/NamespacedAggregatedDocumentSchema.java index 9906f1f30a..cda4e9f2a6 100644 --- a/implementation/elasticsearch/src/main/java/net/opentsdb/meta/NamespacedAggregatedDocumentSchema.java +++ b/implementation/elasticsearch/src/main/java/net/opentsdb/meta/NamespacedAggregatedDocumentSchema.java @@ -315,19 +315,31 @@ public Deferred runQuery( filter = queryPipelineContext.query().getFilter( timeSeriesDataSourceConfig.getFilterId()); } - - final ChainFilter.Builder builder = ChainFilter.newBuilder() - .setOp(FilterOp.AND) - .addFilter(MetricLiteralFilter.newBuilder() - .setMetric(metric) - .build()); + + QueryFilter buildFilter; + + final ChainFilter.Builder builder = ChainFilter + .newBuilder() + .setOp(FilterOp.AND) + .addFilter(MetricLiteralFilter.newBuilder().setMetric(metric).build()); if (filter != null) { builder.addFilter(filter); } + + if (filter instanceof ExplicitTagsFilter) { + final ExplicitTagsFilter.Builder explicitTagBuilder = + ExplicitTagsFilter + .newBuilder() + .setFilter(builder.build()); + buildFilter = explicitTagBuilder.build(); + } else { + buildFilter = builder.build(); + } + final MetaQuery query = DefaultMetaQuery.newBuilder() .setType(QueryType.TIMESERIES) .setNamespace(namespace) - .setFilter(builder.build()) + .setFilter(buildFilter) .build(); SearchSourceBuilder search = NamespacedAggregatedDocumentQueryBuilder @@ -663,4 +675,4 @@ private TimeSeriesId buildTimeseries(final String metric, final return timeseries; } -} \ No newline at end of file +} diff --git a/implementation/elasticsearch/src/test/java/net/opentsdb/meta/TestNamespacedAggregatedDocumentQuery.java b/implementation/elasticsearch/src/test/java/net/opentsdb/meta/TestNamespacedAggregatedDocumentQuery.java index d1847f58b2..a338302b6e 100644 --- a/implementation/elasticsearch/src/test/java/net/opentsdb/meta/TestNamespacedAggregatedDocumentQuery.java +++ b/implementation/elasticsearch/src/test/java/net/opentsdb/meta/TestNamespacedAggregatedDocumentQuery.java @@ -51,6 +51,36 @@ public void testTagValueRegexQuery() { + "{\"term\":{\"tags.key.lowercase\":\"host\"}}]}},\"path\":\"tags\"}}")); } + + @Test + public void testExplicitTagQuery() { + + QueryFilter chainFil = ChainFilter.newBuilder().setOp(ChainFilter.FilterOp.AND) + .addFilter(TagValueRegexFilter.newBuilder().setFilter("cpu").setTagKey("host").build()) + .build(); + MetaQuery query = DefaultMetaQuery.newBuilder() + .setFrom(0) + .setTo(5) + .setNamespace("Yahoo") + .setFilter( ExplicitTagsFilter.newBuilder().setFilter(chainFil).build()) + .setType(QueryType.TIMESERIES) + .build(); + + SearchSourceBuilder source = NamespacedAggregatedDocumentQueryBuilder + .newBuilder(query) + .build(); + + String s = source.toString().replaceAll("\n","") + .replaceAll(" ", ""); + System.out.println(s); + assertTrue(s.contains("\"from\":0")); + assertTrue(s.contains("\"size\":5")); + assertTrue(s.contains("\"query\":{\"bool\":{\"must\":[{\"term\":{\"tags_value\":1}},{\"bool\":" + + "{\"must\":{\"nested\":{\"filter\":{\"bool\":{\"must\":[{\"regexp\":" + + "{\"tags.value\":\".*cpu.*\"}},{\"term\":{\"tags.key.lowercase\":\"host\"}}]}}," + + "\"path\":\"tags\"}}}}]}}")); + + } // // @Test // public void testMetricRegexQuery() {